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]

Reply via email to