Add suspend and resume reasons - #250
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 941fc052-b910-42af-93fa-4f00138813ab
There was a problem hiding this comment.
Pull request overview
This PR adds suspend/resume “reason” parity with the .NET client across the core Durable Task Python clients and the Azure Functions Durable compatibility client, ensuring the optional reason is serialized and preserved.
Changes:
- Added keyword-only optional
reasonparameters to sync/asyncsuspend_orchestrationandresume_orchestrationAPIs and serialized them into the protobuf requests. - Updated Azure Functions Durable v1-compat
suspend()/resume()deprecated aliases to forward the provided reason. - Added unit tests and changelog entries covering both populated and omitted reasons.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| durabletask/client.py | Adds optional keyword-only reason to suspend/resume APIs and serializes it into SuspendRequest/ResumeRequest. |
| tests/durabletask/test_client.py | Adds tests validating reason serialization and omission behavior for sync and async clients. |
| tests/azure-functions-durable/test_client_compat.py | Updates compat tests to ensure deprecated v1 methods forward reason to the new APIs. |
| azure-functions-durable/azure/durable_functions/client.py | Forwards v1 deprecated reason argument to durabletask suspend/resume APIs instead of discarding it. |
| CHANGELOG.md | Documents the new optional suspend/resume reason parameters in the core SDK changelog under Unreleased. |
| azure-functions-durable/CHANGELOG.md | Documents the v1 compat fix to forward suspend/resume reasons under Unreleased. |
Suppressed comments (1)
tests/durabletask/test_client.py:1279
- Test name grammar: "include" should be "includes" (third-person singular) for readability/consistency.
async def test_async_suspend_resume_orchestration_include_optional_reason():
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 941fc052-b910-42af-93fa-4f00138813ab
Bernd Verst (berndverst)
left a comment
Bernd Verst (berndverst)
left a comment
There was a problem hiding this comment.
Please address the inline compatibility, in-memory backend, and package changelog gaps before merging.
| """ | ||
| await self.suspend_orchestration(instance_id) | ||
| """Deprecated alias for :meth:`suspend_orchestration`.""" | ||
| await self.suspend_orchestration(instance_id, reason=reason) |
There was a problem hiding this comment.
This now always passes reason=, but azure-functions-durable/pyproject.toml still permits durabletask>=1.9.0, whose suspend/resume methods do not accept this keyword. A valid dependency resolution can therefore make every deprecated suspend()/resume() call fail with TypeError, even when no reason is supplied. Please raise the minimum core version to the first release containing these signatures and coordinate the package release order.
There was a problem hiding this comment.
Documented the required coordinated release in the Azure Functions changelog and repository agent guidance. Per release policy, package versions and dependency minimums remain unchanged in this feature PR; a dedicated release PR must publish the core package first, then update provider minimums and versions.
| reason: str | None = None) -> None: | ||
| req = pb.SuspendRequest( | ||
| instanceId=instance_id, | ||
| reason=helpers.get_string_value(reason), |
There was a problem hiding this comment.
Please carry this reason through the public in-memory testing backend too. InMemoryOrchestrationBackend.SuspendInstance/ResumeInstance still call new_suspend_event()/new_resume_event() without request.reason, and those helpers create history events with empty input; tests using that backend therefore lose the reason while a real sidecar preserves it. Pass the optional reason through the helpers and add an in-memory history regression test.
There was a problem hiding this comment.
Passed the optional request reason into the in-memory suspend/resume history events and added an in-memory E2E regression that verifies both persisted history inputs. Fixed in 63db777.
|
|
||
| ADDED | ||
|
|
||
| - Added optional `reason` parameters to `TaskHubGrpcClient` and |
There was a problem hiding this comment.
These new parameters are also inherited by DurableTaskSchedulerClient and AsyncDurableTaskSchedulerClient, but durabletask-azuremanaged/CHANGELOG.md still has an empty Unreleased section. Repository policy requires each affected package changelog, and the prior inherited rewind API was documented there. Please add an ADDED entry for suspend/resume reasons.
There was a problem hiding this comment.
Added the inherited suspend/resume reason API entry under durabletask-azuremanaged/CHANGELOG.md Unreleased. Fixed in 63db777.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 941fc052-b910-42af-93fa-4f00138813ab
Updated instructions for creating a dedicated release PR to ensure proper versioning and compatibility.
Summary
Closes #223