amoghrajesh commented on code in PR #68305:
URL: https://github.com/apache/airflow/pull/68305#discussion_r3433375578
##########
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:
`_get_connection_class()` is the correct mechanism here as you mentioned.
The framework already injects the right connection class per execution context
and we should use that instead of pinning to `compat.sdk.Connection`. Thanks
for bringing that up. That solution is fine for now.
But I think the proper way to do this is to migrate `VaultBackend` to
override `get_conn_value()` instead of `get_connection()`, returning a json
string for the field based case and a URI string for the `conn_uri` case. The
base `get_connection()` already calls `deserialize_connection()` which calls
`_get_connection_class()`, so class injection would work automatically. That
also removes the `hasattr(Connection, "from_uri")` guard here since
`_deserialize_connection_value` already handles it.
--
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]
