ashb commented on code in PR #73301:
URL: https://github.com/apache/airflow/pull/73301#discussion_r4080882613
##########
providers/duckdb/src/airflow/providers/duckdb/operators/duckdb.py:
##########
@@ -56,17 +61,36 @@ def __init__(self, *, conn_id: str =
DuckDBHook.default_conn_name, **kwargs) ->
def get_db_hook(self) -> DuckDBHook:
"""
- Build the hook directly instead of resolving it through the connection.
+ Resolve the hook from the connection, and build one directly when
there is no connection.
+
+ ``BaseSQLOperator`` resolves the hook through
``BaseHook.get_connection(conn_id).get_hook()``,
+ which raises when the connection is absent. DuckDB needs no connection
to be useful, so a
+ missing one is handed to ``hook_class`` to interpret rather than being
an error here.
- ``BaseSQLOperator`` resolves the hook class via
``BaseHook.get_connection(conn_id).get_hook()``,
- which raises when the connection is absent. DuckDB needs no connection
to be useful, so the
- hook is constructed here and left to decide what the missing
connection means.
+ When the connection does exist its type chooses the hook. That is what
lets another provider
+ add cloud-specific behaviour, such as brokering credentials, without a
Dag having to swap
+ operator: pointing this operator at a connection of that type is
enough.
"""
hook_params = dict(self.hook_params)
# ``BaseSQLOperator`` applies ``database`` in ``_hook``, which this
override bypasses, and it
# would apply it as ``hook.schema`` — meaningless for DuckDB, where a
database is a file path.
if self.database:
hook_params["database"] = self.database
- return self.hook_class(
- duckdb_conn_id=self.conn_id or self.hook_class.default_conn_name,
**hook_params
- )
+
+ conn_id = self.conn_id or self.hook_class.default_conn_name
+ try:
+ connection = self.hook_class.get_connection(conn_id)
+ except AirflowNotFoundException:
+ return self.hook_class(duckdb_conn_id=conn_id, **hook_params)
Review Comment:
This try/except doesn't quite make sense to me -- why are we not showing the
"conn not found" error to the user as is? (Maybe this is commonly what we do
else where? I'm just surprised by it)
--
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]
