Allow infinite retries for workflows#935
Conversation
acroca
left a comment
There was a problem hiding this comment.
Also, I think it'd be useful to add a scenario for the infinite retries to the examples/workflow/simple.py. The example could be just a counter that makes it fail 10 times, and succeed after all those failures, and assert the workflow completed successfully. makes sense?
828611a to
95feef7
Compare
|
I adjusted the WorkflowRuntime start time so it doesn't fail when the worker isn't ready within the expected time. |
|
Thanks! |
| self._logger.warning( | ||
| 'WorkflowRuntime worker and its stream are not ready. ' | ||
| 'Continuing; workflows scheduled immediately may not be received.' | ||
| ) |
There was a problem hiding this comment.
Hm do you mind explaining this change? I feel it'd be good to raise an an error because the worker is not ready.
There was a problem hiding this comment.
It was due to a pipeline error, where the attempts were altered.
|
I apologize, I thought I had to do a squash commit. |
|
pr a pipeline tb failed again, I don't understand the error. |
|
Hi! 👋 can you try running these cmds pls to fix the DCO build step? |
…lows (dapr#931) Signed-off-by: Albert Callarisa <albert@diagrid.io> Signed-off-by: Matheus André <92062874+matheusandre1@users.noreply.github.com>
Signed-off-by: Matheus André <92062874+matheusandre1@users.noreply.github.com>
Signed-off-by: Matheus André <92062874+matheusandre1@users.noreply.github.com>
Signed-off-by: Matheus André <92062874+matheusandre1@users.noreply.github.com>
|
@matheusandre1 can you address the comments? |
|
@CasperGN I didn't understand, I corrected what was requested. |
|
@CasperGN , I'm having trouble understanding the CI errors; check examples. |
Sorry I wasn't explicit enough. There's a couple of test failures that looks related to the infinite retries. The problem is deeper though: If you note the last line, the value error is thrown from durable task so this PR cannot be alone - it needs to follow with a PR in durable task that allow propagating Would it also be possible, when you get both repo PRs ready, to run it locally with an example of infinite retries? |
Signed-off-by: Albert Callarisa <albert@diagrid.io>
|
@matheusandre1 I updated the PR, I hope you don't mind. We've moved durabletask to be in-repo now, so we don't need another PR for this. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #935 +/- ##
==========================================
- Coverage 86.63% 81.55% -5.08%
==========================================
Files 84 140 +56
Lines 4473 13609 +9136
==========================================
+ Hits 3875 11099 +7224
- Misses 598 2510 +1912 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Enables “infinite retries” for workflow/activity retry policies by allowing max_number_of_attempts=-1, and adds tests plus an updated workflow example to validate the behavior end-to-end.
Changes:
- Allow
max_number_of_attempts=-1in both the publicRetryPolicywrapper and the internal durabletaskRetryPolicy, with updated validation/error messaging. - Update retry scheduling logic to treat
-1as “no attempt cap” when computing retry delays. - Add unit tests (policy validation, delay computation, executor rescheduling) and update the workflow example + README expectations.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ext/dapr-ext-workflow/tests/test_retry_policy.py | Adds unit tests for wrapper RetryPolicy validation and a simple infinite-retry behavior check. |
| ext/dapr-ext-workflow/tests/durabletask/test_task.py | Adds tests for durabletask RetryPolicy validation and RetryableTask.compute_next_delay() behavior for infinite/timeout/capped retries. |
| ext/dapr-ext-workflow/tests/durabletask/test_orchestration_executor.py | Adds executor-level tests ensuring activities and sub-orchestrations keep retrying when max_number_of_attempts=-1. |
| ext/dapr-ext-workflow/dapr/ext/workflow/retry_policy.py | Permits -1 and documents it as infinite retries in the public API wrapper. |
| ext/dapr-ext-workflow/dapr/ext/workflow/_durabletask/task.py | Permits -1 and skips the attempt-cap check in RetryableTask.compute_next_delay(). |
| examples/workflow/simple.py | Demonstrates infinite-retry activity usage and asserts expected retry counts. |
| examples/workflow/README.md | Updates expected output lines and increases the step timeout to account for the longer-running example. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Albert Callarisa <albert@diagrid.io>
Signed-off-by: Albert Callarisa <albert@diagrid.io>
Description
I added the requested validation and created the test according to the project's naming convention.
Issue reference
Please reference the issue this PR will close: #870