Deadline reference evaluation should receive the DagRun instead of dag_id/run_id
- Dominant language
- Python
- Stars
- 46.9k
- Forks
- 17.8k
- Avg merge
- 2d 9h
- Merged PRs (30d)
- 472
Description
### Body
Custom and built-in deadline references are evaluated via `evaluate_with(session=..., interval=..., dag_id=..., run_id=...)` at Dag run creation. The two identifiers exist only so the built-in references can pull the Dag run out of the database, but the caller already has it.
`airflow-core/src/airflow/serialization/definitions/dag.py:764` has a TODO noting this (added in #58248):
```python
# TODO : Pretty sure we can drop these last two; verify after testing is complete
dag_id=self.dag_id,
run_id=orm_dagrun.run_id,
```
`DagRunLogicalDateDeadline` and `DagRunQueuedAtDeadline` pass those straight to `_fetch_from_db` (serialization/definitions/deadline.py:361), which queries `SELECT FROM dag_run WHERE dag_id = ? AND run_id = ?`. That is a query for a column of `orm_dagrun`, which is already loaded in the same session. `AverageRuntimeDeadline` genuinely needs to query historical runs, but it only needs `dag_id`, which is already available as `orm_dagrun.dag_id`.
There is a second symptom. models/taskinstance.py:252 cannot use `evaluate_with()` at all:
```
# We can't use evaluate_with() since the new queued_at is not written to the DB yet.
```
and replicates the deadline recalculation instead. That is the DB-read approach failing where you would expect: the in-memory object is ahead of the database, so the `SELECT` returns stale data. Passing the object directly would let that code path use the normal evaluation logic.
### Proposal
Pass the DagRun (or a small evaluation-context object) into `evaluate_with` instead of loose identifiers:
- Built-in references read attributes directly; `_fetch_from_db` is no longer needed for the two DagRun references
- One fewer query per DagRun-type deadline alert per Dag run creation
- taskinstance.py can use `evaluate_with()` rather than duplicating it
- A context object leaves room to expose more without another signature change
### Consequence for required_kwargs
`required_kwargs` exists solely to declare which of these loose kwargs a reference wants forwarded, and the available pool is exactly `{dag_id, run_id}`. If the identifiers go away, `required_kwargs` has nothing left to select and becomes vestigial.
It is also the most confusing part of the custom-reference API. It reads like a way to pass your own configuration in, but declaring anything outside that pool raises ValueError on every evaluation. The docs shipped with that exact mistake (fix is in #70709).
### Compatibility
This is a breaking change to a public extension point: any custom reference using `kwargs["dag_id"]` would break. It needs a deprecation path rather than a straight removal, probably passing both forms for one release, deprecating `required_kwargs`, then removing it.
### Related
- #70706, #70708, #70709 (papercuts found in the same area)
- #58248 (introduced the TODO)
### Committer
- [x] I acknowledge that I am a maintainer/committer of the Apache Airflow project.
Contributor guide
Research direction
Start with the evaluation call in airflow-core/src/airflow/serialization/definitions/dag.py:764, then trace DagRunLogicalDateDeadline, DagRunQueuedAtDeadline, and AverageRuntimeDeadline in serialization/definitions/deadline.py:361. Compare this with the duplicated recalculation in models/taskinstance.py:252; done means an agreed compatibility and deprecation path for custom references, removal of unnecessary database reads, and reuse of the normal evaluation logic.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend-api-design
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100