Skip to content

Fix log template evaluation#4623

Merged
jpbempel merged 1 commit into
masterfrom
jpbempel/fix-valuescript-result
Jan 30, 2023
Merged

Fix log template evaluation#4623
jpbempel merged 1 commit into
masterfrom
jpbempel/fix-valuescript-result

Conversation

@jpbempel

Copy link
Copy Markdown
Member

What Does This Do

ValueScript is storing result value in a field which can be overwritten by another execution. Now the execute method returns directly the result value and ValueScript became effectively stateless regarding execution.

Motivation

Bug in log template evaluation for concurrent requests

Additional Notes

ValueScript is storing result value in a field which can be
overwritten by another execution. Now the execute method returns
directly the result value and ValueScript became effectively stateless
regarding execution.
@jpbempel
jpbempel requested a review from a team as a code owner January 30, 2023 16:20
@jpbempel
jpbempel requested review from ojung and removed request for a team January 30, 2023 16:20
@@ -47,26 +48,10 @@ public void addStack(List<CapturedStackFrame> stack) {}

@Override
public String build() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

find this method name now confusing... with this is not getMessage and executeExpressions is build?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one implementation of SummaryBuilder with the original idea that we need to gather all infos about the snapshot before building the summary, hence the builder.
With LogTemplate we move the building of the summary at context adding.
If we get rid of the SnapshotSummaryBuilder we can revisit it :)

@ojung ojung left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jpbempel
jpbempel merged commit 8e35845 into master Jan 30, 2023
@jpbempel
jpbempel deleted the jpbempel/fix-valuescript-result branch January 30, 2023 21:21
@jpbempel jpbempel added the comp: debugger Dynamic Instrumentation label Jan 30, 2023
@github-actions github-actions Bot added this to the 1.7.0 milestone Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp: debugger Dynamic Instrumentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants