seanmuth commented on code in PR #68305:
URL: https://github.com/apache/airflow/pull/68305#discussion_r3483003134
##########
providers/hashicorp/src/airflow/providers/hashicorp/secrets/vault.py:
##########
@@ -237,19 +237,35 @@ def get_connection(self, conn_id: str, team_name: str |
None = None) -> Connecti
:return: A Connection object constructed from Vault data
"""
- # The Connection needs to be locally imported because otherwise we get
into cyclic import
- # problems when instantiating the backend during configuration
- from airflow.models.connection import Connection
+ # Use the compat SDK Connection (Pydantic in Airflow 3, SQLAlchemy in
Airflow 2) to avoid
+ # triggering SQLAlchemy mapper initialization for unrelated models
(e.g. DagModel) in
+ # task-execution subprocesses such as PythonVirtualenvOperator.
+ from airflow.providers.common.compat.sdk import Connection
Review Comment:
Thanks for the detailed review — the approach has been reworked per your
suggestions.
**What changed:**
`get_connection()` is now replaced by a `get_conn_value()` override, which
returns a plain string: the `conn_uri` value verbatim, or
`json.dumps(response)` for field-based secrets. The base-class
`get_connection()` then deserializes that string using
`_get_connection_class()`, which the framework populates per execution context
(ORM Connection on the server, SDK Connection in workers) — fixing the original
mapper-init bug without regressing `airflow connections get`.
**Compat shim for older Airflow (3.0/3.1/2.11):**
On those versions `BaseSecretsBackend.get_connection()` predates both
`team_name` support and the `deserialize_connection` / `_get_connection_class`
pipeline, so `get_conn_value()` alone is not enough. A `get_connection()`
override is kept for those versions: it calls `get_conn_value()` then
`deserialize_connection()` when available, and falls back to the compat SDK
Connection (with the same `hasattr(Connection, "from_uri")` guard) when it is
not.
**Tests:**
`_set_connection_class()` calls are guarded with `hasattr` since the method
did not exist before the shared secrets-backend was introduced. The two
`isinstance(SdkConnection)` assertion tests are skipped on older Airflow where
class injection is unavailable.
Also noted: the `akeyless` backend has the mirror of the original bug
(hard-coded ORM Connection); the same `get_conn_value()` pattern would fix it
there too.
---
Drafted-by: Claude Code (Sonnet 4.6); reviewed by @seanmuth 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]
