Skip to content

Commit 92f0cb6

Browse files
committed
Tighten workflow-failure handling comments
1 parent 704c670 commit 92f0cb6

2 files changed

Lines changed: 12 additions & 17 deletions

File tree

backend/apps/ifc_validation/tasks/task_runner.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -158,15 +158,13 @@ def on_workflow_failed(self, *args, **kwargs):
158158
id = args[1]
159159
request = ValidationRequest.objects.get(pk=id)
160160

161-
# Idempotency (IVS-750): both error_handler (link_error) and chord_error_handler
162-
# (chord on_error) can fire for the same failure. If already finalized, do nothing
163-
# so we don't re-send the failure emails.
161+
# Both error callbacks can fire for one failure; skip if already finalized so the
162+
# failure emails aren't sent twice.
164163
if request.status == ValidationRequest.Status.FAILED:
165164
logger.debug(f"Request {id} already marked FAILED; skipping duplicate failure handling.")
166165
return
167166

168-
# IVS-750: record a clear, human-readable reason instead of dumping raw celery
169-
# args/kwargs into status_reason. Full detail stays in the logs (logger.debug above).
167+
# Record a readable reason rather than the raw celery args/kwargs (full detail is logged above).
170168
exc = next((a for a in args if isinstance(a, BaseException)), None)
171169
reason = "Validation failed before any check could run (workflow-level error)."
172170
if exc:

backend/apps/ifc_validation/tests/tests_orphan_failed_request.py

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,13 @@
11
"""
2-
IVS-750 "failing request without relating task" -- option B (the minimal/elegant fix).
3-
4-
This variant does NOT fabricate a synthetic task. Instead it fixes the two real
5-
defects in on_workflow_failed:
6-
1. it could fire twice (error_handler via link_error AND chord_error_handler via
7-
chord on_error), double-sending the failure emails -> add an idempotency guard;
8-
2. it dumped raw celery args/kwargs into status_reason -> replace with a clean,
9-
human-readable reason (full detail stays in the logs).
10-
11-
Note: under option B the orphaned request still has NO ValidationTask
12-
(request.tasks.count() == 0). That is intentional for B -- it explains the failure
13-
on the request itself rather than inventing a task that never ran.
2+
Workflow-level failure handling in on_workflow_failed.
3+
4+
Fixes two defects without fabricating a synthetic task:
5+
1. it could fire twice (link_error + chord on_error), double-sending failure
6+
emails -> idempotency guard;
7+
2. it dumped raw celery args/kwargs into status_reason -> readable reason instead.
8+
9+
The failed request keeps zero tasks by design: the failure is explained on the
10+
request itself rather than by inventing a task that never ran.
1411
"""
1512

1613
from unittest.mock import patch

0 commit comments

Comments
 (0)