⚙️ FEATURE: Set task status to RETRY during retry attempts - #90
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses Issue #84 by making retry attempts observable: when an action fails and is about to retry, the associated Task is now transitioned to TypeStatus.RETRY so logs/notifications can reflect the retry state.
Changes:
- Pass the
taskobject internally intoAction._run_action()(without forwarding it to user functions). - Set
task.status = TypeStatus.RETRYimmediately before the retry delay/sleep. - Add a unit test intended to verify
RETRYis set between retry attempts.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
dotflow/core/action.py |
Threads the task into _run_action() and sets TypeStatus.RETRY on retry paths. |
tests/core/test_action.py |
Adds a test for retry status visibility. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def test_sets_retry_status_before_retrying(self): | ||
| calls = {"count": 0} | ||
|
|
||
| def flaky_step(): | ||
| calls["count"] += 1 | ||
| if calls["count"] == 1: | ||
| raise Exception("Fail once") | ||
| return "ok" | ||
|
|
||
| inside = Action(flaky_step, retry=2, retry_delay=0) | ||
| inside(task=self.task) | ||
|
|
||
| self.assertEqual(self.task.status, TypeStatus.RETRY) | ||
|
|
There was a problem hiding this comment.
This test asserts the task’s final status is RETRY after the action completes successfully. In actual workflow execution (Execution), the task status is set to COMPLETED on success, so RETRY is only meant to be observable between attempts. Consider asserting RETRY at the correct moment (e.g., check self.task.status on the second call inside flaky_step, or patch dotflow.core.action.sleep to assert the status when the retry delay is entered), and also assert the action ultimately returns successfully / status transitions to the expected terminal status in the real execution path.
FernandoCelmer
left a comment
FernandoCelmer
left a comment
There was a problem hiding this comment.
🔍 Code Review
Code issues found: 3
| # | Severity | Description |
|---|---|---|
| 1 | [Blocking] | Task status remains RETRY after successful retry |
| 2 | [Suggestion] | Fragile kwargs.pop approach for passing task |
| 3 | [Suggestion] | Test does not verify mid-retry status |
…els for Python 3.9 compatibility
… v2 on Python 3.9
TypeStatus.RETRY was defined but never used. Tasks now correctly transition to RETRY status before each retry attempt, then reset to IN_PROGRESS after successful retry, making retry behavior visible to callbacks, logs, and notifications. - Store task as instance attribute (_current_task) instead of kwargs.pop - Set task.status = TypeStatus.RETRY before sleep in retry loop - Reset task.status = TypeStatus.IN_PROGRESS after successful retry - Add test verifying RETRY status timing and IN_PROGRESS reset Fixes issue where retry attempts were invisible to workflow observers and status remained incorrectly set to RETRY after completion. Made-with: Cursor
44040d2 to
08a1571
Compare
|
@FernandoCelmer i have pushed my latest with the fixes which you have mentioned, please check that out |
Made-with: Cursor
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.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
PR Title
⚙️ FEATURE: Set task status to RETRY during retry attempts
PR Body
Summary
TypeStatus.RETRY was defined but never used in the retry logic. This PR makes retry attempts visible to workflow observers by setting
task.status = TypeStatus.RETRYbefore each retry sleep, then resetting toIN_PROGRESSafter successful retry.Problem
TypeStatus.RETRYexisted but was never set during retry attemptsIN_PROGRESSbetween retries, making debugging harderChanges
self._current_task) instead of fragilekwargs.poptask.status = TypeStatus.RETRYbeforesleep(retry_delay)in retry looptask.status = TypeStatus.IN_PROGRESSafter successful retry (ifattempt > 1)Impact
IN_PROGRESS→RETRY→IN_PROGRESS→ (eventualCOMPLETED/FAILED)Test Plan
poetry run pytest tests/core/test_action.py -q(11 passed)poetry run tox -e dotflow-py310(OK)test_sets_retry_status_before_retryingvalidates timing and reset behaviorRelated
Fixes the issue where retry attempts were invisible to workflow monitoring systems and status remained incorrect after retry completion.