kaxil commented on code in PR #72855:
URL: https://github.com/apache/airflow/pull/72855#discussion_r3982504962
##########
airflow-core/src/airflow/models/connection.py:
##########
@@ -393,7 +401,19 @@ def get_hook(self, *, hook_params=None):
hook = ProvidersManager().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}"'
Review Comment:
Good catch, fixed in d544036982. You're right that the hyphenated case is
the one that can never resolve, and it was the one still getting the bare
message. It is reached by exactly the connections that never round-trip through
`get_uri()`, so metadata-DB rows and object-form imports.
It now names the spelling it is likely looking for:
```
Unknown hook type "google-cloud-platform". Note that it contains '-', which
is the
URI-scheme encoding of '_', so a connection type spelled with '-' cannot be
resolved;
the registered name is likely 'google_cloud_platform'. Otherwise the
provider supplying
this connection type may not be installed.
```
I have deliberately worded it as a hint rather than a diagnosis. #72854
leaves the runtime provider schema without a `connection-type` pattern, so an
out-of-tree provider can legitimately register a hyphenated type; "the provider
is not installed" has to remain a possible cause, or the message would
confidently mislead in that case.
##########
task-sdk/src/airflow/sdk/definitions/connection.py:
##########
@@ -220,7 +220,19 @@ def get_hook(self, *, hook_params=None):
hook = ProvidersManagerTaskRuntime().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}"'
Review Comment:
Same change applied here in d544036982. This is the copy that actually
raises in the worker path, so it is the more important of the two: the
traceback in #72316 came through the Task SDK, not the model.
--
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]
