kaxil opened a new pull request, #72904:
URL: https://github.com/apache/airflow/pull/72904
## Summary
A hook registers under the `connection-type` string verbatim. A connection
read from a URI, from JSON, or rebuilt from the secrets cache presents the
*decoded* form of that string: `get_uri()` lowercases the type and encodes `_`
as `-`, because RFC 3986 forbids `_` in a URI scheme, and reading a connection
back decodes it again.
Where the declaration and the decoded name differ, the hook is unreachable
for every such connection. A connection created through the UI, the REST API or
the CLI keeps the declared spelling and does resolve, which is what lets the
mismatch go unnoticed until a connection is served from somewhere else. That is
#72316.
Provider discovery now says so once, naming the package that declared it.
## The case that never fails
Where two providers declare the two spellings of one name, a connection for
either resolves whichever hook holds the decoded name:
```
stored directly conn_type='foo-bar' -> provider_a.HookA
after a URI round trip conn_type='foo_bar' -> provider_b.HookB
```
Neither call raises. The connection is handed the other provider's hook, and
because nothing fails there is no error anywhere that could carry the
explanation.
## Why discovery rather than somewhere else
The authoring schema validates the providers in this repository alone, so a
third-party distribution shipping `get_provider_info()` never passes through
it. The runtime `provider_info.schema.json` is deliberately lenient so installs
of already-released providers keep working, which is why #72854 left it alone
and #71104 declined the same tightening.
`Connection.get_hook()` is reached only after a lookup has already failed,
so it cannot reach the collision above, which resolves.
Discovery sees every declared type together with the package that declared
it, and it already warns there when one connection type is registered twice.
This is that check against the decoded names rather than the literal ones.
## What it logs
```
A declared connection type is read back under a different name, so a
connection read from a
URI or from JSON cannot reach its hook.
connection_type='dummy-vendor' read_back_as='dummy_vendor'
package='apache-airflow-providers-dummy'
Several declared connection types are read back under one name, so a
connection for any of
them resolves whichever hook holds that name.
connection_types=['shared-name', 'shared_name']
read_back_as='shared_name'
packages=['apache-airflow-providers-one', 'apache-airflow-providers-two']
A declared connection type cannot be carried in a connection URI, so a
connection read from
a URI or from JSON has no connection type at all.
connection_types=['dummy vendor']
packages=['apache-airflow-providers-dummy']
```
## Design rationale
The decoded name is asked of the decoder, `Connection._normalize_conn_type`
applied to the parsed scheme, rather than restated as a separator swap.
Restating it misses what that decoder actually does: `postgresql` is aliased to
`postgres`, so a provider declaring `postgresql` is unreachable while a
hand-rolled rule calls it fine, and a type that cannot be a URI scheme at all,
such as one containing a space, is lost rather than re-spelled and needs its
own message. A test asserts the derived name against a real
`get_uri`/`from_uri` round trip, so the two cannot drift apart.
The import is inside the function because
`airflow.sdk.definitions.connection` imports this module at module level,
following the commented precedent already in this file.
## Merge order
This has to land after #72853. On `main` today the three hyphenated
connection types in `common.ai` trip the new warning, which turns `test_hooks`
and `test_hook_values` red, since both assert that hook discovery produces no
warnings. With #72853 merged in locally the file is 30 of 30. #72853 renames
those three and constrains `connection-type` in the authoring schema, after
which no in-tree type trips this.
That the check fires on `main` is the point: those three are genuinely
unreachable from any connection served by a secrets backend.
## Note on log style
These warnings pass structlog keyword arguments rather than the `%s`
positional arguments used by the neighbouring warnings in this file. Worker
logs are JSON, where a positional argument is not interpolated into the event
and its value lands in a separate field.
## Related
#72855 improves what `Connection.get_hook()` says when a lookup fails. It is
independent of this change and touches different files; this one covers the
case that does not fail.
--
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]
