o-nikolas commented on code in PR #73301:
URL: https://github.com/apache/airflow/pull/73301#discussion_r4088158001
##########
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:
Fair, it does read oddly. It isn't swallowed though. The hook re-raises
unless conn_id is its own default_conn_name, which is the single id allowed to
be missing so that DuckDB works with zero configuration against an in-memory
database. I kept that decision in the hook rather than also duplicating it
here, since the hook can also be used directly and would need it anyway.
I've added a comment to clarify because it is confusing without context.
--
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]
