Conversation
|
|
||
| message DeploymentVersionTaskQueueInfo { | ||
| // All the Task Queues that have ever polled from this Deployment version. | ||
| repeated VersionTaskQueueInfo task_queue_infos = 7; |
There was a problem hiding this comment.
are we including this feature, of having unique VersionTaskQueueInfos, for pre-release? IMO we should only add this when we complete server work related to this, right?
There was a problem hiding this comment.
@Shivs11, not sure if understand the question, don't we have TQ list per version already?
| string namespace = 1; | ||
| string deployment_name = 2; | ||
| // Must be a Versioned Build. Pass empty string to unset. | ||
| google.protobuf.StringValue version = 3; | ||
| // Required. Can be one of the following: | ||
| // - The Build ID of a Version of this Deployment that supports Versioning Behaviors (i.e. has | ||
| // `WorkflowVersioningMode==VERSIONING_BEHAVIORS`.) | ||
| // - Or, the "__unversioned__" special value, to represent all the unversioned workers (those | ||
| // with `UNVERSIONED` (or unspecified) `WorkflowVersioningMode`.) | ||
| string version = 3; |
There was a problem hiding this comment.
curious: should we pass in temporal.api.deployment.v1.WorkerDeploymentVersion like other API's related to version deployment????
There was a problem hiding this comment.
I think either this should be:
string namespace = 1;
// Required. `version.build_id` can be one of the following:
// - The Build ID of a Version of this Deployment that supports Versioning Behaviors (i.e. has
// `WorkflowVersioningMode==VERSIONING_BEHAVIORS`.)
// - Or, the "__unversioned__" special value, to represent all the unversioned workers (those
// with `UNVERSIONED` (or unspecified) `WorkflowVersioningMode`.)
temporal.api.deployment.v1.WorkerDeploymentVersion version = 2;
bytes conflict_token = 3;
...
or:
string namespace = 1;
string deployment_name = 2;
// Required. Can be one of the following:
// - The Build ID of a Version of this Deployment that supports Versioning Behaviors (i.e. has
// `WorkflowVersioningMode==VERSIONING_BEHAVIORS`.)
// - Or, the "__unversioned__" special value, to represent all the unversioned workers (those
// with `UNVERSIONED` (or unspecified) `WorkflowVersioningMode`.)
string build_id = 3;
bytes conflict_token = 4;
...
There was a problem hiding this comment.
I'll just go with build_id for now since I think it makes sense and it's the smaller change
There was a problem hiding this comment.
build_id sounds good. I hesitate to use WorkerDeploymentVersion message with its build ID being "unversioned" because that is no longer representing a single Worker Deployment Version.
There was a problem hiding this comment.
My thinking is that these APIs are split into:
- APIs that act on the Worker Deployment
- DescribeWorkerDeployment
- ListWorkerDeployments
- SetWorkerDeploymentCurrentVersion
- SetWorkerDeploymentRampingVersion
- APIs that act on the Worker Deployment Version
- DescribeWorkerDeploymentVersion
and right now the APIs that act on the Worker Deployment take in a string deployment_name and a string build_id if necessary. The APIs that act on the WorkerDeploymentVersion take in a WorkerDeploymentVersion version.
cretz
left a comment
cretz
left a comment
There was a problem hiding this comment.
Approving under the assumption that this is not making backwards incompatible alterations with main.
| string namespace = 1; | ||
| string deployment_name = 2; | ||
| // Must be a Versioned Build. Pass empty string to unset. | ||
| google.protobuf.StringValue version = 3; | ||
| // Required. Can be one of the following: | ||
| // - The Build ID of a Version of this Deployment that supports Versioning Behaviors (i.e. has | ||
| // `WorkflowVersioningMode==VERSIONING_BEHAVIORS`.) | ||
| // - Or, the "__unversioned__" special value, to represent all the unversioned workers (those | ||
| // with `UNVERSIONED` (or unspecified) `WorkflowVersioningMode`.) | ||
| string version = 3; |
There was a problem hiding this comment.
I think either this should be:
string namespace = 1;
// Required. `version.build_id` can be one of the following:
// - The Build ID of a Version of this Deployment that supports Versioning Behaviors (i.e. has
// `WorkflowVersioningMode==VERSIONING_BEHAVIORS`.)
// - Or, the "__unversioned__" special value, to represent all the unversioned workers (those
// with `UNVERSIONED` (or unspecified) `WorkflowVersioningMode`.)
temporal.api.deployment.v1.WorkerDeploymentVersion version = 2;
bytes conflict_token = 3;
...
or:
string namespace = 1;
string deployment_name = 2;
// Required. Can be one of the following:
// - The Build ID of a Version of this Deployment that supports Versioning Behaviors (i.e. has
// `WorkflowVersioningMode==VERSIONING_BEHAVIORS`.)
// - Or, the "__unversioned__" special value, to represent all the unversioned workers (those
// with `UNVERSIONED` (or unspecified) `WorkflowVersioningMode`.)
string build_id = 3;
bytes conflict_token = 4;
...
| string previous_version = 2; | ||
| // The ramping version percentage before executing this operation. |
There was a problem hiding this comment.
Do you want to call this previous_build_id ?
There was a problem hiding this comment.
yes! fixed for SetCurrent resp and SetRamping resp
5 participants
READ BEFORE MERGING: All PRs require approval by both Server AND SDK teams before merging! This is why the number of required approvals is "2" and not "1"--two reviewers from the same team is NOT sufficient. If your PR is not approved by someone in BOTH teams, it may be summarily reverted.
What changed?
versionstring fields inWorkerDeploymentOptionsandWorkerDeploymentVersionchanged tobuild_id.WorkerDeploymentVersionInfoandWorkerDeploymentInfomessages completed with all the fields that will be implemented before Replay.conflict_tokento relevant APIs.__unversioned__value instead of empty string to represent unversioned workers.Note: the following APIs are still to be added:
UpdateWorkerDeploymentVersionMetadataDeleteWorkerDeploymentVersionDeleteWorkerDeploymentWhy?
Refinements based on latest design discussions within the crew.
Breaking changes
Server PR