o-nikolas commented on code in PR #71968:
URL: https://github.com/apache/airflow/pull/71968#discussion_r3848158886
##########
airflow-core/newsfragments/71968.bugfix.rst:
##########
@@ -0,0 +1 @@
+``Variable.get``, ``Variable.get_variable_from_secrets`` and
``Variable.setdefault`` now accept an optional keyword-only ``session`` that is
forwarded to the metastore secrets backend, so a lookup made while holding an
open session (most notably inside the scheduler's ``prohibit_commit`` guard)
reuses the caller's transaction instead of opening a scoped session and
committing it; ``Variable.update`` now forwards its session to its own
existence check, which had the same problem.
Review Comment:
```suggestion
``Variable.get``, ``Variable.get_variable_from_secrets`` and
``Variable.setdefault`` now accept an optional keyword-only ``session`` that is
forwarded to the metastore secrets backend. A lookup made while holding an open
session (most notably inside the scheduler's ``prohibit_commit`` guard) now
reuses the caller's transaction instead of opening a scoped session and
committing it. ``Variable.update`` now forwards its session to its own
existence check, which had the same problem.
```
##########
airflow-core/src/airflow/models/variable.py:
##########
@@ -485,9 +515,17 @@ def get_variable_from_secrets(key: str, team_name: str |
None = None) -> str | N
var_val = None
# iterate over backends if not in cache (or expired)
for secrets_backend in ensure_secrets_loaded():
+ # Only the metastore backend touches the metadata database, and it
is the only
Review Comment:
Nit: This comment line confused me at first. I think this clarifies it a
bit.
```suggestion
# Only the metastore variable backend touches the metadata
database, and it is the only
```
--
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]