shivaam commented on code in PR #63080:
URL: https://github.com/apache/airflow/pull/63080#discussion_r2907260766
##########
airflow-core/src/airflow/secrets/metastore.py:
##########
@@ -57,7 +57,8 @@ def get_connection(
)
.limit(1)
)
- session.expunge_all()
Review Comment:
I don't think there is any reason to use session.expunge_all() in a shared
scoped session. It was probably meant to detach the returned Connection from
the session, but expunge_all() removes everything. Given our session config
(expire_on_commit=False), commit won't expire attributes on the objects, so
there's no risk of error from that.
I think we still need expunge(conn) though. The reason to keep expunge at
all is defensive: since this is a shared scoped session, without it the
returned Connection stays in the identity map. If any caller mutates it, those
changes could get flushed to the DB on the next commit from unrelated code on
the same thread. expunge(conn) makes the object session-free so that can't
happen.
--
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]
