Serialization context #1102

dandavison · 2025-09-12T22:20:12Z

Fixes #796

New with_context API for creating context-aware data converters
Make serialization context available for all serde / codec operations
Best-effort test suite: it's not feasible to provide complete coverage for this change

tconley1428 · 2025-09-15T14:53:43Z

temporalio/client.py

                timeout=input.rpc_timeout,
            )

+    def _async_activity_data_converter(


This one seems odd. Is this when the client comes back to complete an activity out of band, they don't give us a lot of the information?

tconley1428 · 2025-09-15T14:57:20Z

temporalio/worker/_workflow.py

-                workflow = _RunningWorkflow(
-                    self._create_workflow_instance(act, init_job)
-                )
+                workflow_instance, det = self._create_workflow_instance(act, init_job)


This seems potentially problematic. The act given to create workflow instance is now given prior to decoding. Maybe not a problem, but is there a reason to change it?

It doesn't currently need to be decoded at this stage but I think you're right that this can be done less intrusively: we can get the workflow ID from init_job.workflow_id. I'll make that change.

It does technically need to be decoded at this stage because info needs decoded memo and headers

cretz · 2025-09-15T14:15:59Z

temporalio/client.py

-            WorkflowExecution._from_raw_info(v, self._client.data_converter)
+            WorkflowExecution._from_raw_info(
+                v,
+                self._client.data_converter._with_context(


Do we want to create a new context-specific converter for each page here? No strong opinion.

cretz · 2025-09-15T14:16:47Z

temporalio/client.py

    ) -> None:
        """Create workflow handle."""
        self._client = client
+        self._data_converter = client.data_converter._with_context(


This class is often created by people that don't care about it (e.g. they start a workflow and don't care about the handle). Are there concerns about creating a context-specific data converter in all cases even if it's never used? I wonder if we should build the converter each call when they make the call, same as top-level client calls.

cretz · 2025-09-15T14:31:31Z

temporalio/client.py

+        return self._client.data_converter._with_context(
+            ActivitySerializationContext(
+                namespace=self._client.namespace,
+                workflow_id=(


What we did here in Java and .NET is had this async activity client/handle implement WithSerializationContext since we don't always have the workflow ID. This way, users who know some of this information can do a "with context" to get context-specific async activity client, and they can choose which fields they are ok being empty and such. The task token approach is by far the most common approach (though in general async activity completion is not that common), so I think we may need to just put this in front of users to let them set the context.

cretz · 2025-09-15T14:33:32Z

temporalio/converter.py

+    namespace: str
+    workflow_id: str


In .NET and Java we had a common interface for both workflow and activity serialization context to show they both had these two fields. No problem not doing here, just noting it if you wanted to.

cretz · 2025-09-15T14:34:36Z

temporalio/converter.py

+    during serialization and deserialization.
+    """
+
+    def with_context(self, context: Optional[SerializationContext]) -> Self:


At least in other SDKs, I am not sure there is ever expected to be a situation where this is called with None. With context always assumes it will be with a context and developers don't have to code around the absence of one.

Removed Optional here. In a previous version of the PR I was passing None to Nexus contexts but not any longer.

cretz · 2025-09-15T14:45:14Z

temporalio/worker/_activity.py

+        data_converter = self._data_converter
+        if activity.info:
+            context = temporalio.converter.ActivitySerializationContext(
+                namespace=activity.info.workflow_namespace,
+                workflow_id=activity.info.workflow_id,
+                workflow_type=activity.info.workflow_type,
+                activity_type=activity.info.activity_type,
+                activity_task_queue=self._task_queue,
+                is_local=activity.info.is_local,
+            )
+            data_converter = data_converter._with_context(context)


Is there a way to get this off the running activity instead of recreating every heartbeat? I know we store the payload converter on the activity context which is being accessed here, maybe we should store the data converter instead and just return its payload converter from activity.payload_converter()? I am unsure if this affects how multiprocessing and pickling work.

cretz · 2025-09-15T14:48:24Z

temporalio/worker/_workflow.py

-                workflow = _RunningWorkflow(
-                    self._create_workflow_instance(act, init_job)
-                )
+                workflow_instance, det = self._create_workflow_instance(act, init_job)


I am concerned with the refactoring that act (and its init_job) has not run through the codec by this point where it had before. I think the logic needs to stay using the codec before workflow instance creation code, but you need to extract the workflow ID from the init_job or the running workflow.

Agreed, we're doing that now.

cretz · 2025-09-15T14:54:53Z

temporalio/worker/_workflow.py

        act: temporalio.bridge.proto.workflow_activation.WorkflowActivation,
        init: temporalio.bridge.proto.workflow_activation.InitializeWorkflow,
-    ) -> WorkflowInstance:
+    ) -> tuple[WorkflowInstance, WorkflowInstanceDetails]:


Not sure we need to change the entire return type here just to get workflow ID. Caller just extract it out of the init job and we don't have to mutate this code at all.

Correct, the changes to this function have been reverted in connection to discussion above.

cretz · 2025-09-15T14:59:00Z

temporalio/worker/_workflow_instance.py

+        self._payload_converter_class = det.payload_converter_class
+        self._failure_converter_class = det.failure_converter_class


Not sure we need to store these. The "with context" can be called on the already-created converters, we should not re-instantiate converters more than once per instance IMO

Discussed offline and resolved in recent commits

cretz · 2025-09-15T14:59:53Z

temporalio/worker/_workflow_instance.py

+        payload_converter = self._payload_converter_class()
+        failure_converter = self._failure_converter_class()


Mentioned above, but I don't believe we need to reinstantiate converters multiple times in this instance, just call the "with context" on the already existing ones.

Discussed offline and resolved in recent commits

cursor · 2025-09-19T15:07:46Z

temporalio/worker/_workflow.py

                    )
+                workflow_id = init_job.workflow_id
+            else:
+                workflow_id = workflow.workflow_id


Bug: Workflow ID Missing in Running Workflow Instances

Accessing workflow.workflow_id on existing _RunningWorkflow instances can cause an AttributeError. This occurs because instances created with the previous constructor lack the workflow_id attribute, which is now expected during workflow activation.

…classes

dandavison mentioned this pull request Sep 12, 2025

Serialization Context - Very early WIP #1088

Closed

dandavison force-pushed the dan-9986-serialization-context branch from 56db57e to 3e53f35 Compare September 12, 2025 22:22

dandavison changed the title ~~Dan 9986 serialization context~~ Serialization context Sep 12, 2025

dandavison force-pushed the dan-9986-serialization-context branch 11 times, most recently from 8e0e193 to 4a0b661 Compare September 15, 2025 09:15

dandavison marked this pull request as ready for review September 15, 2025 09:17

dandavison requested a review from a team as a code owner September 15, 2025 09:17

dandavison force-pushed the dan-9986-serialization-context branch from 4a0b661 to 6000b0d Compare September 15, 2025 10:44

tconley1428 reviewed Sep 15, 2025

View reviewed changes

cretz reviewed Sep 15, 2025

View reviewed changes

dandavison added 4 commits September 15, 2025 21:58

Drive-by: get rid of always-true condition

9c22bec

Support serialization context for all ser/de operations

a6b3d4d

Partial revert

167476a

Refactor activation processing

a0f4c63

dandavison force-pushed the dan-9986-serialization-context branch from 6000b0d to a0f4c63 Compare September 16, 2025 03:12

Do not support None context

d3311e6

dandavison force-pushed the dan-9986-serialization-context branch from adf8bc3 to d3311e6 Compare September 19, 2025 12:17

This comment was marked as outdated.

Sign in to view

dandavison added 2 commits September 19, 2025 08:40

Never reinstantiate converters

87c9831

Stack contexts I

bb365c2

dandavison added 3 commits September 19, 2025 09:25

Reuse and rename helper

4130c80

Don't assume user overrides with same constructor signature

db99d26

Simplify

20946a8

cursor bot reviewed Sep 19, 2025

View reviewed changes

dandavison added 4 commits September 22, 2025 03:57

Refactor

7e4ccdf

Test nexus operations do not use codec

b000cf7

Fix nexus

e37540d

Failing test: child workflow codec

96cf63b

dandavison marked this pull request as draft September 22, 2025 17:48

dandavison added 9 commits September 23, 2025 06:45

DEV: Markers

cfe5ddf

Add payload encryption test

c99d32c

Test fixups

5a61f2c

DEBUG

fce0afd

DEBUG

dd808b3

Change visitor API

9021f95

Add command seq IDs to async context during payload visitor traversal

d725e21

Store data_converter on WorkflowInstanceDetails instead of converter …

ab7c36e

…classes

Command-aware codec for payload visitor

a4d3148

		self._payload_converter_class = det.payload_converter_class
		self._failure_converter_class = det.failure_converter_class

		payload_converter = self._payload_converter_class()
		failure_converter = self._failure_converter_class()

Serialization context #1102

Are you sure you want to change the base?

Serialization context #1102

Conversation

dandavison commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

cursor bot Sep 19, 2025

Choose a reason for hiding this comment

Bug: Workflow ID Missing in Running Workflow Instances

Uh oh!

Uh oh!

dandavison commented Sep 12, 2025 •

edited

Loading