seanghaeli opened a new pull request, #68917:
URL: https://github.com/apache/airflow/pull/68917

   ## Why
   
   This re-introduces the **`VariableInterval` resolution** portion of #66608, 
which was fully reverted in #68909. That revert removed several unrelated 
concerns (callback context delivery, an execution-API token-scope change 
flagged as a security regression, UI work). This PR brings back only the 
self-contained, security-neutral piece: safely resolving a 
`VariableInterval`-backed deadline at DagRun creation.
   
   A `DeadlineAlert` configured with a `VariableInterval` is resolved when the 
scheduler creates a DagRun, inside `DAG._process_dagrun_deadline_alerts` (which 
runs under the `prohibit_commit` guard). Two problems are fixed:
   
   1. **Resolution goes through the full secrets chain** (env vars, configured 
secrets backends, then the metadata DB) via a dedicated 
`_resolve_variable_interval` helper, rather than `Variable.get` / 
`begin_nested`. `Variable.get` and a SAVEPOINT release both commit on the 
scheduler's session, tripping `prohibit_commit` (`UNEXPECTED COMMIT`) and 
silently dropping deadlines for every scheduled DagRun. The helper passes the 
scheduler session through to the metastore backend so the DB read does not 
commit, and reading via the secrets chain (not the `variable` table directly) 
means `AIRFLOW_VAR_*` env vars and secrets-backend-backed Variables resolve too.
   
   2. **Each deadline alert is isolated** with a plain `try`/`except` 
(deliberately **not** `begin_nested`, which would commit a SAVEPOINT and trip 
the same guard). Creating a deadline is auxiliary to creating the DagRun; a 
single bad alert — a missing/invalid backing Variable, or an undecodable 
serialized blob — must never abort the DagRun and stop the DAG from scheduling.
   
   `VariableInterval.resolve` is split into `resolve` + `coerce_to_timedelta` 
so the scheduler-side reader reuses the exact same validation (including the 
`OverflowError` -> `ValueError` translation) without going through 
`Variable.get`.
   
   ## Tests
   
   - `airflow-core/tests/unit/models/test_dagrun.py`: VariableInterval resolves 
from a real Variable row and from an `AIRFLOW_VAR_*` env var; a missing 
Variable and an undecodable alert are isolated (DagRun still created, no 
Deadline row, error logged).
   - `task-sdk/tests/task_sdk/definitions/test_deadline.py`: 
`coerce_to_timedelta` validation (non-integer, `<= 0`, overflow).
   
   Verified locally in Breeze: 8 passed (dagrun deadline/variable) + 14 passed 
(SDK `TestVariableInterval`).
   
   Generated-by: Claude Code (Opus via Claude Code) on behalf of Sean Ghaeli
   


-- 
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]

Reply via email to