Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the Durable Task Python SDK’s timer functionality by adding (1) task cancellation support for timers/external-event waits and (2) long-timer chunking to accommodate backends with maximum timer duration limits (resolving #95 and #101).
Changes:
- Introduces
CancellableTask/TaskCanceledErrorand wires cancellation into timer + external event tasks. - Adds
LongTimerTaskand updates orchestration execution to chunk long timers based on a maximum interval (default 3 days). - Expands orchestration executor tests to cover long-timer chunking, cancellation after
when_any, and long retry timers.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
durabletask/worker.py |
Implements long-timer chunk scheduling in the runtime context and executor; adds a maximum_timer_interval configuration knob. |
durabletask/task.py |
Adds cancellable task primitives and long-timer task implementation. |
tests/durabletask/test_orchestration_executor.py |
Adds unit tests for long-timer chunking/cancellation and retry behavior with long timers. |
You can also share your feedback on Copilot code review. Take the survey.
berndverst
left a comment
There was a problem hiding this comment.
The task cancellation support is a great addition — the CancellableTask pattern with cancel handlers for timers and external events is well-designed and the tests are thorough.
However, the long timer implementation introduces a separate LongTimerTask class that diverges from the .NET design. In the .NET SDK (TaskOrchestrationContextWrapper.cs#L266-L283), long timer chunking is handled entirely within the CreateTimer method using a simple loop — there is no separate timer type. The chunking is an implementation detail of the orchestration context, not a property of the task itself.
In Python we should not have two different classes for timers. Instead, the existing TimerTask should be updated to support long timers internally. The chunking logic should live in create_timer_internal and the timer-fired event handler, with TimerTask optionally holding the final_fire_at and maximum_timer_interval state when needed. This keeps the type hierarchy simple and aligns with the .NET design where the long timer behavior is transparent to consumers.
Note: The PR has a merge conflict (mergeable_state: dirty).
Disclaimer: This review was generated by GitHub Copilot on behalf of Bernd.
Adds the following to improve TimerTask:
Resolves #95
Resolves #101