Worker Deployment Versioning #1679

Sushisource · 2025-04-15T23:58:36Z

What was changed

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

Why?

All aboard the versioning train

Checklist

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

THardy98

Didn't really look through the core bridge stuff, maybe worth coordinating on #1638.

For your workflow test files deployment-versioning-no-annotations... there's a workflows directory in test/src that might make more sense for them to be located, instead of directly in test/src.

Just want to re-affirm how this works:

on the worker, we have the worker's configured deployment version, and the default versioning behaviour for all its workflows
on the workflow, we can override this versioning behaviour with the newly added workflow definition options (it's a little funky, we're making use of JS's quirk that a function is an object, so a workflow with definition options is a function with properties, as already discussed)
workflow versioning behaviour is read into the activator (which uses it solely when concluding the activation) and workflow info

Tests were helpful, though I think you probably want someone whose more familiar with the versioning behaviour to take a look

packages/common/src/worker-deployments.ts

packages/common/src/workflow-definition-options.ts

packages/common/src/workflow-options.ts

packages/test/src/test-worker-deployment-versioning.ts

packages/worker/src/utils.ts

packages/worker/src/worker-options.ts

packages/test/src/test-worker-deployment-versioning.ts

THardy98 · 2025-04-16T17:33:45Z

packages/test/src/test-worker-deployment-versioning.ts

+
+const test = makeTestFunction({ workflowsPath: __filename });
+
+test('Worker deployment based versioning', async (t) => {


IIUC, there isn't any actual upgrading going on in this test? This test is checking that workflows with different versioning behaviours can run on workers with different deployment options?

It definitely does. The wf1 workflow starts on v1 and ends on v3

packages/workflow/src/internals.ts

packages/workflow/src/interfaces.ts

Quinn-With-Two-Ns · 2025-04-17T20:42:04Z

packages/workflow/src/internals.ts

@@ -416,6 +418,8 @@ export class Activator implements ActivationHandler {

  public readonly registeredActivityNames: Set<string>;

+  public versioningBehavior?: VersioningBehavior;


How do dynamic workflows pick their versioning behaviour per type?

They can do it like is shown here: https://github.com/temporalio/sdk-typescript/pull/1679/files#diff-5ae1c5b6523b9493d1f852e9d303621ffdc8f817f1b80f38f572008572aec6feR14

In Python Chad didn't want to have a dynamic getter like we added in Java unless someone asks, which I can get behind, so I figured we'd do the same here.

Not sure it makes sense to cut features from some SDKs, but not other

I think the getter is a touch error prone. We could remove it from Java too, if no one ends up needing it in these other places. I didn't want to hold up being able to finish the Core ones over that, given these can get changed later anyway. Chad is who really cared: temporalio/sdk-python#821 (comment)

I think the getter is a basic requirement since versioning a per workflow type option per the design doc. Dynamic workflows implement multiple workflow types. I am just not sure why other SDKs users have to have reduced functionality.

I'm not the one who didn't want to add it 🤷‍♂️ . I agree with the principle. In practice, I suspect most people doing dynamic workflows would be fine with one behavior. It's hard to say. I'm fine either way, I think Chad is who you have to convince.

Has the getter now. Here, it's allowed to apply to non-dynamic workflows because it's simply not possible to tell if a workflow is dynamic or not (well, at least not without some wild stuff in the bundler we don't want to do).

But, it's also less consequential since versioning behavior is literally the only thing that can be set at all, since even the static definition options are brand new.

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

packages/workflow/src/workflow.ts

+ * @returns The same passed in workflow function, with the specified options applied. You can export
+ * this function to make it available as a workflow function.
+ */
+export function defineWorkflowWithOptions<A extends any[], RT>(


Was it decided that this is only allowed for dynamic workflows and not everyone? May need to change the name and make sure it's only called (and allowed to be implemented) for dynamic workflows.

See my other comment, but, that's just not really possible in TS. Not without some crazy contortions. It's also less important here, though. We could fail at execution time but that feels punitive for sort of no reason

Interesting. So I see where you check isWorkflowFunctionWithOptions on worker init. Can you not check there if that is true and the workflow in question is a dynamic workflow and if not throw? I admit I am not familiar with this repo, but if, say I wanted to know "does a worker have a dynamic workflow registered", is that not possible? If not, how are we preventing people from creating two dynamic workflows on a worker?

It's naturally prevented because there can only be one top-level default export anyway, so the language does that for us basically

I'm hesitant regarding the name and signature of this function.

defineWorkflowWithOptions make it sounds like this is defining a Workflow function, which it is not. It is defining options on a workflow function, but it is the fact that you export the function from the module that actually defines it. One could notably assume incorrectly that they can do this frmo their workflow.ts:

// No `export` here defineWorkflowWithOptions(async function myWorkflow () { ... }, { ... })

The fact that this function returns the wrapped workflow function makes it looks like you can do:

export const myWorkflow = defineWorkflowWithOptions(async () => { // ... })

which would work on the workflow side, but not on the client side, as the impl function won't have a name property in this case. Or it may even carry an invalid name if the user instead do something like export const myWorkflow = defineWorkflowWithOptions(async functino differentName() { ... }, resulting in a worker error trying to resolve the workflow type, or even worst, makes it use the wrong workflow function if there's a wokrlfow function of that name.

I understand you did this because of the default workflow handler case, and I would definitely prefer to have a single wrapper function for this, but I'm affraid that using distinct wrappers for normal workflow functions vs default workflow handler is the "safest" API, unfortunately.

So I would instead suggest this for the common case:

setWorkflowOptions(workflowFunction, { ... }); // No return value

I'm testing an alternative for the default handler, brb.

Ah, yeah, the name thing caused me issues earlier while working on it. I considered trying some way to force that they pass a name that matches the binding, but it's impossible to enforce.

I'm OK with two top-level functions too, I think it's reasonable.

Both of these works, without the need for an extra function.

export default async function (): Promise<string> { return 'dynamic'; } setWorkflowOptions({ versioningBehavior: 'PINNED' }, module.exports.default);

and

const _default = async function (): Promise<string> { return 'dynamic'; } setWorkflowOptions({ versioningBehavior: 'PINNED' }, _default); export default _default;

Ok, cool, I'll recommend the first one in the docstring

mjameswh · 2025-04-28T14:40:46Z

packages/test/src/test-worker-deployment-versioning.ts

+  conflictToken: Uint8Array,
+  version: WorkerDeploymentVersion
+) {
+  return await client.workflowService.setWorkerDeploymentCurrentVersion({


Note that we'll need high level Client API support for these before the feature moves to GA. It's totally fine waiting for later PR.

I think the high level API should hide the conflict token, e.g. they would pass in a lambda function; the client would do a loop of (describe, call user function, setWorkerDeplCurVersion) until the update passes.

Yeah, that's how it will work, but yes saved for later

mjameswh · 2025-04-28T15:07:47Z

packages/workflow/src/workflow.ts

+): WorkflowFunctionWithOptions<A, RT> {
+  const wrappedFn = Object.assign(fn, {
+    options,
+    __temporal_is_workflow_function_with_options: true as const,


Why do you need this marker? Can't you simply look for options? Or if this is to prevent conflicting these options with something else that would happen to be called the same, you can instead change the name of the options property to something more unique.

Ok, I suppose I can. It just feels slightly safer to have something like this but I suppose in this case it's not like there will be other collisions

If we're really worried about collision, then using a symbol would be preferable. But I honestly don't think that's justified in this specific case. In fact, I know that many users do similar things for various reasons, so it is very likely that one may want to merge that functionality with their own wrapping logic.

mjameswh · 2025-04-28T15:40:09Z

packages/workflow/src/worker-interface.ts

@@ -79,7 +79,21 @@ export function initRuntime(options: WorkflowCreateOptionsInternal): void {
    const workflowFn = mod[activator.info.workflowType];
    const defaultWorkflowFn = mod['default'];

-    if (typeof workflowFn === 'function') {
+    if (isWorkflowFunctionWithOptions(workflowFn)) {


We're loosing some important type checking here, and making the "default export with options" win over "exact named function without options", which seems undesirable.

Keep the (typeof workflowFn === 'function') and (typeof defaultWorkflowFn === 'function') at the top level, and move the check for options inside those top level branches.

Also, in the else branch of workflowFn.options, make sure that it is indeed a function before assigning it to workflowDefinitionOptionsGetter.

packages/worker/src/utils.ts

packages/test/src/helpers.ts

packages/common/src/worker-deployments.ts

packages/workflow/src/workflow.ts

Sushisource · 2025-04-28T18:20:05Z

This will need to hang out for a bit since I want temporalio/api#579 in before I incorporate that

Sushisource requested a review from a team as a code owner April 15, 2025 23:58

THardy98 reviewed Apr 16, 2025

View reviewed changes

mjameswh reviewed Apr 17, 2025

View reviewed changes

packages/workflow/src/internals.ts Outdated Show resolved Hide resolved

mjameswh reviewed Apr 17, 2025

View reviewed changes

packages/workflow/src/interfaces.ts Outdated Show resolved Hide resolved

Quinn-With-Two-Ns reviewed Apr 17, 2025

View reviewed changes

Sushisource force-pushed the worker-deployment-versioning branch 4 times, most recently from a120422 to e59e2d0 Compare April 18, 2025 23:33

Sushisource mentioned this pull request Apr 23, 2025

Expose Poller Automation #1691

Closed

Sushisource force-pushed the worker-deployment-versioning branch from a8d4a69 to f18b1ca Compare April 24, 2025 00:00

cretz reviewed Apr 24, 2025

View reviewed changes

mjameswh reviewed Apr 28, 2025

View reviewed changes

packages/worker/src/utils.ts Outdated Show resolved Hide resolved

mjameswh reviewed Apr 28, 2025

View reviewed changes

packages/test/src/helpers.ts Outdated Show resolved Hide resolved

mjameswh reviewed Apr 28, 2025

View reviewed changes

packages/common/src/worker-deployments.ts Outdated Show resolved Hide resolved

mjameswh reviewed Apr 28, 2025

View reviewed changes

packages/workflow/src/workflow.ts Show resolved Hide resolved

Sushisource force-pushed the worker-deployment-versioning branch from e7f4dd6 to 218743e Compare May 1, 2025 17:26

Sushisource added 9 commits May 1, 2025 18:06

Update core

e6ce9c3

Upgrade core & make compile

626080c

First test / options passthrough

3475815

Need a way to run against extant server

c795d56

Versioning behavior passed through properly / definition changes

fa16370

Dynamic workflow support

26be2d5

Address todos / add missed new file

80f179d

Fix review comments / lints

882e9ee

Use more standard enum representation / fix related errors

9b50e54

Sushisource added 4 commits May 1, 2025 18:06

Test fixes

98282fc

Options getter function

efbf547

Review fixes

37bcf27

Versioning override

8169797

Sushisource force-pushed the worker-deployment-versioning branch from 218743e to 8169797 Compare May 2, 2025 01:06

mjameswh mentioned this pull request May 5, 2025

feat(worker): Expose Poller Automation #1704

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Worker Deployment Versioning #1679

Worker Deployment Versioning #1679

Sushisource commented Apr 15, 2025

THardy98 left a comment

THardy98 Apr 16, 2025 •

edited

Loading

Sushisource Apr 17, 2025 •

edited

Loading

Quinn-With-Two-Ns Apr 17, 2025

Sushisource Apr 17, 2025

Quinn-With-Two-Ns Apr 17, 2025

Sushisource Apr 17, 2025

Quinn-With-Two-Ns Apr 17, 2025 •

edited

Loading

Sushisource Apr 17, 2025

Sushisource Apr 23, 2025 •

edited

Loading

cretz Apr 24, 2025

Sushisource Apr 24, 2025 •

edited

Loading

cretz Apr 24, 2025 •

edited

Loading

Sushisource Apr 25, 2025

mjameswh Apr 28, 2025

Sushisource Apr 28, 2025

mjameswh Apr 28, 2025 •

edited

Loading

Sushisource Apr 28, 2025

mjameswh Apr 28, 2025

Sushisource Apr 28, 2025

mjameswh Apr 28, 2025

Sushisource Apr 28, 2025

mjameswh Apr 28, 2025

mjameswh Apr 28, 2025

Sushisource commented Apr 28, 2025


		const test = makeTestFunction({ workflowsPath: __filename });

		test('Worker deployment based versioning', async (t) => {

		@@ -416,6 +418,8 @@ export class Activator implements ActivationHandler {

		public readonly registeredActivityNames: Set<string>;

		public versioningBehavior?: VersioningBehavior;

Worker Deployment Versioning #1679

Are you sure you want to change the base?

Worker Deployment Versioning #1679

Conversation

Sushisource commented Apr 15, 2025

What was changed

Why?

Checklist

THardy98 left a comment

Choose a reason for hiding this comment

THardy98 Apr 16, 2025 • edited Loading

Choose a reason for hiding this comment

Sushisource Apr 17, 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

Quinn-With-Two-Ns Apr 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sushisource Apr 23, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sushisource Apr 24, 2025 • edited Loading

Choose a reason for hiding this comment

cretz Apr 24, 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

mjameswh Apr 28, 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

Sushisource commented Apr 28, 2025

THardy98 Apr 16, 2025 •

edited

Loading

Sushisource Apr 17, 2025 •

edited

Loading

Quinn-With-Two-Ns Apr 17, 2025 •

edited

Loading

Sushisource Apr 23, 2025 •

edited

Loading

Sushisource Apr 24, 2025 •

edited

Loading

cretz Apr 24, 2025 •

edited

Loading

mjameswh Apr 28, 2025 •

edited

Loading