This is an automated email from the ASF dual-hosted git repository.
kaxil pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 6a6a24f2e1a Remove a misleading connection type warning and explain
lookup failures (#72855)
6a6a24f2e1a is described below
commit 6a6a24f2e1a10ac51c9a0c5910cb08b21d9121d2
Author: Kaxil Naik <[email protected]>
AuthorDate: Thu Sep 10 23:16:33 2026 +0100
Remove a misleading connection type warning and explain lookup failures
(#72855)
* Warn about a connection type that cannot round-trip, not one that can
`Connection.get_uri()` warned whenever `conn_type` contained '_', citing
RFC 3986.
The check was inverted. RFC 3986 forbids '_' in a URI scheme, so
`get_uri()` encodes
'_' as '-' and `_normalize_conn_type` decodes it back on read; the
underscore form is
therefore the canonical one, required for the hook-registry key, and the
URI it emits
is already compliant. The configuration that genuinely breaks is the
opposite one: a
literal '-' in `conn_type` is indistinguishable from an encoded '_' once
serialized,
so it decodes to the underscore form on read and the hook registered under
the
hyphenated name is never found.
The warning therefore fired on correct configuration, for 35 connection
types in the
provider tree, once per uncached connection fetch, and stayed silent on the
broken
kind. Its `_prenormalized_conn_type` guard could never help either: an
underscore
scheme parses with no scheme at all, so that attribute can never contain
'_'.
Inverts the condition to flag '-', and names the type the connection will
actually
resolve to so the message says what to change.
Also replaces the `Unknown hook type ""` reported for such a URI, in both
the model
and the Task SDK copy, with a message that names the empty connection type
and the
scheme rule that caused it.
* Explain a hyphenated connection type when its hook cannot be found
`get_hook()` gained an explanation for an empty `conn_type`, but a
`conn_type` set
directly with a hyphen fell through to the bare `Unknown hook type "..."`.
That is the
configuration which can never resolve, since '-' is the URI-scheme encoding
of '_', and
it is reached by exactly the connections that never round-trip through
`get_uri()`:
metadata-DB rows and object-form imports.
The message now names the registered spelling it is likely looking for. It
is worded as
a hint rather than a diagnosis, because the runtime provider schema does
not forbid a
hyphenated `connection-type`, so an out-of-tree provider could legitimately
register
one and "the provider is not installed" remains a possible cause.
Applied to both the model and the Task SDK copy, since both raise it.
* Name the registered connection type when a hook lookup fails
The previous message asserted that a connection type spelled with '-'
cannot be resolved. That is not true: hooks register under the
connection-type from provider.yaml verbatim, so a hyphenated type does
register and does resolve for a connection built with that type
directly.
The hyphen only breaks resolution once a connection has been stored,
because a URI scheme encodes '_' as '-' and reading a connection back
from a URI or from JSON decodes it again. That normalization runs in the
opposite direction from what the message described, so the message never
appeared for the failure that motivated it -- a connection for a
hyphenated connection-type fails as an unknown *underscored* type.
Look the other spelling up in the registry instead of guessing at it,
which covers both directions and says nothing that is not known:
Unknown hook type "pydanticai_vertex", but a hook is registered for
'pydanticai-vertex'. No stored connection can reach that hook, because
'-' is decoded back to '_' whenever a connection is read from a URI or
from JSON, so the provider has to declare its connection-type with '_'.
With neither spelling registered, the message stays as it was.
* Remove the connection type warning from get_uri instead of inverting it
get_uri() serializes a connection; it does not validate one. The warning
it carried said a conn_type containing '_' broke RFC 3986, which is the
one spelling that survives a round trip: get_uri() encodes '_' as '-'
because a URI scheme cannot contain '_', and reading a connection back
decodes it. Every URI it emits is compliant by construction, so the rule
being cited was never the one at stake.
Inverting the condition to flag '-' says something true but says it in
the wrong place. It fires on every uncached connection fetch, repeats for
the life of the deployment, and is addressed to whoever reads the logs
rather than to the provider author who declares the connection-type. A
connection type spelled with '-' is not merely discouraged, it is
unrepresentable, and an invariant of that kind belongs in validation.
It now has two better homes. get_hook() reports it where it fails, naming
the registered spelling it looked up, and the provider schema rejects a
hyphenated connection-type at authoring time.
The test that pinned the warning becomes one that pins its absence, so
the canonical underscore form cannot start warning again.
* Only name the other connection type spelling when its hook resolves
ProvidersManager.hooks maps a connection type to None when its hook
cannot be imported, so testing for membership was not enough to claim the
other spelling would work. A connection type of google-cloud-platform
whose google_cloud_platform hook fails to import produced advice that
still raised Unknown hook type when followed, which is worse than no
advice.
Resolve the alternative rather than testing for membership, and name it
only when it yields a hook. Resolution is already what the primary lookup
one line earlier does, it happens only on the failing path, and it cannot
raise: the discovery code returns None for a hook it could not import.
---
airflow-core/src/airflow/models/connection.py | 47 ++++--
airflow-core/tests/unit/models/test_connection.py | 159 ++++++++++++++++-----
task-sdk/src/airflow/sdk/definitions/connection.py | 40 +++++-
.../tests/task_sdk/definitions/test_connection.py | 67 +++++++++
4 files changed, 263 insertions(+), 50 deletions(-)
diff --git a/airflow-core/src/airflow/models/connection.py
b/airflow-core/src/airflow/models/connection.py
index 1b4b0f8f867..4424316c225 100644
--- a/airflow-core/src/airflow/models/connection.py
+++ b/airflow-core/src/airflow/models/connection.py
@@ -287,13 +287,6 @@ class Connection(Base, FernetFieldsMixin, LoggingMixin):
Note that the URI returned by this method is **not**
SQLAlchemy-compatible, if you need a SQLAlchemy-compatible URI, use the
:attr:`~airflow.providers.common.sql.hooks.sql.DbApiHook.sqlalchemy_url`
"""
- conn_type = getattr(self, "_prenormalized_conn_type", self.conn_type)
or ""
- if "_" in conn_type:
- self.log.warning(
- "Connection schemes (type: %s) shall not contain '_' according
to RFC3986.",
- conn_type,
- )
-
if self.conn_type:
uri = f"{self.conn_type.lower().replace('_', '-')}://"
else:
@@ -390,10 +383,46 @@ class Connection(Base, FernetFieldsMixin, LoggingMixin):
"""Return hook based on conn_type."""
from airflow.providers_manager import ProvidersManager
- hook = ProvidersManager().hooks.get(self.conn_type, None)
+ hooks = ProvidersManager().hooks
+ hook = hooks.get(self.conn_type, None)
if hook is None:
- raise AirflowException(f'Unknown hook type "{self.conn_type}"')
+ if not self.conn_type:
+ # A URI scheme cannot contain '_' (RFC 3986), so "foo_bar://h"
parses with no
+ # scheme at all and leaves conn_type empty. Name that, instead
of reporting an
+ # unknown hook type of "".
+ message = (
+ f"Connection {self.conn_id!r} has no connection type, so
no hook could be "
+ "looked up. If it was defined as a URI, note that a URI
scheme cannot "
+ "contain '_' (RFC 3986) and such a URI parses with no
scheme at all: use "
+ "'-' in the URI instead, which is decoded back to '_' on
read."
+ )
+ else:
+ message = f'Unknown hook type "{self.conn_type}"'
+ # get_uri() encodes '_' as '-' because RFC 3986 forbids '_' in
a scheme, and
+ # reading a connection back from a URI or from JSON decodes it
again, so both
+ # characters serialize to '-' and a connection type spelled
one way cannot
+ # resolve a hook registered the other way. Hooks register
under the
+ # connection-type verbatim, so look the other spelling up
rather than asserting
+ # which one is right, and resolve it rather than testing for
membership: a
+ # registered connection type maps to None when its hook cannot
be imported, and
+ # naming a spelling that still will not resolve is worse than
naming none.
+ alternative = (
+ self.conn_type.replace("-", "_")
+ if "-" in self.conn_type
+ else self.conn_type.replace("_", "-")
+ )
+ if alternative != self.conn_type and hooks.get(alternative) is
not None:
+ message += f", but a hook is registered for
{alternative!r}. "
+ if "-" in self.conn_type:
+ message += "Spell this connection's type with '_' to
reach it."
+ else:
+ message += (
+ "Reading a connection from a URI or from JSON
decodes '-' back to "
+ "'_', so this connection cannot reach that hook;
the provider has "
+ "to declare its connection-type with '_'."
+ )
+ raise AirflowException(message)
try:
hook_class = import_string(hook.hook_class_name)
except ImportError:
diff --git a/airflow-core/tests/unit/models/test_connection.py
b/airflow-core/tests/unit/models/test_connection.py
index 94cabe5e4da..e7f6cbf6cb1 100644
--- a/airflow-core/tests/unit/models/test_connection.py
+++ b/airflow-core/tests/unit/models/test_connection.py
@@ -272,53 +272,134 @@ class TestConnection:
assert connection.get_uri() == expected_uri
@pytest.mark.parametrize(
- ("connection", "expected_warned"),
+ ("conn_type", "registered", "expected_remedy"),
[
- (Connection(conn_id="test-uri-1",
uri="google-cloud-platform://testlogin:testpassword@"), False),
- (Connection(conn_id="test-uri-2", uri="amazon://test:test@"),
False),
- (
- Connection(
- conn_id="test-non-uri-1",
- conn_type="google-cloud-platform",
- login="testlogin",
- password="testpassword",
- ),
- False,
+ # Typed with '-' where the provider registers '_': the connection
is the thing to
+ # fix, so say which spelling reaches the hook.
+ pytest.param(
+ "google-cloud-platform",
+ "google_cloud_platform",
+ "Spell this connection's type with '_' to reach it.",
+ id="hyphenated-conn-type",
),
- (
- Connection(
- conn_id="test-non-uri-2",
- conn_type="google_cloud_platform",
- login="testlogin",
- password="testpassword",
- ),
- True,
+ # The reverse, which is what a hyphenated connection-type actually
produces: a
+ # connection read from a URI or from JSON normalizes to '_' and
cannot reach the
+ # registered name, so there is nothing that connection can do and
the provider
+ # has to rename. A metadata-DB row keeps the hyphen and does
resolve.
+ pytest.param(
+ "pydanticai_vertex",
+ "pydanticai-vertex",
+ "so this connection cannot reach that hook",
+ id="hyphenated-registration",
),
- (
- Connection(
- conn_id="test-non-uri-3", conn_type="amazon",
login="testlogin", password="testpassword"
- ),
- False,
+ ],
+ )
+ def test_get_hook_names_the_other_spelling_when_it_is_the_registered_one(
+ self, conn_type, registered, expected_remedy
+ ):
+ """
+ '-' and '_' are the same character to a URI scheme, so a connection
type spelled one
+ way cannot resolve a hook registered the other way. The bare 'Unknown
hook type' named
+ neither the cause nor which spelling would work.
+ """
+ conn = Connection(conn_id="c", conn_type=conn_type)
+
+ with mock.patch("airflow.providers_manager.ProvidersManager") as
mock_manager:
+ mock_manager.return_value.hooks = {registered: mock.MagicMock()}
+ with pytest.raises(AirflowException, match="Unknown hook type") as
exc_info:
+ conn.get_hook()
+
+ message = str(exc_info.value)
+ assert conn_type in message
+ assert registered in message
+ assert expected_remedy in message
+
+ def test_get_hook_does_not_guess_an_unregistered_spelling(self):
+ """
+ A hyphenated connection-type registers verbatim and resolves for a
connection built
+ with that type directly, so an unresolved hyphenated type is not
evidence that the
+ underscored name exists. With nothing registered either way, say only
what is known.
+ """
+ conn = Connection(conn_id="c", conn_type="google-cloud-platform")
+
+ with mock.patch("airflow.providers_manager.ProvidersManager") as
mock_manager:
+ mock_manager.return_value.hooks = {}
+ with pytest.raises(AirflowException) as exc_info:
+ conn.get_hook()
+
+ assert str(exc_info.value) == 'Unknown hook type
"google-cloud-platform"'
+
+ def
test_get_hook_does_not_advise_a_spelling_whose_hook_cannot_be_imported(self):
+ """
+ ProvidersManager.hooks holds None for a connection type whose hook
could not be
+ imported, so membership alone does not mean the other spelling would
resolve. Advice
+ that still fails when followed is worse than no advice.
+ """
+ conn = Connection(conn_id="c", conn_type="google-cloud-platform")
+
+ with mock.patch("airflow.providers_manager.ProvidersManager") as
mock_manager:
+ mock_manager.return_value.hooks = {"google_cloud_platform": None}
+ with pytest.raises(AirflowException) as exc_info:
+ conn.get_hook()
+
+ assert str(exc_info.value) == 'Unknown hook type
"google-cloud-platform"'
+
+ def test_get_hook_explains_a_uri_whose_scheme_was_dropped(self):
+ """
+ A URI scheme cannot contain '_' (RFC 3986), so ``foo_bar://h`` parses
with no scheme
+ at all and leaves conn_type empty. The resulting failure used to read
+ ``Unknown hook type ""``, which named neither the cause nor the fix.
+ """
+ conn = Connection(conn_id="c", uri="pydanticai_azure://h")
+ assert conn.conn_type == ""
+
+ with pytest.raises(AirflowException, match="has no connection type")
as exc_info:
+ conn.get_hook()
+
+ assert "RFC 3986" in str(exc_info.value)
+
+ @pytest.mark.parametrize(
+ "connection",
+ [
+ # Parsed from a URI, so _normalize_conn_type has already decoded
'-' to '_'.
+ Connection(conn_id="test-uri-1",
uri="google-cloud-platform://testlogin:testpassword@"),
+ Connection(conn_id="test-uri-2", uri="amazon://test:test@"),
+ # Set directly with a hyphen, which is the spelling that cannot
round-trip.
+ Connection(
+ conn_id="test-non-uri-1",
+ conn_type="google-cloud-platform",
+ login="testlogin",
+ password="testpassword",
+ ),
+ # The canonical underscore form, which serializes to
'google-cloud-platform://'
+ # and decodes back unchanged.
+ Connection(
+ conn_id="test-non-uri-2",
+ conn_type="google_cloud_platform",
+ login="testlogin",
+ password="testpassword",
+ ),
+ Connection(
+ conn_id="test-non-uri-3", conn_type="amazon",
login="testlogin", password="testpassword"
),
],
)
- def test_get_uri_conn_type_warning(self, connection: Connection,
expected_warned: bool):
+ def test_get_uri_does_not_warn_about_the_connection_type(self, connection:
Connection):
+ """
+ get_uri() serializes a connection; it does not validate one.
+
+ It used to warn that a conn_type containing '_' broke RFC 3986. That
is the one
+ spelling which survives the round trip, since get_uri() encodes '_' as
'-' and
+ reading a connection back decodes it, and the warning fired on every
uncached
+ connection fetch. A connection type that cannot round-trip is reported
where it
+ fails, by get_hook(), and is rejected by the provider schema.
+ """
with capture_logs() as captured_logs:
connection.get_uri()
- conn_type_warnings = list(
- filter(
- lambda captured_log: (
- captured_log["log_level"] == "warning" and "RFC3986" in
captured_log["event"]
- ),
- captured_logs,
- )
- )
- if expected_warned:
- assert conn_type_warnings, f"RFC3986 warning expected for
connection '{connection.conn_id}'."
- else:
- assert not conn_type_warnings, (
- f"RFC3986 warning not expected for connection
'{connection.conn_id}'."
- )
+
+ assert [
+ captured_log for captured_log in captured_logs if
captured_log["log_level"] == "warning"
+ ] == []
@pytest.mark.parametrize(
("connection", "expected_conn_id"),
diff --git a/task-sdk/src/airflow/sdk/definitions/connection.py
b/task-sdk/src/airflow/sdk/definitions/connection.py
index 8e9ab232a7c..ba70dca3359 100644
--- a/task-sdk/src/airflow/sdk/definitions/connection.py
+++ b/task-sdk/src/airflow/sdk/definitions/connection.py
@@ -233,10 +233,46 @@ class Connection:
"""Return hook based on conn_type."""
from airflow.sdk._shared.module_loading import import_string
- hook = ProvidersManagerTaskRuntime().hooks.get(self.conn_type, None)
+ hooks = ProvidersManagerTaskRuntime().hooks
+ hook = hooks.get(self.conn_type, None)
if hook is None:
- raise AirflowException(f'Unknown hook type "{self.conn_type}"')
+ if not self.conn_type:
+ # A URI scheme cannot contain '_' (RFC 3986), so "foo_bar://h"
parses with no
+ # scheme at all and leaves conn_type empty. Name that, instead
of reporting an
+ # unknown hook type of "".
+ message = (
+ f"Connection {self.conn_id!r} has no connection type, so
no hook could be "
+ "looked up. If it was defined as a URI, note that a URI
scheme cannot "
+ "contain '_' (RFC 3986) and such a URI parses with no
scheme at all: use "
+ "'-' in the URI instead, which is decoded back to '_' on
read."
+ )
+ else:
+ message = f'Unknown hook type "{self.conn_type}"'
+ # get_uri() encodes '_' as '-' because RFC 3986 forbids '_' in
a scheme, and
+ # reading a connection back from a URI or from JSON decodes it
again, so both
+ # characters serialize to '-' and a connection type spelled
one way cannot
+ # resolve a hook registered the other way. Hooks register
under the
+ # connection-type verbatim, so look the other spelling up
rather than asserting
+ # which one is right, and resolve it rather than testing for
membership: a
+ # registered connection type maps to None when its hook cannot
be imported, and
+ # naming a spelling that still will not resolve is worse than
naming none.
+ alternative = (
+ self.conn_type.replace("-", "_")
+ if "-" in self.conn_type
+ else self.conn_type.replace("_", "-")
+ )
+ if alternative != self.conn_type and hooks.get(alternative) is
not None:
+ message += f", but a hook is registered for
{alternative!r}. "
+ if "-" in self.conn_type:
+ message += "Spell this connection's type with '_' to
reach it."
+ else:
+ message += (
+ "Reading a connection from a URI or from JSON
decodes '-' back to "
+ "'_', so this connection cannot reach that hook;
the provider has "
+ "to declare its connection-type with '_'."
+ )
+ raise AirflowException(message)
try:
hook_class = import_string(hook.hook_class_name)
except ImportError:
diff --git a/task-sdk/tests/task_sdk/definitions/test_connection.py
b/task-sdk/tests/task_sdk/definitions/test_connection.py
index d471cccfcf6..aad4c8f46db 100644
--- a/task-sdk/tests/task_sdk/definitions/test_connection.py
+++ b/task-sdk/tests/task_sdk/definitions/test_connection.py
@@ -78,6 +78,73 @@ class TestConnections:
with pytest.raises(AirflowException, match='Unknown hook type
"unknown_type"'):
conn.get_hook()
+ @pytest.mark.parametrize(
+ ("conn_type", "registered", "expected_remedy"),
+ [
+ pytest.param(
+ "google-cloud-platform",
+ "google_cloud_platform",
+ "Spell this connection's type with '_' to reach it.",
+ id="hyphenated-conn-type",
+ ),
+ pytest.param(
+ "pydanticai_vertex",
+ "pydanticai-vertex",
+ "so this connection cannot reach that hook",
+ id="hyphenated-registration",
+ ),
+ ],
+ )
+ def test_get_hook_names_the_other_spelling_when_it_is_the_registered_one(
+ self, mock_providers_manager, conn_type, registered, expected_remedy
+ ):
+ """Worker-side copy: this is the path a task actually raises from."""
+ mock_providers_manager.return_value.hooks = {registered:
mock.MagicMock()}
+ conn = Connection(conn_id="test_conn", conn_type=conn_type)
+
+ with pytest.raises(AirflowException, match="Unknown hook type") as
exc_info:
+ conn.get_hook()
+
+ message = str(exc_info.value)
+ assert conn_type in message
+ assert registered in message
+ assert expected_remedy in message
+
+ def test_get_hook_does_not_guess_an_unregistered_spelling(self,
mock_providers_manager):
+ """With neither spelling registered, say only what is known."""
+ mock_providers_manager.return_value.hooks = {}
+ conn = Connection(conn_id="test_conn",
conn_type="google-cloud-platform")
+
+ with pytest.raises(AirflowException) as exc_info:
+ conn.get_hook()
+
+ assert str(exc_info.value) == 'Unknown hook type
"google-cloud-platform"'
+
+ def
test_get_hook_does_not_advise_a_spelling_whose_hook_cannot_be_imported(self,
mock_providers_manager):
+ """A registered connection type maps to None when its hook cannot be
imported."""
+ mock_providers_manager.return_value.hooks = {"google_cloud_platform":
None}
+ conn = Connection(conn_id="test_conn",
conn_type="google-cloud-platform")
+
+ with pytest.raises(AirflowException) as exc_info:
+ conn.get_hook()
+
+ assert str(exc_info.value) == 'Unknown hook type
"google-cloud-platform"'
+
+ def test_get_hook_explains_a_uri_whose_scheme_was_dropped(self,
mock_providers_manager):
+ """
+ A URI scheme cannot contain '_' (RFC 3986), so ``foo_bar://h`` parses
with no scheme at
+ all and leaves conn_type empty. This is the worker-side copy of that
failure, which
+ used to read ``Unknown hook type ""`` and named neither the cause nor
the fix.
+ """
+ mock_providers_manager.return_value.hooks = {}
+ conn = Connection(conn_id="test_conn", uri="pydanticai_azure://h")
+ assert conn.conn_type == ""
+
+ with pytest.raises(AirflowException, match="has no connection type")
as exc_info:
+ conn.get_hook()
+
+ assert "RFC 3986" in str(exc_info.value)
+
def test_get_uri(self):
"""Test that get_uri generates the correct URI based on connection
attributes."""
