Worker Deployment Versioning #821

Sushisource · 2025-04-10T00:24:59Z

What was changed

Added the annotations and options for worker-deployment-based versioning

Why?

Feature parity choo choo 🚄

Checklist

Closes [Feature Request] Support New Worker Versioning API #793
How was this tested:
Added tests
Any docs updates needed?

Sushisource · 2025-04-10T00:29:51Z

temporalio/workflow.py

+                    # TODO: Not sure this needs to exist?
+                    # seen_run_attr = True


Wanted to mention in review since this seems sorta wrong. It sets that a run attr was seen if it was seen on the base class, but that's only allowed iff there was one on the subclass, which would mean it was already set true.

This was so that you don't have duplicate issues in the issues list. So today you'd get only @workflow.run defined on {base_member.__qualname__} but not on the override, but when not setting this, you get that and Missing @workflow.run method which is redundant.

Sushisource · 2025-04-10T00:30:48Z

temporalio/worker/_workflow_instance.py

            self._current_completion.failed.failure.SetInParent()
+            # TODO: Review - odd that we don't un-set success here?


Wanted to get an eye on this too. Seems strange we never un-set the set-by-default success variant if there was a failure. I'm kinda surprised this doesn't cause problems, and maybe it just works by a happy accident of field ordering. Maybe it's unset somewhere I missed?

In Python protobuf, a oneof is automatically unset when you set another of the oneof, ref https://protobuf.dev/reference/python/python-generated/#oneof

cretz · 2025-04-10T12:52:41Z

temporalio/common.py

+            raise ValueError(f"Unknown VersioningBehavior: {self}")
+
+
+class WorkerDeploymentVersion:


I think you should make this a dataclass (can be frozen dataclass, no prob), it gives you that init and you get equality checks, good default string representation, etc.

My worry was if we add fields later on that might cause problems with compat, but I suppose that's not a real issue

👍 We can default them or we can technically even override __init__ if we must

cretz · 2025-04-10T12:55:31Z

temporalio/common.py

@@ -1016,6 +1016,74 @@ def __post_init__(self):

 Priority.default = Priority(priority_key=None)

+
+class VersioningBehavior(Enum):


Our other enums that map to proto equivalents use IntEnum and just set the values as int values of the actual proto values (except for some experimental versioning ones that should have been done this way IMO and are going to be deleted at some point I assume). Can look at the existing IntEnums in the SDK.

cretz · 2025-04-10T12:58:50Z

temporalio/worker/_worker.py

            disable_safe_workflow_eviction: If true, instead of letting the
                workflow collect its tasks properly, the worker will simply let
                the Python garbage collector collect the tasks. WARNING: Users
                should not set this value to true. The garbage collector will
                throw ``GeneratorExit`` in coroutines causing them to wake up
                in different threads and run ``finally`` and other code in the
                wrong workflow environment.
+            deployment_options: Deployment options for the worker. Exclusive with `build_id` and


Suggested change

deployment_options: Deployment options for the worker. Exclusive with `build_id` and

deployment_config: Deployment config for the worker. Exclusive with `build_id` and

We prefer the term "config" in the Python SDK. Should also change the data type to WorkerDeploymentConfig.

cretz · 2025-04-10T13:01:59Z

temporalio/worker/_worker.py

+
+
+@dataclass
+class WorkerDeploymentOptions:


May need to mark this and all places it is used as experimental (usually as a docstring warning)

cretz · 2025-04-10T13:04:46Z

temporalio/worker/_workflow_instance.py

            self._current_completion.failed.failure.SetInParent()
+            # TODO: Review - odd that we don't un-set success here?


In Python protobuf, a oneof is automatically unset when you set another of the oneof, ref https://protobuf.dev/reference/python/python-generated/#oneof

cretz · 2025-04-10T13:06:37Z

temporalio/workflow.py

