-
Notifications
You must be signed in to change notification settings - Fork 213
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
Add Deployment Names to Worker options #1675
Add Deployment Names to Worker options #1675
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving since it's just going to a branch
internal/worker.go
Outdated
// Optional: Assign a Deployment Name to this worker, an identifier for Worker Versioning that | ||
// groups task queues for the given BuildID. | ||
// NOTE: Experimental | ||
// Note: Both BuildID and UseBuildIDForVersioning need to also be set to enable the new Worker Versioning-3 feature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit using Note
and NOTE
in the same doc we should be consistent here
Can we validate this note and fail if DeploymentName
is set but BuildID
or UseBuildIDForVersioning
is not set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I think sometimes we may want to provide a Deployment Name even if we have UseBuildIDForVersioning=false
, the same as we can currently provide the BuildID without forcing versioning. For example, when turning versioning on dynamically with an env property, or using it with visibility queries, or with pollers info...
Further comments will be addressed before merging versioning-3 into master |
cc508c6
into
temporalio:versioning-3
@@ -113,6 +113,8 @@ type ( | |||
BuildID string | |||
// Whether the worker is using the versioning feature. | |||
UseVersioning bool | |||
// An identifier to group task queues based on Build ID. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very low-level description. The point of a deployment is to abstract the task queues.
I would say something like "A Deployment corresponds to a set of Worker instances you want deployed together. For example, it may represent a Worker service. Each time you deploy, you'll be changing the build ID for that deployment."
What was changed
Deployment Names are a new way to group task queues, based on Build IDs, to simplify rollouts in versioning-3.
In this PR we just add a new Worker Option for it, propagate it during polling, and include it with task completion responses.