Versioning 3.1 APIs #540

carlydf · 2025-01-30T21:32:05Z

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?

Renamed DeploymentSeries -> WorkerDeployment and Deployment -> WorkerDeploymentVersion.
Added the following experimental APIs:
- ListWorkerDeployments
- DescribeWorkerDeployment
- DescribeWorkerDeploymentVersion
- SetWorkerDeploymentCurrentVersion
- SetWorkerDeploymentRampingVersion
- UpdateWorkerVersionMetadata
- DeleteWorkerDeployment
- DeleteWorkerDeploymentVersion
Deprecated the following experimental APIs:
- DescribeDeployment
- ListDeployments
- GetDeploymentReachability
- GetCurrentDeployment
- SetCurrentDeployment

Why?
These are changes and improvements we made based on user feedback from the previous iteration.

Breaking changes
No breaking changes, the old APIs still work until cleaned up at a later point with all other old Versioning APIs.

Server PR
temporalio/temporal#7233

…PI's (#515) _**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?** - Updated protos to reflect changes based on the recent conclusion regarding naming conventions for versioning-3 - `Deployment` -> `WorkerDeploymentVersion` - Added comments to remove pre-release versioning-3 code. Note: This API does not update pre-release versioning-3 changes related to dataplane. That currently is being worked in separate [PR](#514). The aim of this PR is to update those protos which are required by the deployment entity workflows so that work on those can commence there ASAP. Additionally, the updates to the proto are not finalized and will bring in more additions. Right now, they contain the bare minimum fields/structs required to make the deployment entity workflows run. With that being said, care has been taken to not add fields here and delete them in the near future. In other words, future changes will only bring in additions to these new protos.  **Why?** - Versioning-3  **Breaking changes**  **Server PR** - The Server PR is currently being built. Note, the server PR will only contain renamings (to agree with the namings of this PR) so I don't think that should serve as a blocker for reviewing this. --------- Co-authored-by: Shahab Tajik <[email protected]>

_**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?** The following concepts in Worker Versioning V3 is being renamed. This PR adds the data plane fields with new names so SDK can populate them like the old ones. `DeploymentSeries` -> `WorkerDeployment` `Deployment` -> `WorkerDeploymentVersion` Also `WorkerVersionCapabilities` and its `use_versioning` boolean are replaced with `WorkerDeploymentOptions` and `WorkflowVersioningMode` enum to be more compatible with V3 changes and also extensible in the future. `RespondActivityTask*Request` used to send `deployment` but that is now deprecated. Since Server does not have any anticipated use for these values, I did not add a replacement for them at the moment.  **Why?** See ^^  **Breaking changes** Not breaking, some things are deprecated that will be cleaned up at a later time with other old Versioning stuff.  **Server PR** Server PR will follow soon. But these changes should not break server compile because methods are not touched. --------- Co-authored-by: Carly de Frondeville <[email protected]>

_**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?** ^ title  **Why?** - versioning-3.1  **Breaking changes** - No, merging to feature.  **Server PR** - feature so not relevant

_**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?** - Proto changes to add `DescribeWorkerDeployment`  **Why?** - versioning-3.1  **Breaking changes** - no, going into feature branch  **Server PR** - Server PR in development.

…beTaskQueue (#524) _**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?** - Title  **Why?** - Versioning-3.1  **Breaking changes**  **Server PR** - soon to come (developing `SetWorkerDeploymentCurrentVersion` and Ramping functionality :) ) --------- Co-authored-by: ShahabT <[email protected]>

#527) _**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?** - Pretty much the title  **Why?** - versioning-3.1  **Breaking changes**  **Server PR** - developing

_**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?** - title  **Why?** - versioning-3.1  **Breaking changes**  **Server PR** - developing :)

@latest

