Repository navigation
Remove a misleading connection type warning and explain lookup failures - #72855
Merged
Merged
Conversation
kaxil
force-pushed
the
conn-type-diagnostics
branch
from
September 10, 2026 01:07
7ec6430 to
e66ed62
Compare
`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.
kaxil
force-pushed
the
conn-type-diagnostics
branch
from
September 10, 2026 01:10
e66ed62 to
3cb196d
Compare
kaxil
marked this pull request as ready for review
September 10, 2026 13:35
pankajastro
reviewed
Sep 10, 2026
pankajastro
approved these changes
Sep 10, 2026
`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.
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.
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.
kaxil
changed the title
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.
imrichardwu
pushed a commit
to imrichardwu/airflow
that referenced
this pull request
Sep 11, 2026
…es (apache#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.
xvega
pushed a commit
to xvega/airflow
that referenced
this pull request
Sep 13, 2026
…es (apache#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.
dlactin
pushed a commit
to dlactin/airflow
that referenced
this pull request
Sep 14, 2026
…es (apache#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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Connection.get_uri()warned that aconn_typecontaining_violates RFC 3986. That is the one spelling which survives a round trip, so the warning fired on the correct form and stayed silent on the broken one. This removes it, and reports a connection type that cannot resolve at the point where it actually fails.The mechanics
A URI scheme cannot contain
_(RFC 3986 section 3.1 permits ALPHA, DIGIT,+,-and.), soget_uri()writes the scheme asconn_type.lower().replace('_', '-')and_normalize_conn_typedecodes-back to_on read. That substitution covers the one forbidden character Airflow itself introduces, and it is a substitution rather than validation: a connection type containing a space or a leading digit still yields a scheme that is not compliant. The encoding is also lossy in one direction:conn_typegoogle_cloud_platformgoogle-cloud-platform://google_cloud_platform, unchangedgoogle-cloud-platformgoogle-cloud-platform://google_cloud_platform, changedBoth rows emit the same URI.
-and_collapse onto-on the way out and the decode can only pick one preimage, so a connection type spelled with-is not discouraged, it is unrepresentable: it returns as a different type, and a hook registered under the hyphenated name is then not found.RFC 3986 is why the encoding exists. It is not why either spelling fails.
Why the warning goes rather than flipping
The old guard read
_prenormalized_conn_typewhen the connection was parsed from a URI, andconn_typeotherwise. That attribute holds a parsed URI scheme, and an underscore scheme parses with no scheme at all:So it could suppress the warning but never trigger it. A connection parsed from a URI never warned; the warning fired for a metadata-DB row,
from_json, or kwargs. Sinceget_uri()removes every_before the URI exists, it was warning about a violation it had already prevented, on the input side of its own encoder.32 registered connection types contain
_, and the warning was not gated on the secrets cache.conn.get_uri()is evaluated as an argument tosave_connection_uri()and the cache-enabled check sits inside_save, so it ran after every successful backend fetch even where the cache was never initialized.SecretCache.init()is called only by the dag processor.It also advised the wrong thing. #62817 renamed
pydantic_aitopydanticaispecifically to silence it, and the vendor types added in #62816 then followed the same instinct and used hyphens, which is the bug fixed in #72853.Inverting the condition to flag
-would say something true in the wrong place. It fires per fetch for what is static configuration, and its audience is whoever reads the logs rather than the provider author who declares theconnection-type. An invariant with no valid instance belongs in validation, and it now has two better homes: the failure is explained where it fails, and #72853 rejects a hyphenatedconnection-typein the authoring schema.What replaces it
get_hook()explained neither of its two failures.An underscore scheme parses with no scheme, which left
conn_typeempty and surfaced asUnknown hook type "". It now names the empty connection type and the scheme rule behind it.A lookup that misses now names the other spelling when that is the registered one. Hooks register under the provider's
connection-typeverbatim, so a hyphenated type does register and does resolve for a connection built with that type directly; the two spellings only diverge once a connection is read back:The first is #72316's own failure, which reports the underscored form because the connection normalized on the way in. The other spelling is resolved in the registry rather than guessed at, and named only when it yields a usable hook. A registered connection type maps to None when its hook cannot be imported, so mere presence would have produced advice that still fails when followed. With neither spelling usable the message is unchanged. Both
airflow.models.Connectionand the Task SDK copy raise this, so both carry it. The existingraiseis reused rather than adding one, to respectcheck-no-new-airflow-exceptions.Alternatives considered
Allowing
-requires the decode to stop rewriting it, which breaks all 32 underscore types to save the 3 hyphenated ones. Not encoding on write emitsgoogle_cloud_platform://, whichurlsplitreads as having no scheme, soconn_typearrives empty. An encoding that escapes both characters rather than mapping one onto the other would let both spellings coexist, and would change the URI spelling of every connection already written down. Mapping_to--alone does not achieve it, sincefoo_barandfoo--barthen collide. Decoding conditionally, rewriting-only when the hyphenated name is absent from the registry, would fix #72316 with no rename, at the cost of consultingProvidersManagerduring connection parsing and making the same URI mean different things depending on which providers happen to be installed.Relationship to #72853
The three hyphenated
connection-typevalues in the tree arepydanticai-azure,pydanticai-bedrockandpydanticai-vertex, andProvidersManagerregisters those strings verbatim. How they fail depends on where the connection is stored. From a secrets backend serving a URI or JSON they never resolve, becauseConnection(uri=...)andfrom_jsonboth normalize; a custom backend that constructs aConnectionitself is the exception, since it bypasses both parsers. From a metadata-DB row the hyphen survives into the object and the hook does resolve, until the secrets cache is enabled and a cache hit rebuilds the connection from the cached URI. #72853 renames all three and forbids a hyphenatedconnection-typein the authoring schema. This PR is independent of it and does not depend on the order the two merge.Known gap this does not close
Where two providers register the two spellings of one name, a connection resolves a different
provider's hook after a round trip, and nothing raises:
get_hook()only speaks when a lookup fails, so no message in this PR can reach that case, andan inverted
get_uri()warning would not have described it either. It predates this change andis unaffected by it. The right place for it is a once-per-discovery advisory in the provider
registration path, naming the declared type, the name it normalizes to, and any collision, which
also covers third-party wheels that the authoring schema never sees. That belongs in its own
change rather than here.
Note on the Task SDK
Only
airflow-core'sConnectionever had theget_uri()warning, so the worker path that raises in practice was silent throughout. That is not becauseget_uri()is off the SDK's path, since it is called there after every successful backend fetch as well, but because the SDK's copy carries no warning code. The improved error messages apply to both copies.