temporalio / sdk-python

Temporal Python SDK
MIT License
473 stars 77 forks source link

Honor all non-completion commands #569

Closed dandavison closed 3 months ago

dandavison commented 5 months ago

Fixes https://github.com/temporalio/sdk-python/issues/528

With this change we honor all non-completion commands emitted by workflow coroutines, even if they come after a completion command (i.e. complete/CAN/cancel/fail). A consequence is that when an update completion is returned in the same WFT response as a workflow completion, the client will always get the update response; previously that was only the case if the update handler returned prior to any completion command being emitted by another coroutine.

The solution involves devolving responsibilites for this logic to core: see explanatory code comments in https://github.com/temporalio/sdk-core/pull/776.

Evidence that this is correct

$ pytest -k "completion" tests/worker/test_workflow.py

# These two fail on main
tests/worker/test_workflow.py::test_update_completion_is_honored_when_after_workflow_return_1 PASSED
tests/worker/test_workflow.py::test_update_completion_is_honored_when_after_workflow_return_2 PASSED

# These two check that we respect ordering of completion commands. They pass on main as well as with the new changes.
tests/worker/test_workflow.py::test_first_of_two_signal_completion_commands_is_honored PASSED                                                                                                                
tests/worker/test_workflow.py::test_workflow_return_is_honored_when_it_precedes_signal_completion_command PASSED