…patible with latest protoc-gen packages (#531) _**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._  pull api/cmd/protogen@master instead of @latest while latest is incompatible with latest protoc-gen packages  To fix the panic error caused by installing @latest of all packages in `make grpc-install` ``` panic: interface conversion: ast.Expr is *ast.CallExpr, not *ast.CompositeLit goroutine 1 [running]: main.(*disableUtf8Validation).visit(0x0?, {0x1044665a8?, 0x1400039a480?}) /Users/cdf/go/pkg/mod/go.temporal.io/[email protected]/cmd/protogen/disable_utf8.go:90 +0x484 go/ast.inspector.Visit(0x140003a20b0, {0x1044665a8?, 0x1400039a480?}) ``` Another option would be to figure out which of these other versions caused the problem and pin that package to an older version that is compatible with v1.43.2 of our api-go repo, but that requires a lot more work to narrow down the right combination of versions. If we want to pin to an old version instead, it would need to be an older version of one of these packages: ``` Install/update protoc and plugins... go: downloading google.golang.org/protobuf v1.36.4 go: downloading google.golang.org/grpc v1.70.0 go: downloading github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.0 go: downloading google.golang.org/grpc v1.69.4 go: downloading google.golang.org/protobuf v1.36.3 go: downloading google.golang.org/genproto/googleapis/api v0.0.0-20250115164207-1a7da9e5054f go: downloading google.golang.org/genproto/googleapis/rpc v0.0.0-20250115164207-1a7da9e5054f go: downloading github.com/mikefarah/yq/v4 v4.45.1 go: downloading github.com/goccy/go-json v0.10.4 go: downloading github.com/elliotchance/orderedmap v1.7.1 go: downloading github.com/goccy/go-yaml v1.13.3 go: downloading github.com/magiconair/properties v1.8.9 go: downloading golang.org/x/net v0.33.0 ```  **Breaking changes**  **Server PR**

_**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._  Add DrainageStatus  **Why?**  **Breaking changes**  **Server PR**

_**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?** - `version` string fields in `WorkerDeploymentOptions` and `WorkerDeploymentVersion` changed to `build_id`. - `WorkerDeploymentVersionInfo` and `WorkerDeploymentInfo` messages completed with all the fields that will be implemented before Replay. - Added `conflict_token` to relevant APIs. - Now using special `__unversioned__` value instead of empty string to represent unversioned workers. Note: the following APIs are still to be added: - `UpdateWorkerDeploymentVersionMetadata` - `DeleteWorkerDeploymentVersion` - `DeleteWorkerDeployment`  **Why?** Refinements based on latest design discussions within the crew.  **Breaking changes**  **Server PR** --------- Co-authored-by: Carly de Frondeville <[email protected]>

_**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?** - Add API methods for two Delete API's to be used in versioning-3.1  **Why?**  **Breaking changes**  **Server PR**

_**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?**  **Why?**  **Breaking changes**  **Server PR**

…g_task_queues (#541) _**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?** - Adding new flag inside SetWorkerDeploymentCurrentVersionRequest for poller verification  **Why?**  **Breaking changes**  **Server PR**

_**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?** - Similar to #541 in that this flag should also be added to the ramping request  **Why?**  **Breaking changes**  **Server PR**

_**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?** - UpdateMetadata API has been added  **Why?** - versioning-3.1  **Breaking changes**  **Server PR**

…ting APIs

…ting APIs (#544) _**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._  Switch `string build_id` to `string version` in SetCurrent and SetRamp. Explain that in RoutingInfo the `current_version` and `ramping_version` strings will be the fully-qualified version string instead of just build id. btw, I'm assuming that when we add BYOK, we will simply add to `WorkerDeploymentOptions` and `WorkerDeploymentVersion`, so I'm not renaming build id there. Right now they are: ``` // Identifies a Worker Deployment Version. The combination of `deployment_name` and `build_id` // serve as the identifier. message WorkerDeploymentVersion { // The name of the Deployment this version belongs too. string deployment_name = 1; // Build ID uniquely identifies the Deployment Version within a Deployment, but the same Build // ID can be used in multiple Deployments. string build_id = 2; } // Worker Deployment options set in SDK that need to be sent to server in every poll. message WorkerDeploymentOptions { // Required. Worker Deployment name. string deployment_name = 1; // Required. Build ID of the worker. `deployment_name` and `build_id` together identify this // Worker Deployment Version. string build_id = 2; // Required. Workflow versioning mode for this Deployment Version. Must be the same for all // pollers in a given Deployment Version, across all Task Queues. temporal.api.enums.v1.WorkflowVersioningMode workflow_versioning_mode = 3; } ``` Not trying to design this now, just trying to validate that the current structure is BYOK-compatible -- I'm imagining that in the future they could look something like: ``` + // Identifies a Worker Deployment Version, which is identified by either: + // - The combination of `deployment_name` and `build_id`. + // - If present, `id`. message WorkerDeploymentVersion { // The name of the Deployment this version belongs too. string deployment_name = 1; // Build ID uniquely identifies the Deployment Version within a Deployment, but the same Build // ID can be used in multiple Deployments. string build_id = 2; + // Uniquely identifies the Deployment Version across a Namespace. + string id = 3; } // Worker Deployment options set in SDK that need to be sent to server in every poll. message WorkerDeploymentOptions { + // Required unless `version_id` is present. Worker Deployment name. string deployment_name = 1; + // Required unless `version_id` is present. Build ID of the worker. `deployment_name` and + // `build_id` together identify this Worker Deployment Version. string build_id = 2; // Required. Workflow versioning mode for this Deployment Version. Must be the same for all // pollers in a given Deployment Version, across all Task Queues. temporal.api.enums.v1.WorkflowVersioningMode workflow_versioning_mode = 3; + // Required only if `deployment_name` or `build_id` are not present. + // Uniquely identifies this Worker Deployment Version in the namespace. + string version_id 4; } ```  **Why?**  **Breaking changes**  **Server PR**

_**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?** - Using string type in all the places for `version` fields. Removed `WorkerDeploymentVersion` from the public API. This is for two reasons: - String type gives ability to specify special values of `__unversioned__` (and maybe later `<deployment_name>/__unversioned__`. - We might add ability for users to pass custom version IDs in with case the struct would be hard for users to use (they'd need to write the logic to convert the struct to an ID based on whether it has a custom ID or not). - Using `WorkerDeploymentOptions` in the task response APIs instead of `WorkerDeploymentVersion` so that server has the full info regardless of the worker participating in versioning or not. - Added `worker_deployment` field to execution info (top level) so both versioned and unversioned workflows can have this info (and its corresponding SA). - Renamed `WorkflowVersioningMode` to `WorkerVersioningMode` and its `VERSIONING_BEHAVIORS` enum to `VERSIONED`, for the following reasons: - This enum is more about the worker than the workflows, whether or not workflows are versioned or not depends on whether or not the worker enables versioning. - We tried to not use the terms "versioned" and "unversioned" for workers for a while but the attempt did work and we ended up even creating special `__unversioned__` value for them. Hence, it'd be very confusing if they don't see some config such as `WorkerVersioningMode` that directly specifies the worker as versioned or unversioned. The previous name, while doing the same thing, twisted the naming and I believe it'll confuse the users. - Deployment Versions are not created for the unversioned mode, at least not so far. This naming and updated comments reflects that better. - WorkflowVersioningMode is easy to be confused with Workflow VersioningBehavior.  **Why?**  **Breaking changes**  **Server PR** --------- Co-authored-by: Carly de Frondeville <[email protected]>

antlai-temporal · 2025-02-03T23:26:22Z

temporal/api/deployment/v1/message.proto

+    // The fully-qualified string representation of the version, in the form "<deployment_name>/<build_id>".
+    string version = 1;


Using / in comments, do we change this before merge?

antlai-temporal · 2025-02-03T23:28:47Z

temporal/api/deployment/v1/message.proto

+    google.protobuf.Timestamp ramping_since_time = 6;
+


@carlydf did you want to add percentange_since_time or not anymore?

So in this case the timestamps are a bit different from RoutingInfo, because these timestamps are in the context of a single Version, so when this version stops being current, the current_since_time will be set to nil and the routing_update_time will be set to now. Then we have current_since_time == nil equivalent to is_current == false (current_since_time captures the info that could have been held in a separate is_current field).

Because of that, routing_changed_time cannot simply be reconstructed by taking the max of the timestamps.

The main benefit of all this is to not have to include the booleans is_ramping and is_current, so maybe the confusion is not worth it, and I should add those two booleans so that the timestamps can all be x_changed_time instead of x_since_time... open to that if folks feel like having different semantics for the "since" timestamps vs "changed" timestamps is confusing.

RoutingConfig (on the Deployment as a whole) has the ramping and current versions for the Deployment, + these timestamps:

// Last time current version was changed. google.protobuf.Timestamp current_version_changed_time = 4; // Last time ramping version was changed. Not updated if only the ramp percentage changes. google.protobuf.Timestamp ramping_version_changed_time = 5; // Last time ramping version percentage was changed. // If ramping version is changed, this is also updated, even if the percentage stays the same. google.protobuf.Timestamp ramping_version_percentage_changed_time = 6;

antlai-temporal · 2025-02-03T23:49:52Z

temporal/api/workflowservice/v1/request_response.proto

    temporal.api.common.v1.WorkerVersionCapabilities worker_version_capabilities = 5;
+    // Worker deployment options that user has set in the worker.


do you want to mark this with [deprecated = true]

antlai-temporal · 2025-02-03T23:54:04Z

temporal/api/workflowservice/v1/request_response.proto

    temporal.api.common.v1.WorkerVersionCapabilities worker_version_capabilities = 5;
+    // Worker deployment options that user has set in the worker.


can we add [deprecated = true]

cretz

LGTM since there are no backwards incompatible changes.

I assume we've done the due diligence and feel these are fairly stable APIs? Or if not, would we consider continuing to develop them outside of master (same for the server and SDK code, even releasing separate versions for trying out if needed).

Sushisource · 2025-02-05T17:43:47Z

temporal/api/deployment/v1/message.proto

+    //   with `UNVERSIONED` (or unspecified) `WorkerVersioningMode`.)
+    // Note: Current Version is overridden by the Ramping Version for a portion of traffic when a ramp
+    // is set (see `ramping_version`.)
+    string current_version = 1;


Stylistically I'd make this (and below) a oneof of version | unversioned where unversioned is a bool / empty, which saves clients needing to understand the special string.

But, that's not a blocker or a big deal.

temporal/api/workflowservice/v1/request_response.proto

_**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?** Child workflows start in parent's pinned Worker Deployment Version.  **Why?** So user do not have to worry about interface compatibility between pinned parents and children.  **Breaking changes**  **Server PR**

addressed comments

Shivs11 and others added 15 commits January 16, 2025 13:18

Move WorkerDeploymentOptions to deployments package

737d468

Add conflict token to DeleteVersion, add percentage update time

74b203f

Merge branch 'master' of github.com:temporalio/api into versioning-3.1

2e36488

rename routing_update_time -> routing_changed_time

3c89dab

carlydf requested review from a team as code owners January 30, 2025 21:32

carlydf marked this pull request as draft January 30, 2025 21:32

carlydf and others added 9 commits January 30, 2025 13:35

use fully-qualified version string (deployment_name/build_id) for rou…

3261978

…ting APIs

remove conflictToken from DeleteWorkerDeploymentVersion

dadfad0

reorder WorkerDeploymentVersion proto fields and fix punctuation

56c7158

add todo comment about WorkflowExecutionVersioningInfo.version

00f230a

carlydf force-pushed the versioning-3.1 branch from 89fc4af to 00f230a Compare February 2, 2025 07:04

ShahabT and others added 2 commits February 3, 2025 12:56

Merge remote-tracking branch 'origin/master' into versioning-3.1

0b2a762

prepare for merge

39f4a0a

ShahabT marked this pull request as ready for review February 3, 2025 22:53

antlai-temporal reviewed Feb 3, 2025

View reviewed changes

antlai-temporal approved these changes Feb 4, 2025

View reviewed changes

rename UpdateWorkerDeploymentVersionMetadata

1b9110b

cretz approved these changes Feb 4, 2025

View reviewed changes

ShahabT added 2 commits February 4, 2025 11:24

Change deployment version delimiter to .

927f457

Merge remote-tracking branch 'origin/master' into versioning-3.1

1546427

Sushisource approved these changes Feb 5, 2025

View reviewed changes

ShahabT added 2 commits February 5, 2025 14:00

Apply suggestions from code review

b1b8fe2

addressed comments

ShahabT force-pushed the versioning-3.1 branch from b9473fd to b1b8fe2 Compare February 5, 2025 23:05

Merge remote-tracking branch 'origin/master' into versioning-3.1

a761e56

ShahabT merged commit 475af1c into master Feb 6, 2025
4 checks passed

ShahabT deleted the versioning-3.1 branch February 6, 2025 23:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Versioning 3.1 APIs #540

Versioning 3.1 APIs #540

carlydf commented Jan 30, 2025 •

edited by ShahabT

Loading

antlai-temporal Feb 3, 2025

antlai-temporal Feb 3, 2025

carlydf Feb 4, 2025

carlydf Feb 4, 2025

antlai-temporal Feb 3, 2025

antlai-temporal Feb 3, 2025

cretz left a comment

Sushisource Feb 5, 2025

		// The fully-qualified string representation of the version, in the form "<deployment_name>/<build_id>".
		string version = 1;

		temporal.api.common.v1.WorkerVersionCapabilities worker_version_capabilities = 5;
		// Worker deployment options that user has set in the worker.

Versioning 3.1 APIs #540

Versioning 3.1 APIs #540

Conversation

carlydf commented Jan 30, 2025 • edited by ShahabT Loading

antlai-temporal Feb 3, 2025

Choose a reason for hiding this comment

antlai-temporal Feb 3, 2025

Choose a reason for hiding this comment

carlydf Feb 4, 2025

Choose a reason for hiding this comment

carlydf Feb 4, 2025

Choose a reason for hiding this comment

antlai-temporal Feb 3, 2025

Choose a reason for hiding this comment

antlai-temporal Feb 3, 2025

Choose a reason for hiding this comment

cretz left a comment

Choose a reason for hiding this comment

Sushisource Feb 5, 2025

Choose a reason for hiding this comment

carlydf commented Jan 30, 2025 •

edited by ShahabT

Loading