potiuk commented on code in PR #71976:
URL: https://github.com/apache/airflow/pull/71976#discussion_r3978105953
##########
providers/influxdb/src/airflow/providers/influxdb/hooks/influxdb3.py:
##########
@@ -148,7 +154,14 @@ def get_conn(self) -> InfluxDBClient3:
- Connection password field (as fallback for token)
- Connection extras JSON (for manual configuration)
"""
- self.connection = self.get_connection(self.influxdb3_conn_id)
+ return self._create_client(self.get_connection(self.influxdb3_conn_id))
+
+ async def aget_conn(self) -> InfluxDBClient3:
+ """Initiate a new InfluxDB 3.x connection asynchronously."""
+ return self._create_client(await
self.aget_connection(self.influxdb3_conn_id))
Review Comment:
`aget_connection` is not available on every Airflow version this provider
supports, so this raises `AttributeError` in the triggerer on the oldest one.
The chain:
- `pyproject.toml` declares `apache-airflow>=2.11.0`.
- The `BaseHook` you inherit comes from
`airflow.providers.common.compat.sdk`, which maps it to `airflow.sdk` on
Airflow 3 and falls back to `airflow.hooks.base` on Airflow 2.
- `aget_connection` was added to the Task SDK `BaseHook` by #53831 (AIP-86,
async notifiers). Airflow 2.11's `airflow.hooks.base.BaseHook` predates it and
has no such method.
So `deferrable=True` works on Airflow 3 and fails on 2.11, which is the
version floor in your own `pyproject.toml`.
The tests don't catch this because they create the attribute themselves.
`test_aget_conn` does `self.influxdb3_hook.aget_connection =
mock.AsyncMock(return_value=self.connection)`, which succeeds whether or not
the base class defines it, so the suite is green on any Airflow version.
The fix is the compat helper that exists for exactly this case — added in
#57143, titled "backwards comp get async conn". It does the `hasattr` probe and
falls back to `sync_to_async(hook.get_connection)`, and passing `hook=self`
keeps any subclass override honoured:
```suggestion
async def aget_conn(self) -> InfluxDBClient3:
"""Initiate a new InfluxDB 3.x connection asynchronously."""
return self._create_client(await
get_async_connection(self.influxdb3_conn_id, hook=self))
```
with `from airflow.providers.common.compat.connection import
get_async_connection` at the top. You already depend on
`apache-airflow-providers-common-compat>=1.8.0`, so nothing new is needed.
`sftp/hooks/sftp.py:829` uses it the same way.
One piece of counter-evidence you should have: `smtp/hooks/smtp.py:162`
calls `self.aget_connection(...)` directly with the same `>=2.11.0` floor. I
read that as a latent bug in the SMTP provider rather than a green light, but
you may know differently — if `aget_connection` is in fact reachable on 2.11
through some path I've missed, say so and I'll withdraw this.
--
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]
