Skip to content

Allow Variable.get to reuse the caller's database session - #71968

Open
ferruzzi wants to merge 3 commits into
apache:mainfrom
aws-mwaa:ferruzzi/plumb-variable-session
Open

Allow Variable.get to reuse the caller's database session#71968
ferruzzi wants to merge 3 commits into
apache:mainfrom
aws-mwaa:ferruzzi/plumb-variable-session

Conversation

@ferruzzi

@ferruzzi ferruzzi commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

MetastoreBackend.get_variable is decorated with @provide_session. Called without a session, it goes through create_session(), which returns the caller's own session for a scoped session and commits it on exit. So any code that reads a Variable while holding a transaction gets that transaction committed underneath it. That surfaces two ways:

  1. Outside the scheduler guard: the commit succeeds early and the caller's ORM objects are detached silently.
  2. Under the scheduler's prohibit_commit guard: it raises RuntimeError: UNEXPECTED COMMIT, which is then swallowed per-backend and surfaces as a missing Variable.

Variable.get, Variable.get_variable_from_secrets, and Variable.setdefault now take an optional keyword-only session, forwarded only to MetastoreBackend. Variable.update forwards its own.

Affected today:

  • Deadline Alerts using VariableInterval return "Variable not found" despite the Variable existing
  • Custom timetables reading a Variable in next_dagrun_info leave next_dagrun NULL, so the Dag is never eligible and never runs, with nothing logged

Both of the above are reached from _create_dagruns_for_dags, inside the guard. Dag sync via update_dags, Variable.update (fixed here), and Variable.setdefault (fixed here) hold a session without the guard, so they fail the silent way instead.

There are likely others; these are the ones I found.

Scope: This fixes the core Variable.update and Variable.setdefault outright and lets the remaining callers be fixed by passing session. airflow.sdk.Variable.get and Connection.get_connection_from_secrets share the same issue and are left as follow-ups, so callers going through those are still affected.

Related (that I know of)

closes: #71801


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Generated-by: Kiro (Claude Opus 5) following the guidelines


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

`MetastoreBackend.get_variable` is decorated with `@provide_session`. Called
without a session it goes through `create_session()`, which for a scoped session
returns *the caller's own session* and commits it on exit. So any code that reads
a Variable while holding a transaction gets that transaction committed underneath
it — detaching its objects, or raising `UNEXPECTED COMMIT` under the scheduler's
`prohibit_commit` guard, where the error is then swallowed per-backend and
surfaces as a missing Variable.

`Variable.get` and `Variable.get_variable_from_secrets` now take an optional
keyword-only `session`, forwarded only to `MetastoreBackend`. `Variable.update`
forwards its own.

Affected today, all reached from `_create_dagruns_for_dags` inside the guard:

* Deadline Alerts using `VariableInterval`
* Custom timetables reading a Variable in `next_dagrun_info` — `next_dagrun` is
left NULL, so the Dag is never eligible and never runs, with nothing logged
* Dag sync via `update_dags`; `Variable.update`; `Variable.setdefault`

Scope: this fixes the core read path only. `airflow.sdk.Variable.get` and
`Connection.get_connection_from_secrets` share the defect and are unchanged, so
callers going through those are still affected. apache#68917 can drop its duplicated
backend walk once this lands.

closes: apache#71801
@ferruzzi
ferruzzi marked this pull request as ready for review August 21, 2026 23:27
@ferruzzi
ferruzzi requested review from XD-DENG and ashb as code owners August 21, 2026 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow Variable.get_variable_from_secrets to reuse a caller's session

1 participant