Add trampolining API for versioned continue-as-new#2216
Add trampolining API for versioned continue-as-new#2216
Conversation
carlydf
left a comment
carlydf
left a comment
There was a problem hiding this comment.
approved once a few comments that still reference the previous trampolining implementation are fixed
- ContinueAsNewVersioningBehavior/InitialVersioningBehavior: "Upgrade-on-Continue-as-New is currently experimental" - ContinueAsNewSuggestedReason: "ContinueAsNewSuggestedReasons are currently experimental" - Update test comment to reflect new GetTargetWorkflowDeploymentVersionChanged() API Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tWorkerDeploymentVersionChanged
yuandrew
left a comment
yuandrew
left a comment
There was a problem hiding this comment.
Requested small docs improvements, but looks good otherwise
workflow/workflow.go
Outdated
| // If the source workflow was Pinned, the new workflow will start Pinned to the same Build ID. | ||
| // If the source workflow had a Pinned Versioning Override, the new workflow will inherit that Versioning Override. | ||
| // | ||
| // Exposed as: [go.temporal.io/sdk/workflow.ContinueAsNewVersioningBehaviorUnspecified] |
There was a problem hiding this comment.
This "Exposed as:" should be only on internal/ files. Please remove any instances in this file
| // first workflow task completes, use whatever Versioning Behavior the workflow is annotated with in the workflow | ||
| // code. | ||
| // | ||
| // Note that if the previous workflow had a Pinned override, that override will be inherited by the new workflow |
There was a problem hiding this comment.
Should we do Note: here to match the other notes?
| // GetContinueAsNewSuggestedReasons returns a list of reasons why continue as new is suggested, | ||
| // if the server is configured to suggest continue as new and it is suggested. | ||
| // This value may change throughout the life of the workflow. | ||
| // Note: ContinueAsNewSuggestedReasons are currently experimental. |
There was a problem hiding this comment.
Can we add a extra empty // line so the Note renders on a separate line?
|
|
||
| const ( | ||
| // ContinueAsNewSuggestedReasonUnspecified - The reason is unknown. | ||
| // Exposed as: [go.temporal.io/sdk/workflow.ContinueAsNewSuggestedReasonUnspecified] |
There was a problem hiding this comment.
Can we have an extra // line to the "Exposed as" line renders as its own line? I should probably make the doclink tool enforce this
There was a problem hiding this comment.
Both here and in all of the other places in this file
3 participants
Summary
GetTargetWorkflowDeploymentVersionChanged()onWorkflowInfofor detecting when a workflow's target deployment version has changedContinueAsNewVersioningBehaviortype withAutoUpgradeoption forContinueAsNewError/ContinueAsNewErrorOptionsContinueAsNewSuggestedReasonenum andGetContinueAsNewSuggestedReasons()onWorkflowInfoSetContinueAsNewSuggestedReasons()andSetTargetWorkflowDeploymentVersionChanged()How this differs from #2171
This PR implements the API change from temporalio/api#709, which replaces the approach in #2171.
Problem with #2171's approach: It expanded
SuggestContinueAsNewwith aTargetWorkerDeploymentVersionChangedreason. Users with existingif suggestContinueAsNew { doCAN() }code in Pinned workflows would get stuck in infinite CAN loops, since the default CAN behavior for Pinned workflows is to stay Pinned on the same version.This PR's approach: Adds a separate
targetWorkerDeploymentVersionChangedboolean flag onWorkflowInfo, keeping version-change detection completely independent from the continue-as-new-suggested mechanism. This makes upgrade-on-CAN fully opt-in on both the detection and action sides:GetTargetWorkflowDeploymentVersionChanged()InitialVersioningBehavior: ContinueAsNewVersioningBehaviorAutoUpgradePart of: temporalio/features#738
Test plan
TestContinueAsNewWithVersionUpgradevalidates full trampolining flow🤖 Generated with Claude Code