Allow Variable.get to reuse the caller's database session - #71968
Open
ferruzzi wants to merge 3 commits into
Open
Allow Variable.get to reuse the caller's database session#71968ferruzzi wants to merge 3 commits into
ferruzzi wants to merge 3 commits into
Conversation
`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
marked this pull request as ready for review
August 21, 2026 23:27
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MetastoreBackend.get_variableis decorated with@provide_session. Called without a session, it goes throughcreate_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:prohibit_commitguard: it raisesRuntimeError: UNEXPECTED COMMIT, which is then swallowed per-backend and surfaces as a missing Variable.Variable.get,Variable.get_variable_from_secrets, andVariable.setdefaultnow take an optional keyword-only session, forwarded only toMetastoreBackend.Variable.updateforwards its own.Affected today:
VariableIntervalreturn "Variable not found" despite the Variable existingnext_dagrun_infoleavenext_dagrunNULL, so the Dag is never eligible and never runs, with nothing loggedBoth of the above are reached from
_create_dagruns_for_dags, inside the guard. Dag sync viaupdate_dags,Variable.update(fixed here), andVariable.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.updateandVariable.setdefaultoutright and lets the remaining callers be fixed by passingsession.airflow.sdk.Variable.getandConnection.get_connection_from_secretsshare the same issue and are left as follow-ups, so callers going through those are still affected.Related (that I know of)
sessionthrough itsVariable.get()callcloses: #71801
Was generative AI tooling used to co-author this PR?
Generated-by: Kiro (Claude Opus 5) following the guidelines
{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.