kaxil opened a new pull request, #72855:
URL: https://github.com/apache/airflow/pull/72855

   ## Summary
   
   `Connection.get_uri()` warns whenever `conn_type` contains `_`, citing RFC 
3986. The check is inverted: it fires on the form that is correct and required, 
and is silent on the form that is actually broken.
   
   RFC 3986 forbids `_` in a URI scheme, so 
[`get_uri()`](https://github.com/apache/airflow/blob/bdf2abcbc6/airflow-core/src/airflow/models/connection.py#L297)
 encodes `_` as `-` on the way out and 
[`_normalize_conn_type`](https://github.com/apache/airflow/blob/bdf2abcbc6/airflow-core/src/airflow/models/connection.py#L228-L233)
 decodes it back on the way in. The underscore form is therefore the canonical 
one, and it is what the hook registry is keyed on. The URI being warned about 
is compliant by construction.
   
   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 repo's own test pins the inversion:
   
   | case | warns before | warns after |
   |---|---|---|
   | `conn_type="google_cloud_platform"` (canonical, correct) | yes | no |
   | `conn_type="google-cloud-platform"` (cannot round-trip) | no | yes |
   
   ## Why it matters beyond tidiness
   
   35 connection types in the provider tree contain `_`, so the warning fires 
for all of them, once per uncached connection fetch, in the scheduler and API 
server. That is the noisy half.
   
   The costly half is that it advises the wrong thing. #62817 renamed 
`pydantic_ai` to `pydanticai` specifically to silence this warning, and the 
vendor types added in #62816 then followed the same instinct and used hyphens, 
which is the bug fixed in #72853. The diagnostic told a contributor that 
underscores were wrong. They were not.
   
   Its `_prenormalized_conn_type` guard could never have helped, either. That 
attribute is only assigned from a parsed URI scheme, and an underscore scheme 
parses with no scheme at all:
   
   ```python
   >>> urlsplit("foo_bar://h").scheme
   ''
   ```
   
   So it can never contain `_`, and the guard can only ever suppress the 
warning, never trigger it.
   
   ## What changes
   
   The condition now flags `-`, and the message names the type the connection 
will actually resolve to, so it says what to change:
   
   ```
   Connection type 'google-cloud-platform' contains '-', which does not survive 
URI
   serialization: '-' is the URI-scheme encoding of '_', so this connection 
resolves
   back to 'google_cloud_platform' and its hook will not be found. Declare the
   connection type with '_' instead.
   ```
   
   No false positives are possible on a connection parsed from a URI, because 
`_normalize_conn_type` replaces every `-` before `get_uri()` could ever see it. 
A hyphen at that point can only come from kwargs or a metadata-DB column, which 
is precisely the broken configuration.
   
   ## Second change: the failure now explains itself
   
   An underscore scheme parses with no scheme, which left `conn_type` empty and 
produced:
   
   ```
   AirflowException: Unknown hook type ""
   ```
   
   That names neither the cause nor the fix. It now reports the empty 
connection type and the scheme rule behind it, in both 
`airflow.models.Connection` and the Task SDK copy, since both raise it. The 
existing `raise` is reused rather than adding one, to respect 
`check-no-new-airflow-exceptions`.
   
   ## Note on the Task SDK
   
   Only `airflow-core`'s `Connection` ever had the `get_uri()` warning; the SDK 
copy has none, so the worker path that actually raises in practice was silent 
throughout. This PR does not add a warning there, since `get_uri()` is not on 
the SDK's hot path, but the improved error message is applied to both.
   


-- 
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]

Reply via email to