kaxil commented on code in PR #68305:
URL: https://github.com/apache/airflow/pull/68305#discussion_r3383520046


##########
providers/hashicorp/src/airflow/providers/hashicorp/secrets/vault.py:
##########
@@ -237,19 +237,27 @@ 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
 
         response = self._get_team_or_global_secret(self.connections_path, 
team_name, conn_id)
         if response is None:
             return None
 
         uri = response.get("conn_uri")
         if uri:
-            return Connection(conn_id, uri=uri)
+            # In Airflow 3 the SDK Connection's __init__ is keyword-only and 
attrs-generated;
+            # uri= is not in the attrs fields so we use the classmethod 
directly.
+            # In Airflow 2 the SQLAlchemy Connection accepts uri= in its 
__init__.
+            from airflow.providers.common.compat.version_compat import 
AIRFLOW_V_3_0_PLUS
 
-        return Connection(conn_id, **response)
+            if AIRFLOW_V_3_0_PLUS:
+                return Connection.from_uri(uri, conn_id=conn_id)  # type: 
ignore[attr-defined]

Review Comment:
   `from_uri` only exists on the SDK Connection starting with task-sdk 1.2.0 / 
Airflow 3.2 (added in #55972). This provider supports `apache-airflow>=2.11.0`, 
so on Airflow 3.0/3.1 this line raises `AttributeError` (the attrs Connection 
there has neither `from_uri` nor a `uri=` kwarg), and the secrets-backend loop 
in `execution_time/context.py` swallows it at DEBUG level, the same silent 
failure this PR is fixing.
   
   The shared secrets base uses `hasattr(conn_class, "from_uri")` as the 
discriminator for exactly this case (`_deserialize_connection_value` in 
`airflow_shared/secrets_backend/base.py`), which would be a safer gate than the 
Airflow version check. 3.0/3.1 would still need an explicit fallback (or a 
comment noting that `conn_uri`-style secrets remain broken there).
   
   The comment above is also slightly off for 3.2+: since #62211 the SDK 
Connection has a hand-written `__init__` that does accept `uri=`, so it's only 
3.0/3.1 where neither spelling works.



##########
providers/hashicorp/src/airflow/providers/hashicorp/secrets/vault.py:
##########
@@ -237,19 +237,27 @@ 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
 
         response = self._get_team_or_global_secret(self.connections_path, 
team_name, conn_id)
         if response is None:
             return None
 
         uri = response.get("conn_uri")
         if uri:
-            return Connection(conn_id, uri=uri)
+            # In Airflow 3 the SDK Connection's __init__ is keyword-only and 
attrs-generated;
+            # uri= is not in the attrs fields so we use the classmethod 
directly.
+            # In Airflow 2 the SQLAlchemy Connection accepts uri= in its 
__init__.
+            from airflow.providers.common.compat.version_compat import 
AIRFLOW_V_3_0_PLUS

Review Comment:
   If the version check stays, this import can move to the top of the file next 
to the existing `compat.sdk` import. The circular-import concern in the old 
comment was about the Connection model, not `version_compat`.



-- 
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