+
+
+@overload
+def run(


Unlike Java, Python uses @workflow.defn for implementation options, not @workflow.run. Python (or any other SDK) does not have the forced interface separation Java does. This option, like name, dynamic, sandboxed, failure_exception_types, etc is a whole workflow implementation option and should be set where whole workflow implementation options are set (on @workflow.defn).

cretz · 2025-04-10T13:09:12Z

temporalio/workflow.py

@@ -215,6 +255,25 @@ class UnfinishedSignalHandlersWarning(RuntimeWarning):
    """The workflow exited before all signal handlers had finished executing."""


+def dynamic_versioning_behavior(


I think this should either be available to every workflow or should not exist. There is nothing about dynamic workflows specifically that deserves special versioning treatment. Might as well allow every workflow implementation to do this. Also, we should allow dynamic workflows to set the same options as regular workflows, so dynamic workflows should be allowed to set a fixed value here on the @workflow.defn decorator instead of only allowed to set versioning this way.

Dynamic versioning behavior doesn't have to be related to dynamic workflows. We should decide if we want dynamic versioning behavior and if we do, it should be generally applicable.

Might as well isn't a strong reason here. It's a big footgun I think. It only needs to exist for dynamic workflows because the static annotation isn't sufficient here, so there is something special about them. The reason to restrict it to dynamic workflows is to mitigate the risk of that footgun.

And they are allowed to use the static version on @run-to-be-@defn. That works too.

I feel pretty strongly about that. I'm sort-of-ok with just taking it away entirely until someone asks, too, but we already added this in Java and it feels weird to not do it here.

It only needs to exist for dynamic workflows because the static annotation isn't sufficient here

Why does it need to exist for dynamic workflows? We don't have the ability to have a callback to define failure exception types.

I feel pretty strongly about that. I'm sort-of-ok with just taking it away entirely until someone asks, too, but we already added this in Java and it feels weird to not do it here.

👍 I like this option too. I don't think we should have done it in Java either. I think we should do workflow implementation options the way that the SDK prefers to do workflow implementation options and not make this workflow implementation option any more special than any other.

And they are allowed to use the static version on @run-to-be-@defn. That works too.

The code inside of _WorkflowWorker.__init__ fails if you have a dynamic workflow without this getter and without worker-level setting

I fixed that. It's allowed now. The reason you need it is that conceptually a dynamic workflow can implement multiple workflow types. You might want those different types to have different versioning behavior.

We can yeet it until someone asks though, and considering it's easy to misuse, I can just go for that.

We can yeet it until someone asks though, and considering it's easy to misuse, I can just go for that.

👍 Yes, I vote for this. I suspect when someone says they this or they want dynamic failure exception types per workflow type (or any other workflow implementation option per workflow type), we can definitely add them.

temporalio/worker/_workflow_instance.py

Sushisource · 2025-04-10T17:06:57Z

Note to self: Realized core isn't setting deployment options on activity task completions. Fix that and upgrade core in here before merge.

cretz · 2025-04-10T21:55:50Z

temporalio/worker/_worker.py

+    """
+
+    version: WorkerDeploymentVersion
+    use_worker_versioning: bool


Hrmm, I wonder if we should default this to False. I admit I am unsure what the situations are where you'd set deployment config without wanting to set this to True, but it is default false in other SDKs. Regardless, non-blocking...

I think it's nice to have people be explicit about it here. It'd generally be True but if you want to set a build ID, not use versioning, and not use the deprecated APIs that'd be the reason for False.

Hrmm, do we consider setting build_id= as deprecated if they don't want versioning? We'll always have to support the shortcut. I guess it's similar to worker tuning, which arguably there if we always have to support the simpler shortcut options we might as well not deprecate their use.

Yeah, I think the problem is that from a product perspective they want to de-emphasize build id in favor of the deployment as a whole

Sushisource · 2025-04-11T23:15:10Z

Can't merge because it looks like the new deadlock interruptor is causing a test to fail that doesn't fail when I comment it out. We may need to back out that change.

Sushisource requested a review from a team as a code owner April 10, 2025 00:24

Sushisource commented Apr 10, 2025

View reviewed changes

cretz reviewed Apr 10, 2025

View reviewed changes

cretz approved these changes Apr 10, 2025

View reviewed changes

Sushisource enabled auto-merge (squash) April 10, 2025 22:22

Sushisource force-pushed the worker-deployment-versioning branch from 6b65e36 to e5a9f82 Compare April 11, 2025 18:40

Sushisource added 11 commits April 11, 2025 11:42

Update core & protos

b43165e

Fix compile

e479c8a

Python API updates

be7d269

Test

29870b1

Dynamic workflows

12572b0

Enforce at registration time

718f9af

TODO cleanup

ec96780

Fix some nonsense auto-import

5503673

Review feedback

b9bc00e

Remove dynamic behavior getter for now

5651483

Update core again

f4f6483

Sushisource force-pushed the worker-deployment-versioning branch 2 times, most recently from 3597af6 to 1bbff88 Compare April 11, 2025 18:52

Expose deployment version on contexts

ecd1fd7

Sushisource force-pushed the worker-deployment-versioning branch from 1bbff88 to ecd1fd7 Compare April 11, 2025 21:00

Sushisource disabled auto-merge April 11, 2025 22:37

Sushisource force-pushed the worker-deployment-versioning branch 4 times, most recently from a64791c to 25ed9a4 Compare April 15, 2025 18:47

Try skipping new tests on <3.12

08c5fcc

Sushisource force-pushed the worker-deployment-versioning branch 2 times, most recently from c5e4604 to 0a81383 Compare April 15, 2025 23:37

Sushisource mentioned this pull request Apr 16, 2025

Expose poller autoscaling options #830

Merged

Sushisource force-pushed the worker-deployment-versioning branch 6 times, most recently from 4cf77fa to 3d1c940 Compare April 16, 2025 19:59

Try pytest plain asserts on 3.9

707473a

Sushisource force-pushed the worker-deployment-versioning branch from 3d1c940 to 707473a Compare April 16, 2025 21:16

Sushisource enabled auto-merge (squash) April 16, 2025 21:18

Sushisource merged commit fd49c4e into main Apr 16, 2025
14 checks passed

Sushisource deleted the worker-deployment-versioning branch April 16, 2025 22:30

Sushisource mentioned this pull request Apr 17, 2025

Worker Deployment Versioning temporalio/sdk-typescript#1679

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Worker Deployment Versioning #821

Worker Deployment Versioning #821

Sushisource commented Apr 10, 2025

Sushisource Apr 10, 2025

cretz Apr 10, 2025 •

edited

Loading

Sushisource Apr 10, 2025

cretz Apr 10, 2025

cretz Apr 10, 2025

Sushisource Apr 10, 2025

cretz Apr 10, 2025

cretz Apr 10, 2025

cretz Apr 10, 2025

cretz Apr 10, 2025

cretz Apr 10, 2025

cretz Apr 10, 2025 •

edited

Loading

cretz Apr 10, 2025

Sushisource Apr 10, 2025 •

edited

Loading

cretz Apr 10, 2025

Sushisource Apr 10, 2025 •

edited

Loading

cretz Apr 10, 2025

Sushisource Apr 10, 2025

Sushisource commented Apr 10, 2025

cretz Apr 10, 2025

Sushisource Apr 10, 2025 •

edited

Loading

cretz Apr 11, 2025 •

edited

Loading

Sushisource Apr 11, 2025

Sushisource commented Apr 11, 2025

		self._current_completion.failed.failure.SetInParent()
		# TODO: Review - odd that we don't un-set success here?

		raise ValueError(f"Unknown VersioningBehavior: {self}")


		class WorkerDeploymentVersion:

		@@ -1016,6 +1016,74 @@ def __post_init__(self):

		Priority.default = Priority(priority_key=None)


		class VersioningBehavior(Enum):

	deployment_options: Deployment options for the worker. Exclusive with `build_id` and
	deployment_config: Deployment config for the worker. Exclusive with `build_id` and

		@@ -215,6 +255,25 @@ class UnfinishedSignalHandlersWarning(RuntimeWarning):
		"""The workflow exited before all signal handlers had finished executing."""


		def dynamic_versioning_behavior(

Worker Deployment Versioning #821

Worker Deployment Versioning #821

Conversation

Sushisource commented Apr 10, 2025

What was changed

Why?

Checklist

Choose a reason for hiding this comment

cretz Apr 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cretz Apr 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sushisource Apr 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sushisource Apr 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sushisource commented Apr 10, 2025

Choose a reason for hiding this comment

Sushisource Apr 10, 2025 • edited Loading

Choose a reason for hiding this comment

cretz Apr 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sushisource commented Apr 11, 2025

cretz Apr 10, 2025 •

edited

Loading

cretz Apr 10, 2025 •

edited

Loading

Sushisource Apr 10, 2025 •

edited

Loading

Sushisource Apr 10, 2025 •

edited

Loading

Sushisource Apr 10, 2025 •

edited

Loading

cretz Apr 11, 2025 •

edited

Loading