avanish-garg opened a new pull request, #72424:
URL: https://github.com/apache/airflow/pull/72424
Fixes #72379
## Problem
`on_task_instance_failed` fires identically whether a task instance is about
to be automatically
retried (state set to `UP_FOR_RETRY`) or has permanently failed (state set
to `FAILED`) — both
call sites (`models/taskinstance.py`'s `fetch_handle_failure_context`, and
the task-sdk's
`finalize()` in `execution_time/task_runner.py`) invoke it unconditionally
in both branches, even
though the hookspec's own docstring says "Execute when task state changes to
FAIL."
This makes the listener API noisy for a common use case: an alerting/paging
integration built on
`on_task_instance_failed` fires on every transient retry of a flaky task,
not just the final
terminal failure. There's a partial workaround today (checking
`task_instance.state` inside the
hook), but the hook's name and documented contract don't make that obvious.
## Change
- New `on_task_instance_up_for_retry(previous_state, task_instance, error)`
hookspec in the shared
listener spec (`airflow_shared/listeners/spec/taskinstance.py`), picked up
automatically by both
the core and task-sdk listener managers (no separate registration needed,
unlike specs that live
in their own module).
- Fired from both existing `on_task_instance_failed` call sites, right after
that call, only when
the task instance's resulting state is `UP_FOR_RETRY`:
- `models/taskinstance.py::TaskInstance.fetch_handle_failure_context`
(API-server-triggered path)
- `execution_time/task_runner.py`'s `finalize()` (task-execution path —
this one already has a
distinct `on_retry_callback` vs `on_failure_callback` split at the
DAG-author callback level,
so this hook follows the same precedent already established for
user-facing callbacks)
- `on_task_instance_failed` is left firing unchanged in both branches — this
is purely additive,
so no existing listener's behavior changes.
- Docs: new bullet + example in
`administration-and-deployment/listeners.rst`'s "Compatibility"
table and TaskInstance events section, plus a runnable example in
`example_dags/plugins/event_listener.py`.
- Tests covering both call sites: that the new hook fires for the
retry-eligible branch, that it
does *not* fire for the terminal-failure branch, and that
`on_task_instance_failed` still fires
in both cases as before.
## Design note
I considered instead scoping `on_task_instance_failed` to the
terminal-failure branch only (as
suggested in the issue), but that would be a breaking behavior change for
any listener already
relying on it firing for retries too. Adding the new hook without touching
the existing one's
firing behavior is a strictly additive, backward-compatible change, so I
went with that instead.
Happy to discuss if a different tradeoff is preferred.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
