Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail registration without versioning behavior #1742

Conversation

antlai-temporal
Copy link
Contributor

What was changed

Remove programmatic SetVersioningBehavior API, and fail workflow registration when behavior missing.See #1739 for details.

  1. Closes
    Fail registration of workflow types without versioning behavior #1739
  2. How was this tested:

One system test

@antlai-temporal antlai-temporal requested a review from a team as a code owner December 3, 2024 22:45
@@ -413,8 +413,7 @@ type (
// inside a workflow as a child workflow.
Name string
DisableAlreadyRegisteredCheck bool
// Optional: Provides a default Versioning Behavior to workflows of this type.
// See workflow.SetVersioningBehavior to override this default.
// Optional: Provides a Versioning Behavior to workflows of this type.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we mention that this is not optional of there is no default behavior and DeploymentOptions.UseBuildIDForVersioning==true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I'll make this more clear.


ts.worker.Stop()
ts.workerStopped = true
w := worker.New(ts.client, ts.taskQueueName, worker.Options{
w := worker.New(c, ts.taskQueueName, worker.Options{
BuildID: "1.0",
UseBuildIDForVersioning: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

is there any different between UseBuildIDForVersioning and DeploymentOptions.UseBuildIDForVersioning? Do we want to make UseBuildIDForVersioning exclusively for versioning 1-2 and the latter for versioning 3?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no DeploymentOptions.UseBuildIDForVersioning until we deprecate versioning-2, otherwise it is just too confusing to have same field twice without deprecation.

@antlai-temporal antlai-temporal merged commit 6631916 into temporalio:versioning-3 Dec 5, 2024
10 of 12 checks passed
@antlai-temporal antlai-temporal mentioned this pull request Dec 5, 2024
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.

3 participants