You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "shivaam (via GitHub)" <gi...@apache.org> on 2026/06/25 03:17:51 UTC
[PR] Handle missed deadline alerts when DagRuns fail [airflow]
shivaam opened a new pull request, #68961:
URL: https://github.com/apache/airflow/pull/68961
### Summary
- Handle pending DagRun deadlines immediately when `DagRun.update_state()` transitions a run to `FAILED`.
- Keep `Deadline.prune_deadlines()` limited to successful DagRun cleanup.
- Persist deadline callback context/routing data by reassigning `callback.data`, so both triggerer and executor callback paths survive commit/reload.
- Serialize `deadline.id` as a string in callback context so async trigger kwargs can be stored and run by the triggerer.
Closes: #60927
### Details
Failed DagRuns currently leave pending deadlines untouched until the scheduler's expired-deadline loop reaches `deadline_time`. This can delay the configured deadline alert by hours after the run has already failed.
This change treats scheduler-driven DagRun failure as an immediate missed deadline for pending deadlines on that DagRun. It uses the same row-locking shape as the scheduler deadline loop and filters already-missed rows to avoid duplicate handling.
While validating the callback path, the executor callback path also needed top-level routing fields persisted in `callback.data`; otherwise the scheduler could mark the callback pending but not enqueue it. The triggerer callback path needed the same JSON reassignment discipline and a string deadline id because trigger kwargs are serialized before the triggerer runs them.
Manual/API/UI "mark failed" remains out of scope for this patch.
### Tests
```bash
uv run --project airflow-core pytest -q airflow-core/tests/unit/models/test_deadline.py::TestDeadline::test_handle_miss airflow-core/tests/unit/models/test_deadline.py::TestDeadline::test_handle_miss_persists_triggerer_callback_context airflow-core/tests/unit/models/test_deadline.py::TestDeadline::test_handle_miss_persists_executor_callback_routing_data airflow-core/tests/unit/models/test_dagrun.py::TestDagRun::test_dagrun_success_prunes_dagrun_deadlines airflow-core/tests/unit/models/test_dagrun.py::TestDagRun::test_dagrun_success_skips_pruning_non_dagrun_deadlines airflow-core/tests/unit/models/test_dagrun.py::TestDagRun::test_dagrun_success_handles_empty_deadline_list airflow-core/tests/unit/models/test_dagrun.py::TestDagRun::test_dagrun_failure_handles_pending_deadline airflow-core/tests/unit/models/test_dagrun.py::TestDagRun::test_dagrun_deadline_variable_interval_stable --with-db-init
```
Result: `8 passed, 1 warning`.
```bash
uv run ruff check airflow-core/src/airflow/models/dagrun.py airflow-core/src/airflow/models/deadline.py airflow-core/tests/unit/models/test_dagrun.py airflow-core/tests/unit/models/test_deadline.py
```
Result: `All checks passed!`
Also tested manually in a real local Airflow daemon environment:
- API server, dag processor, triggerer, and two schedulers.
- Local Postgres metadata DB and LocalExecutor.
- Sync/executor deadline callback reached `success` and wrote a marker row.
- Async/triggerer deadline callback reached `success` and wrote a marker row.
Final E2E rows:
```text
async: dagrun_state=failed, deadline_missed=True, callback_type=triggerer, callback_state=success
sync: dagrun_state=failed, deadline_missed=True, callback_type=executor, callback_state=success
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "shivaam (via GitHub)" <gi...@apache.org>.
shivaam commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r3559593037
##########
airflow-core/src/airflow/ui/openapi-gen/requests/schemas.gen.ts:
##########
@@ -9426,14 +9426,18 @@ export const $DeadlineAlertResponse = {
title: 'Interval',
Review Comment:
Right. Curious how did you figure out that this line is failing? I cant see anything in the logs?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "shivaam (via GitHub)" <gi...@apache.org>.
shivaam commented on PR #68961:
URL: https://github.com/apache/airflow/pull/68961#issuecomment-4814955871
@ferruzzi thanks, this makes sense. I split the independent `callback.data` persistence fix out into #69073 so it can be reviewed separately from the failure-behavior question. That PR is scoped to `Deadline.handle_miss()` persisting the callback context/routing data correctly.
For this PR, I agree that always firing the Deadline Alert on failure is too broad as the default. I will rework this toward an opt-in `DeadlineAlert` flag (name TBD, maybe something like `fire_on_failure` / `alert_on_failure`, defaulting to `False`) so users can choose this behavior for the cases where a failed run means the deadline is inevitably missed.
The prune-hook and UI/manual-failure options also make sense to me, but I think those can be tracked as follow-up issues unless you think they should be included in this PR.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "shivaam (via GitHub)" <gi...@apache.org>.
shivaam commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r3554962880
##########
airflow-core/src/airflow/ui/openapi-gen/requests/schemas.gen.ts:
##########
@@ -9426,14 +9426,18 @@ export const $DeadlineAlertResponse = {
title: 'Interval',
Review Comment:
It is generated. The source change is the backend `DeadlineAlertResponse.fire_on_failure` field. The pre-commit generation updated `_private_ui.yaml`, and the UI codegen generated the matching `schemas.gen.ts` / `types.gen.ts` changes.
##########
airflow-core/src/airflow/ui/openapi-gen/requests/schemas.gen.ts:
##########
@@ -9426,14 +9426,18 @@ export const $DeadlineAlertResponse = {
title: 'Interval',
Review Comment:
It is generated. The source change is the backend `DeadlineAlertResponse.fire_on_failure` field. The pre-commit generation updated `_private_ui.yaml`, and the UI codegen generated the matching `schemas.gen.ts` / `types.gen.ts` changes.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r3555657921
##########
airflow-core/src/airflow/ui/openapi-gen/requests/schemas.gen.ts:
##########
@@ -9426,14 +9426,18 @@ export const $DeadlineAlertResponse = {
title: 'Interval',
Review Comment:
Just checking because it looks like that is what is causing the CI failure
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r4134341118
##########
airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml:
##########
@@ -3643,6 +3643,9 @@ components:
description: Interval in seconds between the reference time and the deadline.
Null for a dynamic interval (e.g. a VariableInterval) whose value is only
resolved at scheduler evaluation time.
+ fire_on_failure:
Review Comment:
I am not sure I understand why this setting is needed?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "shivaam (via GitHub)" <gi...@apache.org>.
shivaam commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r4169557532
##########
airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml:
##########
@@ -3643,6 +3643,9 @@ components:
description: Interval in seconds between the reference time and the deadline.
Null for a dynamic interval (e.g. a VariableInterval) whose value is only
resolved at scheduler evaluation time.
+ fire_on_failure:
Review Comment:
I think it is a new feature but yeah it is in grey area between feature and bug fix. @ferruzzi let me know what think.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r4170701328
##########
airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml:
##########
@@ -3643,6 +3643,9 @@ components:
description: Interval in seconds between the reference time and the deadline.
Null for a dynamic interval (e.g. a VariableInterval) whose value is only
resolved at scheduler evaluation time.
+ fire_on_failure:
Review Comment:
Sorry, I was out sick most of the week and just catching up. Here's my opinion and my reasoning. My intent was fail-early, I would like to drop the flag and call this a bug-fix. But that is not how the feature is documented. The doc pages say (some quotes, not exhaustive):
- "This example will send an email notification if the Dag hasn't finished 30 minutes after it was queued." (Core Concepts)
- "would trigger the alert if the Dag hasn't completed 15 minutes after it was scheduled to start, regardless of when (or if) it actually began executing." (how-to, DAGRUN_LOGICAL_DATE)
- "Alert if 30 minutes past average runtime", "Dag is running longer than expected!" (how-to, AVERAGE_RUNTIME)
Which means a user going off of only what they can see in the docs would expect that failure is a "finished" state and the callback shouldn't fire. That's what the Issue claims, and it's a fair reading of the docs.
To test this, I set Claude a task: "There is some interesting discussion around the Deadline Alerts feature in Airflow. I want you to answer this question using ONLY official public airflow doc pages, do not use my design notes, code comments, open github issues, web searches, etc. I want a blind reading direct from official documentation as a new user might have, not as an insider with design and implementation knowledge. If I create a Dag with a DeadlineAlert whose interval is three hours and that Dag fails after ten minutes, what is the expected behavior? Would the callback fire after 10 minutes, 3 hours, or never? Form your answer in the format `Setting a Deadline Alert could be read as "If this Dag _____ before this time, then fire this callback"`.
and it replied with: "If this Dag has not finished before this time, then fire this callback"` It would never fire. That is the only one of the three a doc-only reader can reasonably land on.
Which implies that regardless of my intention, the documentation as written doesn't convey what I meant for it to say.
If we rephrase the docs to say "if this Dag has not **succeeded** before this time, then do this callback", this is closer to what I had in mind and what Shivaam implemented. I'd say that was a bug-fix toward my vision, but if we're being fair, it's moving away from the documented behavior and the docs should be rephrased accordingly from "finished" to "succeeded", etc. But, regardless of my intention, I think I have to admit that a fair reading of the docs would be "do not fire on failure" which makes it hard to argue that this is a bug-fix and why I backed the flag.
Now, If we're open to changing the default behavior, then I'd actually propose inverting the flag. Make it work as-intended (fire on fail by default) with an opt-out (treat failure as "finished"; prune on failure and do not fire the callback). Users could then combine Deadlines with `on_failure_callback` or other methods of monitoring Dag failures to distinguish between "ran long" and "failed", and they don't necessarily want duplicate alarms on failure. Which I think is the use-case that caused me to implement it this way in the first place, but now it's just causing confusion.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "shivaam (via GitHub)" <gi...@apache.org>.
shivaam commented on PR #68961:
URL: https://github.com/apache/airflow/pull/68961#issuecomment-4928368602
> IMHO, this looks like it is working how I would expect it to work. I know there is still some discussion on whether this is the right approach, so I'll let that discussion play out before approving.
>
> Also, looks like a merge conflict needs to be resolved.
@ferruzzi
Thanks for the review. I will resolve the current merge conflict by rebasing the migration onto the latest 3.4.0 migration head.
One note: since this PR adds a migration, this kind of conflict may keep coming back whenever another migration lands on main first. To avoid churn, would it be okay to get the functional/design review settled first, and then I can do one final rebase of the migration metadata right before merge?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "shivaam (via GitHub)" <gi...@apache.org>.
shivaam commented on PR #68961:
URL: https://github.com/apache/airflow/pull/68961#issuecomment-5638453325
@eladkal Rebased onto the latest main and resolved the conflicts. CI is now green, and the PR is mergeable. Thanks!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "shivaam (via GitHub)" <gi...@apache.org>.
shivaam closed pull request #68961: Handle missed deadline alerts when DagRuns fail
URL: https://github.com/apache/airflow/pull/68961
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "shivaam (via GitHub)" <gi...@apache.org>.
shivaam commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r4174317420
##########
airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml:
##########
@@ -3643,6 +3643,9 @@ components:
description: Interval in seconds between the reference time and the deadline.
Null for a dynamic interval (e.g. a VariableInterval) whose value is only
resolved at scheduler evaluation time.
+ fire_on_failure:
Review Comment:
I won’t have the bandwidth to rework the implementation further, but we can settle the behavior in this PR so whoever picks it up has a clear direction.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "shivaam (via GitHub)" <gi...@apache.org>.
shivaam commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r4130851519
##########
airflow-core/src/airflow/migrations/versions/0141_3_4_0_add_fire_on_failure_to_deadline_alert.py:
##########
@@ -0,0 +1,48 @@
+#
Review Comment:
Addressed in 72e4f0db65cbc99b6d6d7fbc3b9ce3f71a42f095. The migration is now 0141_3_4_0_add_fire_on_failure_to_deadline_alert.py, following upstream revision e5a91c7f42b3. The Alembic-generated revision c70138c21ecf is unchanged, and db.py plus the migration reference are synchronized. All 142 migrations passed the local SQLite upgrade/downgrade round-trip. Fresh CI is queued for the rebased head.
---
Drafted-by: Codex (no human review before posting)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r4143484205
##########
airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml:
##########
@@ -3643,6 +3643,9 @@ components:
description: Interval in seconds between the reference time and the deadline.
Null for a dynamic interval (e.g. a VariableInterval) whose value is only
resolved at scheduler evaluation time.
+ fire_on_failure:
Review Comment:
My point is that this feels more like a bug fix rather than a feature hence no need for a flag/setting.
cc @ferruzzi
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on PR #68961:
URL: https://github.com/apache/airflow/pull/68961#issuecomment-4812887549
Thanks for the PR and the thorough testing. This had a pretty long discussion in the Issue and I still don't think short-circuiting by default is the right answer.
Let's say you have a Dag which generates a report which you need for your morning meeting so you set a Deadline Alert for 9AM which emails you saying your report won't be ready.
Case 1) You work at Big Company. The Dag fails at 7AM, an admin sees this, determines it's just a network hiccup and clears the failing tasks which then pass at 7:30. If the deadline fired on failure, you would have your report **and** a notice saying you won't.
Case 2) Your Dag may have an `on_failure_callback` configured which notifies you of the failure. If the deadline also fires immediately, you now get two notifications about the same event with different messaging: Schrodinger's Dag is both failed and late.
Case 3) You work at Small Company and don't have an admin team, you do it all yourself, so a Dag failing likely won't be cleared and the missed deadline is inevitable. I'd argue that `on_failure_callback` currently handles this, but I can see the advantage of bundling it into the Deadline Alert directly..
Case 4) You realize something is wrong with the input and you manually mark the Dag as FAILED so the report doesn't get generated with bad data. Are you going to fix the data and re-trigger the Dag, or is that the end of the story? We don't know and shouldn't guess. Worth noting: this PR wouldn't cover this case either, which would introduce a difference in behavior between an automatic failure and a manual one.
Overall, always short-circuiting is not the answer. I would propose three alternatives which are not mutually exclusive:
1. Add a flag to the Deadline Alert class. Naming is hard, but something like `fire_on_failure: bool = False` and let the user decide depending on their use case.
2. Add a hook in the Deadline class which accepts a `run_id` and prunes all future deadlines for that run. This would allow `on_failure_callback` (or any other callback) to take action and then call the new hook to remove the now-redundant deadline(s) so you don't get double-notified.
3. Anywhere in the UI where a user can mark a Dag as FAILED should have an option to also clear related Deadlines.
To be clear, I do agree that there is a real user need here, I'm not trying to kill this or dissuade you. I just want to make sure we solve it without breaking the existing use cases. It's one very valid use case, but that doesn't invalidate the others.
That said, here's my suggestion: I'd also ask that the `callback.data` persistence fixes in `deadline.py` be split into a separate PR since those fix a real bug independent of this feature question. Whatever we do about the failure state, that looks like an actual legit improvement/bug-fix. Then I'd suggest reworking this PR to implement the flag and tracking the other two options as new Issues.
((tagging @ramitkataria since he was also involved in the original discussion and may have thoughts.))
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "shivaam (via GitHub)" <gi...@apache.org>.
shivaam commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r3560542539
##########
airflow-core/src/airflow/ui/openapi-gen/requests/schemas.gen.ts:
##########
@@ -9426,14 +9426,18 @@ export const $DeadlineAlertResponse = {
title: 'Interval',
Review Comment:
All checks approved now. Also I think Ramit does not have any other comments on the design as he +1'ed your comment earlier. I will wait for any other reviews.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "shivaam (via GitHub)" <gi...@apache.org>.
shivaam commented on PR #68961:
URL: https://github.com/apache/airflow/pull/68961#issuecomment-5081606649
@amoghrajesh / @kaxil If you get a chance, could you please help review this PR
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r3531584776
##########
airflow-core/docs/howto/deadline-alerts.rst:
##########
@@ -44,6 +44,7 @@ Creating a Deadline Alert requires three mandatory parameters:
* Reference: When to start counting from
* Interval: How far before or after the reference point to trigger the alert (either a timedelta or a dynamic interval such as VariableInterval)
* Callback: A Callback object which contains a path to a callable and optional kwargs to pass to it if the deadline is exceeded
+* Fire on failure: Optional. If set to ``True``, a pending Dag run deadline alert fires immediately when the scheduler marks the Dag run failed. Manual UI/API mark-failed paths are unchanged. The default is ``False``.
Review Comment:
I feel like this is the correct answer for cases 1 and 3 from above, and Ramit's `skip_on_failure` would be the solution for case 2:
`fire_on_failure=False` covers case 1 from above:
A user sees the Dag failed and re-triggers it; the user doesn't want that deadline alert to go out until they can't possibly recover.
`fire_on_failure=True` covers case 3 from above:
If the Dag has failed and can not recover then it will inevitably miss the deadline, the user may want that advanced notice, which is what this PR allows.
`skip_on_failure` covers case 2 from above:
If the Dag has an `on_failure_callback` and the user decides that they don't need the deadline callback if that one goes off, then that would be where Ramit's `skip_on_failure` comes in.
I don't see them as mutually exclusive and I feel like shivaam can implement `skip_on_failure` in a follow-up.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "shivaam (via GitHub)" <gi...@apache.org>.
shivaam commented on PR #68961:
URL: https://github.com/apache/airflow/pull/68961#issuecomment-5081078576
> @shivaam can you rebase and fix conflicts?
Updated the branch with the latest `main` and resolved the migration conflict.
Since this PR adds a migration, another conflict may appear if a different migration is merged into `main` first. Hopefully, we can merge before that happens again
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #68961:
URL: https://github.com/apache/airflow/pull/68961#issuecomment-5081005698
@shivaam can you rebase wnd fix conflicts?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #68961:
URL: https://github.com/apache/airflow/pull/68961#issuecomment-5628165318
@shivaam can you rebase and resolve conflicts?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #68961:
URL: https://github.com/apache/airflow/pull/68961#issuecomment-5888387529
@shivaam tests are failing
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "shivaam (via GitHub)" <gi...@apache.org>.
shivaam commented on PR #68961:
URL: https://github.com/apache/airflow/pull/68961#issuecomment-4928374536
@ramitkataria do you have any other feedback comments on it?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "ramitkataria (via GitHub)" <gi...@apache.org>.
ramitkataria commented on PR #68961:
URL: https://github.com/apache/airflow/pull/68961#issuecomment-4940782134
> @ramitkataria do you have any other feedback comments on it?
Nope, looks good to me
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r3531627914
##########
airflow-core/src/airflow/migrations/versions/0124_3_3_0_add_fire_on_failure_to_deadline_alert.py:
##########
@@ -0,0 +1,48 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Add fire_on_failure to deadline_alert.
+
+Revision ID: b4f8c2a1d9e0
+Revises: d2f4e1b3c5a7
+Create Date: 2026-07-01 12:00:00.000000
+"""
+
+from __future__ import annotations
+
+import sqlalchemy as sa
+from alembic import op
+
+revision = "b4f8c2a1d9e0"
+down_revision = "d2f4e1b3c5a7"
+branch_labels = None
+depends_on = None
+airflow_version = "3.3.0"
Review Comment:
3.3.0 shipped this morning, so that ship has sailed; bump this to 3.4.0 I guess?
##########
airflow-core/tests/unit/models/test_dagrun.py:
##########
@@ -1505,6 +1532,225 @@ def test_dagrun_success_handles_empty_deadline_list(self, mock_prune, dag_maker,
mock_prune.assert_not_called()
assert dag_run.state == DagRunState.SUCCESS
+ @mock.patch.object(Deadline, "handle_miss")
+ @mock.patch.object(Deadline, "prune_deadlines")
+ def test_dagrun_failure_handles_pending_deadline(
+ self, mock_prune, mock_handle_miss, session, deadline_test_dag
+ ):
+ scheduler_dag = deadline_test_dag()
+
+ dag_run = self.create_dag_run(
+ dag=scheduler_dag,
+ task_states={"task_1": TaskInstanceState.SUCCESS, "task_2": TaskInstanceState.FAILED},
+ session=session,
+ )
+ deadline_alert = self.create_deadline_alert_model(
+ scheduler_dag, session=session, fire_on_failure=True
+ )
+ scheduler_dag.deadline = [str(deadline_alert.id)]
+ dag_run.dag = scheduler_dag
+ session.add(
+ Deadline(
+ deadline_time=timezone.utcnow() + datetime.timedelta(hours=3),
+ callback=AsyncCallback(empty_callback_for_deadline),
+ dagrun_id=dag_run.id,
+ dag_id=dag_run.dag_id,
+ deadline_alert_id=deadline_alert.id,
+ )
+ )
+ session.flush()
+
+ dag_run.update_state(session=session)
+
+ mock_handle_miss.assert_called_once()
+ mock_prune.assert_not_called()
+ assert dag_run.state == DagRunState.FAILED
+
+ @mock.patch.object(Deadline, "handle_miss")
+ def test_dagrun_deadlock_handles_pending_deadline(self, mock_handle_miss, dag_maker, session):
+ with dag_maker(
+ dag_id="test_dagrun_deadlock_handles_pending_deadline",
+ schedule=datetime.timedelta(days=1),
+ session=session,
+ ):
+ up = EmptyOperator(task_id="upstream")
+ middle = EmptyOperator(task_id="wrong")
+ down = EmptyOperator(task_id="downstream")
+
+ middle.trigger_rule = TriggerRule.ONE_FAILED
+ middle.set_upstream(up)
+ middle.set_downstream(down)
+
+ dag_run = dag_maker.create_dagrun()
+ scheduler_dag = dag_run.get_dag()
+ deadline_alert = self.create_deadline_alert_model(
+ scheduler_dag, session=session, fire_on_failure=True
+ )
+ scheduler_dag.deadline = [str(deadline_alert.id)]
+ dag_run.dag = scheduler_dag
+ session.add(
+ Deadline(
+ deadline_time=timezone.utcnow() + datetime.timedelta(hours=3),
+ callback=AsyncCallback(empty_callback_for_deadline),
+ dagrun_id=dag_run.id,
+ dag_id=dag_run.dag_id,
+ deadline_alert_id=deadline_alert.id,
+ )
+ )
+
+ ti_up: TI = dag_run.get_task_instance(task_id=up.task_id, session=session)
+ ti_middle: TI = dag_run.get_task_instance(task_id=middle.task_id, session=session)
+ ti_up.set_state(state=TaskInstanceState.SUCCESS, session=session)
+ ti_middle.set_state(state=None, session=session)
+ ti_middle.task.trigger_rule = "invalid"
+ session.flush()
+
+ dag_run.update_state(session=session)
+
+ mock_handle_miss.assert_called_once()
+ assert dag_run.state == DagRunState.FAILED
+
Review Comment:
Do we want a unit test for the case where the DeadlineAlert has already been missed and assert that is isn't re-fired on failure?
If the deadline alert is for 10 seconds, fire_on_failure=True, and the dag fails at 1 minute, do we expect that to fire again?
The query uses `~Deadline.missed` so it shouldn't fire again, which I think is correct; we just don't have a test to verify it or prevent regression.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "ramitkataria (via GitHub)" <gi...@apache.org>.
ramitkataria commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r3522300270
##########
airflow-core/docs/howto/deadline-alerts.rst:
##########
@@ -44,6 +44,7 @@ Creating a Deadline Alert requires three mandatory parameters:
* Reference: When to start counting from
* Interval: How far before or after the reference point to trigger the alert (either a timedelta or a dynamic interval such as VariableInterval)
* Callback: A Callback object which contains a path to a callable and optional kwargs to pass to it if the deadline is exceeded
+* Fire on failure: Optional. If set to ``True``, a pending Dag run deadline alert fires immediately when the scheduler marks the Dag run failed. Manual UI/API mark-failed paths are unchanged. The default is ``False``.
Review Comment:
I guess I'm just not sure if we should be overlapping functionality with on_failure_callback. And wouldn't you want a notification saying "the Dagrun ... has failed" instead of "the Dagrun missed the deadline" when it fails?
I think most people would probably need either a failure notification or a late notification depending on the outcome but not both, right? So, the most intuitive (for me) way to achieve that would be to set both deadline and failure alerts but add a flag to either deadline alert (maybe `skip_on_failure`) or on_failure_callback (maybe `clear_deadline_alerts`) to clear the scheduled deadline alerts. Or we could just let the users use the proposed hook to prune deadlines.
I wonder what @ferruzzi thinks about this as well
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r3786270181
##########
airflow-core/src/airflow/serialization/encoders.py:
##########
@@ -251,6 +251,7 @@ def encode_deadline_alert(d: DeadlineAlert | SerializedDeadlineAlert) -> dict[st
"reference": encode_deadline_reference(d.reference),
"interval": serialize(d.interval),
"callback": serialize(d.callback),
+ "fire_on_failure": d.fire_on_failure,
Review Comment:
Sorry I missed this earlier. This is going to be a back-compat issue in older SDK versions which don't have `d.fire_on_failure` yet. Easy fix though, set a fallback to be the existing current behavior if `d.fire_on_failure` doesn't exist :
```
"fire_on_failure": getattr(d, "fire_on_failure", False)
```
##########
airflow-core/src/airflow/models/dagrun.py:
##########
@@ -1240,6 +1241,42 @@ def _emit_dagrun_span(self, state: DagRunState):
span.set_status(status_code)
span.end()
+ def _handle_missed_deadlines(self, *, session: Session) -> None:
+ """Handle pending deadlines that opt in to firing when their DagRun fails."""
+ deadline_query = (
+ select(Deadline)
+ .join(DeadlineAlertModel, Deadline.deadline_alert_id == DeadlineAlertModel.id)
+ .where(Deadline.dagrun_id == self.id)
+ .where(~Deadline.missed)
+ .where(DeadlineAlertModel.fire_on_failure.is_(True))
+ .options(
+ selectinload(Deadline.callback),
+ selectinload(Deadline.dagrun),
+ selectinload(Deadline.deadline_alert),
+ )
+ )
+ for deadline in session.scalars(
+ with_row_locks(
+ deadline_query,
+ of=Deadline,
+ session=session,
+ skip_locked=True,
+ key_share=False,
+ )
+ ):
+ deadline_id = deadline.id
+ try:
+ deadline.handle_miss(session)
+ except DBAPIError:
Review Comment:
Sorry I missed this earlier, this should likely be `SQLAlchemyError` as used elsewhere. If you disagree, feel free to just resolve this, but I noticed that one was already imported and you had to add an import for this new one
##########
airflow-core/src/airflow/migrations/versions/0128_3_4_0_add_fire_on_failure_to_deadline_alert.py:
##########
@@ -0,0 +1,48 @@
+#
Review Comment:
Don't forget you'll need to rename this file when you fix the db.py conflicts, we're on migration 0131
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "shivaam (via GitHub)" <gi...@apache.org>.
shivaam commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r4140053047
##########
airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml:
##########
@@ -3643,6 +3643,9 @@ components:
description: Interval in seconds between the reference time and the deadline.
Null for a dynamic interval (e.g. a VariableInterval) whose value is only
resolved at scheduler evaluation time.
+ fire_on_failure:
Review Comment:
`fire_on_failure` lets a deadline alert fire immediately when its DagRun fails, even if the deadline hasn’t expired. It defaults to `False` to preserve existing behavior.
This particular YAML change exposes that policy in the API response for a future UI addition—for example, showing “Also alert on DagRun failure” alongside the deadline details, so users understand why an alert fired before its configured deadline.
The current UI doesn’t consume this field yet, so we can omit it from the private UI response for now and add it alongside that UI change.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #68961:
URL: https://github.com/apache/airflow/pull/68961#issuecomment-4798242623
@shivaam — the **Static checks** CI job is failing here, which needs a code fix on your side (a rerun won't clear it). You can reproduce and fix locally with:
```
pre-commit run --all-files # or: breeze static-checks
```
Once those pass and CI is green, it'll be ready for a maintainer to pick up. Thanks!
See the [PR quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
<sub>Automated first-pass triage note drafted by an AI-assisted tool — may get things wrong; once addressed, a real Apache Airflow maintainer takes the next look. ([why automated](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md))</sub>
---
Drafted-by: Claude Code (Opus 4.8); reviewed by @potiuk before posting
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "shivaam (via GitHub)" <gi...@apache.org>.
shivaam commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r3471719063
##########
airflow-core/src/airflow/models/dagrun.py:
##########
@@ -1164,6 +1173,30 @@ def _emit_dagrun_span(self, state: DagRunState):
span.set_status(status_code)
span.end()
+ def _handle_missed_deadlines(self, *, session: Session) -> None:
+ deadline_query = (
+ select(Deadline)
+ .where(Deadline.dagrun_id == self.id)
+ .where(~Deadline.missed)
+ .options(selectinload(Deadline.callback), selectinload(Deadline.dagrun))
+ )
+ for deadline in session.scalars(
+ with_row_locks(
+ deadline_query,
+ of=Deadline,
+ session=session,
+ skip_locked=True,
+ key_share=False,
+ )
+ ):
+ deadline.handle_miss(session)
+
+ def _handle_missed_deadlines_with_error_capture(self, *, session: Session) -> None:
+ try:
+ self._handle_missed_deadlines(session=session)
+ except Exception:
Review Comment:
missed deadline is a side effect and it should not impact the state change of the dag.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "shivaam (via GitHub)" <gi...@apache.org>.
shivaam commented on PR #68961:
URL: https://github.com/apache/airflow/pull/68961#issuecomment-5080263350
@ferruzi is this PR ready to merge?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "shivaam (via GitHub)" <gi...@apache.org>.
shivaam commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r3553942289
##########
airflow-core/tests/unit/models/test_dagrun.py:
##########
@@ -1505,6 +1532,225 @@ def test_dagrun_success_handles_empty_deadline_list(self, mock_prune, dag_maker,
mock_prune.assert_not_called()
assert dag_run.state == DagRunState.SUCCESS
+ @mock.patch.object(Deadline, "handle_miss")
+ @mock.patch.object(Deadline, "prune_deadlines")
+ def test_dagrun_failure_handles_pending_deadline(
+ self, mock_prune, mock_handle_miss, session, deadline_test_dag
+ ):
+ scheduler_dag = deadline_test_dag()
+
+ dag_run = self.create_dag_run(
+ dag=scheduler_dag,
+ task_states={"task_1": TaskInstanceState.SUCCESS, "task_2": TaskInstanceState.FAILED},
+ session=session,
+ )
+ deadline_alert = self.create_deadline_alert_model(
+ scheduler_dag, session=session, fire_on_failure=True
+ )
+ scheduler_dag.deadline = [str(deadline_alert.id)]
+ dag_run.dag = scheduler_dag
+ session.add(
+ Deadline(
+ deadline_time=timezone.utcnow() + datetime.timedelta(hours=3),
+ callback=AsyncCallback(empty_callback_for_deadline),
+ dagrun_id=dag_run.id,
+ dag_id=dag_run.dag_id,
+ deadline_alert_id=deadline_alert.id,
+ )
+ )
+ session.flush()
+
+ dag_run.update_state(session=session)
+
+ mock_handle_miss.assert_called_once()
+ mock_prune.assert_not_called()
+ assert dag_run.state == DagRunState.FAILED
+
+ @mock.patch.object(Deadline, "handle_miss")
+ def test_dagrun_deadlock_handles_pending_deadline(self, mock_handle_miss, dag_maker, session):
+ with dag_maker(
+ dag_id="test_dagrun_deadlock_handles_pending_deadline",
+ schedule=datetime.timedelta(days=1),
+ session=session,
+ ):
+ up = EmptyOperator(task_id="upstream")
+ middle = EmptyOperator(task_id="wrong")
+ down = EmptyOperator(task_id="downstream")
+
+ middle.trigger_rule = TriggerRule.ONE_FAILED
+ middle.set_upstream(up)
+ middle.set_downstream(down)
+
+ dag_run = dag_maker.create_dagrun()
+ scheduler_dag = dag_run.get_dag()
+ deadline_alert = self.create_deadline_alert_model(
+ scheduler_dag, session=session, fire_on_failure=True
+ )
+ scheduler_dag.deadline = [str(deadline_alert.id)]
+ dag_run.dag = scheduler_dag
+ session.add(
+ Deadline(
+ deadline_time=timezone.utcnow() + datetime.timedelta(hours=3),
+ callback=AsyncCallback(empty_callback_for_deadline),
+ dagrun_id=dag_run.id,
+ dag_id=dag_run.dag_id,
+ deadline_alert_id=deadline_alert.id,
+ )
+ )
+
+ ti_up: TI = dag_run.get_task_instance(task_id=up.task_id, session=session)
+ ti_middle: TI = dag_run.get_task_instance(task_id=middle.task_id, session=session)
+ ti_up.set_state(state=TaskInstanceState.SUCCESS, session=session)
+ ti_middle.set_state(state=None, session=session)
+ ti_middle.task.trigger_rule = "invalid"
+ session.flush()
+
+ dag_run.update_state(session=session)
+
+ mock_handle_miss.assert_called_once()
+ assert dag_run.state == DagRunState.FAILED
+
Review Comment:
Done
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r3554541012
##########
airflow-core/src/airflow/ui/openapi-gen/requests/schemas.gen.ts:
##########
@@ -9426,14 +9426,18 @@ export const $DeadlineAlertResponse = {
title: 'Interval',
Review Comment:
Did you modify this or did the pre-commits do it?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r3582999234
##########
airflow-core/docs/howto/deadline-alerts.rst:
##########
@@ -44,6 +44,7 @@ Creating a Deadline Alert requires three mandatory parameters:
* Reference: When to start counting from
* Interval: How far before or after the reference point to trigger the alert (either a timedelta or a dynamic interval such as VariableInterval)
* Callback: A Callback object which contains a path to a callable and optional kwargs to pass to it if the deadline is exceeded
+* Fire on failure: Optional. If set to ``True``, a pending Dag run deadline alert fires immediately when the scheduler marks the Dag run failed. Manual UI/API mark-failed paths are unchanged. The default is ``False``.
Review Comment:
Ramit approved the PR after this discussion so I'm going to assume that means we're aligned and close this. Thanks for the discussion.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle missed deadline alerts when DagRuns fail [airflow]
Posted by "shivaam (via GitHub)" <gi...@apache.org>.
shivaam commented on code in PR #68961:
URL: https://github.com/apache/airflow/pull/68961#discussion_r4174316278
##########
airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml:
##########
@@ -3643,6 +3643,9 @@ components:
description: Interval in seconds between the reference time and the deadline.
Null for a dynamic interval (e.g. a VariableInterval) whose value is only
resolved at scheduler evaluation time.
+ fire_on_failure:
Review Comment:
I am adding a third approach below :)
My understanding is that failure notifications and deadline notifications serve different purposes. on_failure_callback tells users that a Dag has failed; a deadline alert tells them that the expected outcome wasn’t achieved by the deadline. I’d prefer keeping that distinction explicit so users know they would need both.
Under that approach, deadline alerts would retain their deadline-based timing. Users who already receive failure notifications could optionally suppress the pending deadline alert after failure, perhaps through a flag such as suppress_deadline_on_failure. Users wanting immediate failure notifications would use on_failure_callback.
For example, a Dag could fail at 7 a.m. with a 9 a.m. deadline. The on-call receives the failure notification, clears the failed tasks, and attempts recovery. If the pending deadline alert was cancelled at 7 a.m., they would lose the notification that recovery still hadn’t succeeded by 9 a.m.
That leaves a question: should clearing the failed tasks restore deadline monitoring? From an on-call perspective, I would want that coverage restored while attempting recovery.
Would this separation make sense, rather than making deadline callbacks fire immediately on failure?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org