shahar1 commented on code in PR #74766:
URL: https://github.com/apache/airflow/pull/74766#discussion_r4306767718
##########
providers/common/compat/src/airflow/providers/common/compat/connection/__init__.py:
##########
@@ -43,13 +45,27 @@ async def get_async_connection(conn_id: str, hook: BaseHook
| type[BaseHook] | N
hook = hook or BaseHook
hook_name = hook.__name__ if isinstance(hook, type) else
type(hook).__name__
- if hasattr(hook, "aget_connection"):
+ if hasattr(hook, "aget_connection") and not
_aget_connection_masks_synchronously(hook):
log.debug("Get connection using `%s.aget_connection()`.", hook_name)
return await hook.aget_connection(conn_id=conn_id)
log.debug("Get connection using `%s.get_connection()`.", hook_name)
return await sync_to_async(hook.get_connection)(conn_id=conn_id)
+def _aget_connection_masks_synchronously(hook: BaseHook | type[BaseHook]) ->
bool:
+ """
+ Return whether ``hook.aget_connection`` is the base one of an Airflow
older than 3.3.2.
+
+ ``Connection.aextra_dejson`` came with the asynchronous secret masking in
Airflow 3.3.2.
+ """
+ if hasattr(Connection, "aextra_dejson"):
+ return False
+ hook_class = hook if isinstance(hook, type) else type(hook)
+ return getattr(hook_class.aget_connection, "__func__", None) is getattr(
+ BaseHook.aget_connection, "__func__", object()
Review Comment:
**major**: `BaseHook.aget_connection` only exists from Airflow 3.1. On 2.11
and 3.0, `Connection` has no `aextra_dejson` either, so the early return does
not fire. Any hook that defines its own `aget_connection` then fails here with:
```
AttributeError: type object 'BaseHook' has no attribute 'aget_connection'
```
Before this change, that hook's `aget_connection` was awaited. The new
`test_overridden_aget_connection_is_honored` hits exactly this case, and it is
why `Compat 2.11.1` and `Compat 3.0.6` are red ([2.11.1
job](https://github.com/apache/airflow/actions/runs/38638598665/job/116111935154),
[3.0.6
job](https://github.com/apache/airflow/actions/runs/38638598665/job/116111935252)).
One way to fix it: read the base method with `getattr(BaseHook,
"aget_connection", None)` and return `False` when it is `None`. On those
versions the base `BaseHook` has no `aget_connection` that could deadlock.
> On 3.3.2+ and on Airflow 2 nothing changes.
> — PR description
--
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]
