ferruzzi opened a new issue, #70710:
URL: https://github.com/apache/airflow/issues/70710
### 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 <column> 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 exactly that
mistake
until #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.
--
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]
