Skip to content

Conversation

@THardy98
Copy link
Contributor

What was changed

Expose InitialVersioningBehavior on

  • ContinueAsNewError
  • ContinueAsNewErrorOptions
    this specifies the versioning behavior that the first task of the new run should use.

Expose continueAsNewSuggestedReasons on WorkflowInfo, which describes the reason(s) that the CAN is suggested.

Why?

Provides workflow version control on CAN boundaries. This change in particular allows users to auto-upgrade on CAN.

  1. Part of: Expose upgrade on continue as new in SDKs features#738

  2. How was this tested:
    Integration tests

  3. Any docs updates needed?
    Yes.

@THardy98 THardy98 requested a review from a team as a code owner January 28, 2026 21:06
@THardy98 THardy98 requested a review from carlydf January 28, 2026 21:06
Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

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

Nothing blocking, unsure if @yuandrew also wants to review


// InitialVersioningBehavior specifies the versioning behavior that the first task of the new run should use.
// For example, choose to AutoUpgrade on continue-as-new instead of inheriting the pinned version of the previous run.
InitialVersioningBehavior ContinueAsNewVersioningBehavior
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be marked experimental IMO

// 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.
func (wInfo *WorkflowInfo) GetContinueAsNewSuggestedReasons() []ContinueAsNewSuggestedReason {
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be marked experimental IMO

return false
}
return false
}, 10*time.Second, 300*time.Millisecond)
Copy link
Member

Choose a reason for hiding this comment

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

Would like to see all of the worker deployment "eventually" stuff reduces to max timeout 5s and interval 100ms if we can, but not blocking

func convertContinueAsNewSuggestedReasonsFromProto(
reasons []enumspb.SuggestContinueAsNewReason,
) []ContinueAsNewSuggestedReason {
var converted []ContinueAsNewSuggestedReason
Copy link
Member

Choose a reason for hiding this comment

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

Pedantic, but you should pre-set the capacity here if you know it

@yuandrew
Copy link
Contributor

I'd like to take a look, will look later today or early tmr

Copy link
Contributor

@yuandrew yuandrew left a comment

Choose a reason for hiding this comment

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

one small nit, but LGTM otherwise. Agree with all of Chad's points on marking experimental and decreasing eventually() bounds

if options.RetryPolicy != nil {
continueAsNewErr.RetryPolicy = options.RetryPolicy
}
if options.InitialVersioningBehavior != ContinueAsNewVersioningBehaviorUnspecified {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I don't think we need this if check, since the default of both continueAsNewErr.InitialVersioningBehavior and options.InitialVersioningBehavior are the same

Copy link
Contributor

@carlydf carlydf left a comment

Choose a reason for hiding this comment

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

approved! not sure why tests are failing, but lmk if you want any help fixing them

@THardy98
Copy link
Contributor Author

THardy98 commented Feb 2, 2026

approved! not sure why tests are failing, but lmk if you want any help fixing them

We're testing on a version of CLI (v1.5.0-rc) that doesn't have your downstream server changes and so we're hitting timeouts. Testing locally on CLI main (which uses version v1.30.0-148.4 - has your changes) tests pass

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants