Dev-iL commented on code in PR #73414:
URL: https://github.com/apache/airflow/pull/73414#discussion_r4324361637
##########
airflow-core/src/airflow/secrets/metastore.py:
##########
@@ -61,6 +90,36 @@ def get_connection(
session.expunge(conn)
return conn
+ async def aget_connection(
+ self, conn_id: str, team_name: str | None = None, *, session:
AsyncSession | None = None
+ ) -> Connection | None:
+ """Get an Airflow Connection from the metadata DB using native async
I/O."""
+ if session is None:
+ async with create_session_async() as owned_session:
+ return await self.aget_connection(conn_id,
team_name=team_name, session=owned_session)
+
+ from airflow.models import Connection
+
+ row = (
+ (
+ await session.execute(
+ select(Connection.__table__)
Review Comment:
`select(Connection)` hydrates the ORM object and invokes
[`Connection.on_db_load()`](https://github.com/apache/airflow/blob/e23a472a3321505f89d06efa175bccee0a634c3b/airflow-core/src/airflow/models/connection.py#L222)
before the async query returns. That hook reads `password`, which calls
[`get_fernet()`](https://github.com/apache/airflow/blob/e23a472a3321505f89d06efa175bccee0a634c3b/airflow-core/src/airflow/models/crypto.py).
If initialization failed, this retries the key lookup on the event loop;
configuration can invoke a command or a synchronous secrets backend. Expunging
afterward is too late. SQLAlchemy documents the [load-time reconstructor
behavior](https://docs.sqlalchemy.org/en/20/orm/mapping_api.html#sqlalchemy.orm.reconstructor).
The [column
select](https://github.com/apache/airflow/blob/e23a472a3321505f89d06efa175bccee0a634c3b/airflow-core/src/airflow/secrets/metastore.py#L104)
keeps database I/O on the borrowed async session and lets reconstruction run
in a worker while the key is uncached. The session stays on its owning thread.
The existing
[`test_fernet_key_loads_stay_off_event_loop`](https://github.com/apache/airflow/blob/e23a472a3321505f89d06efa175bccee0a634c3b/airflow-core/tests/unit/always/test_secrets_resolver.py#L471)
covers both successful initialization and repeated failure. I recommend
retaining this split.
I checked a local async `select(Connection)` plus `expunge()` alternative
against that regression: the valid-key case passes, but the failed-key case
fails the assertion that every Fernet configuration read happens off the
event-loop thread. The current column loader passes both.
---
Drafted-by: Codex (GPT-6) (reviewed by @Dev-iL before posting)
--
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]
