Add last BuildID to workflow info #1335
Conversation
Sushisource
force-pushed
the
add-build-id-wfinfo
branch
from
9e3d054 to
97e6583
Compare
|
Realized I did this a bit of a silly way at first. Draft while I fix it. |
| ts.Equal("1.0", lastBuildID) | ||
|
|
||
| // Add new compat ver | ||
| err = ts.client.UpdateWorkerBuildIdCompatibility(ctx, &client.UpdateWorkerBuildIdCompatibilityOptions{ |
There was a problem hiding this comment.
nit: this doesn't require versioning to test, you can use build ID with unversioned workers.
There was a problem hiding this comment.
Could make sense to test with and without versioning just to be safe.
There was a problem hiding this comment.
The bit you reviewed is subtly wrong in terms of querying on a cached worker after the worker has completed a task with its' ID. New test also makes sure the first task doesn't have an ID, which covers this path.
(will need another update to fix the problem I mention)
Sushisource
force-pushed
the
add-build-id-wfinfo
branch
from
97e6583 to
2885d7a
Compare
Sushisource
force-pushed
the
add-build-id-wfinfo
branch
from
1b3434e to
a33c6d3
Compare
Sushisource
force-pushed
the
add-build-id-wfinfo
branch
2 times, most recently
from
0626a9f to
fdc5610
Compare
Sushisource
force-pushed
the
add-build-id-wfinfo
branch
from
fdc5610 to
fb4f811
Compare
| // GetCurrentBuildID returns the Build ID of the worker that processed this task, which may be | ||
| // empty. During replay this id may not equal the id of the replaying worker. If not replaying and | ||
| // this worker has a defined Build ID, it will equal that ID. It is safe to use for branching. | ||
| func (wInfo *WorkflowInfo) GetCurrentBuildID() string { |
There was a problem hiding this comment.
May want to add a note for queries. Since they are not replaying, but also not using the current workers build ID.
| @@ -1016,6 +1026,11 @@ ProcessEvents: | |||
| } else { | |||
| w.workflowInfo.BinaryChecksum = binaryChecksum | |||
| } | |||
| if isReplay { | |||
| w.workflowInfo.currentTaskBuildID = currentBuildID | |||
| } else if !isReplay && !isQueryOnlyTask { | |||
There was a problem hiding this comment.
can we add a comment here explaining why a query only task is treated special.
What was changed
Add the BuildID of the last completed task to info. Importantly this does not use the worker's current build id if it isn't in history, as that would be nondeterministic, unlike Binary Checksum which is guaranteed to be set as soon as the first WFT is completed which is not the case here.
Why?
Useful
Checklist
Part of [Feature Request] Expose build ID via WorkflowInfo. features#253
How was this tested:
Added IT
Any docs updates needed?