Skip to content

Conversation

@timl3136
Copy link
Member

What changed?
Implement signal_with_start_workflow client method

Why?
It's a necessary feature for signaling in python client

How did you test it?
Add an integration test to test signal_with_start.

Potential risks

Release notes

Documentation Changes

timl3136 and others added 3 commits October 30, 2025 13:08
Signed-off-by: Tim Li <ltim@uber.com>
Signed-off-by: Tim Li <ltim@uber.com>
Signed-off-by: Tim Li <ltim@uber.com>
…n-client into signal_with_start

Signed-off-by: Tim Li <ltim@uber.com>
Signed-off-by: Tim Li <ltim@uber.com>
Resolved conflicts:
- Added Generic import to support Generic[C] type parameter
- Removed unused TYPE_CHECKING import and empty block
- Removed client() method from WorkflowContext (removed in upstream)
- Removed test_workflow_engine_integration.py as upstream reorganized tests
- Adopted upstream's frozen=True for WorkflowInfo
self,
workflow: Union[str, WorkflowDefinition],
signal_name: str,
signal_input: Any = None,
Copy link
Member

Choose a reason for hiding this comment

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

I think we similarly want a list here

Test cases:
1. new_workflow: SignalWithStartWorkflow starts a new workflow if it doesn't exist
2. existing_workflow: SignalWithStartWorkflow signals existing workflow without restart
Copy link
Member

Choose a reason for hiding this comment

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

While these two scenarios are testing different behaviors, they're largely testing server behaviors rather than client behaviors. In both scenarios the client sends the same data. I think just one of them is fine.

I'd expand the test to verify that the workflow history actually has the signal event and that the payload/name match what we expect, since that's demonstrating end to end that we can signal a specific workflow.

Signed-off-by: Tim Li <ltim@uber.com>
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