Skip to content

Conversation

@dandavison
Copy link
Contributor

@dandavison dandavison commented Oct 13, 2025

  • Eliminate shared base class -- it's not appropriate in the future when ActivitySerializationContext will be applied at the top level (and even in the original workflow-activity case, we moved away from nested scopes)

  • Make workflow identifiers optional in activity context

  • Add activity ID to activity context


Note

Restructures workflow/activity serialization contexts (removing shared base), makes workflow IDs optional, adds activity IDs, and updates workers/runtime/tests to construct and assert the new contexts.

  • Converter:
    • Remove BaseWorkflowSerializationContext; WorkflowSerializationContext now directly extends SerializationContext with namespace and optional workflow_id.
    • Redefine ActivitySerializationContext: add activity_id, include namespace, and make workflow_id/workflow_type optional; add field docstrings.
  • Worker/Runtime:
    • Update all context construction in worker/_activity.py and _workflow_instance.py to the new shapes and to populate activity_id.
    • Adjust heartbeat and activity start paths to pass revised context fields.
  • Tests:
    • Add activity_id to activity executions and expected context dicts; relax types (e.g., trace map key to Optional[str]).
    • Rename EventWorkflow to WaitForSignalWorkflow in async activity completion test and update signal calls accordingly.

Written by Cursor Bugbot for commit 89b721a. This will update automatically on new commits. Configure here.

@dandavison dandavison requested a review from a team as a code owner October 13, 2025 16:05
namespace: str
"""Namespace used by the worker executing the workflow."""

workflow_id: Optional[str]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be optional in WorkflowSerializationContext?



test_traces: dict[str, list[TraceItem]] = defaultdict(list)
test_traces: dict[Optional[str], list[TraceItem]] = defaultdict(list)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to this change, but a quick glance makes it seem like we might be able to move this global state into individual tests. If possible I'd prefer that vs tests calling del test_traces[] to clean up their expected entries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants