Conversation
Signed-off-by: Albert Callarisa <albert@diagrid.io>
There was a problem hiding this comment.
Pull request overview
This PR updates the durable workflow SDK to populate the newly added protobuf origin oneof on timer actions/events, and extends wait_for_external_event to support optional timeouts by internally scheduling a timer (or a far-future timer when no timeout is provided), aligning behavior with other SDKs.
Changes:
- Add a
timeoutparameter towait_for_external_eventacross the public context APIs and implement it via a composite task that races the external event against a timer. - Populate timer
originmetadata for user timers (create_timer), external-event wait timers, and retry delay timers (activity + child workflow). - Rebuild protobuf Python outputs/stubs and add/adjust unit tests for the new timer-origin behavior.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ext/dapr-ext-workflow/tests/durabletask/test_orchestration_executor.py | Updates existing external-event tests for the new “auto timer” behavior and adds new assertions/tests around timer origin fields. |
| ext/dapr-ext-workflow/dapr/ext/workflow/workflow_context.py | Extends the public workflow context interface to support wait_for_external_event(..., timeout=...). |
| ext/dapr-ext-workflow/dapr/ext/workflow/dapr_workflow_context.py | Threads the new timeout keyword through the public wrapper context to the underlying durable task context. |
| ext/dapr-ext-workflow/dapr/ext/workflow/_durabletask/worker.py | Sets timer origins for create-timer and retry timers, and implements external-event timeout/far-future timer scheduling. |
| ext/dapr-ext-workflow/dapr/ext/workflow/_durabletask/task.py | Adds ExternalEventWithTimeoutTask composite task and updates the orchestration context contract for the new timeout parameter. |
| ext/dapr-ext-workflow/dapr/ext/workflow/_durabletask/internal/helpers.py | Extends timer action helper to populate the correct origin oneof field. |
| ext/dapr-ext-workflow/dapr/ext/workflow/_durabletask/internal/orchestrator_actions_pb2.pyi | Regenerated protobuf typing stubs for new timer origin fields. |
| ext/dapr-ext-workflow/dapr/ext/workflow/_durabletask/internal/orchestrator_actions_pb2.py | Regenerated protobuf runtime code for new timer origin fields. |
| ext/dapr-ext-workflow/dapr/ext/workflow/_durabletask/internal/history_events_pb2.pyi | Regenerated protobuf typing stubs for new timer origin fields on history events. |
| ext/dapr-ext-workflow/dapr/ext/workflow/_durabletask/internal/history_events_pb2.py | Regenerated protobuf runtime code for new timer origin fields on history events. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Albert Callarisa <albert@diagrid.io>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Albert Callarisa <albert@diagrid.io>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Albert Callarisa <albert@diagrid.io>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #976 +/- ##
==========================================
- Coverage 86.63% 81.06% -5.58%
==========================================
Files 84 137 +53
Lines 4473 13184 +8711
==========================================
+ Hits 3875 10687 +6812
- Misses 598 2497 +1899 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Albert Callarisa <albert@diagrid.io>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Albert Callarisa <albert@diagrid.io>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Albert Callarisa <albert@diagrid.io>
sicoyle
left a comment
sicoyle
left a comment
There was a problem hiding this comment.
nice job 👏 👏 👏 🙌 few comments so far and I've reviewed mostttt files
| if self.is_complete: | ||
| return | ||
| if completed_task is self._event_task: | ||
| if completed_task.is_failed: |
There was a problem hiding this comment.
this is missing a few other terminal states i believe in the if conditional check
There was a problem hiding this comment.
on_child_completed is only called after a child completes, so it's guaranteed to be in one of two states
Signed-off-by: Albert Callarisa <albert@diagrid.io>
Signed-off-by: Albert Callarisa <albert@diagrid.io>
originfield in timers.wait_for_external_eventto create a timer associated with such event. Other SDKs follow this same approach.