potiuk commented on code in PR #42891:
URL: https://github.com/apache/airflow/pull/42891#discussion_r1800204947
##########
airflow/operators/generic_transfer.py:
##########
@@ -68,13 +76,17 @@ def __init__(
self.sql = sql
self.destination_table = destination_table
self.source_conn_id = source_conn_id
+ self.source_hook_params = source_hook_params
self.destination_conn_id = destination_conn_id
+ self.destination_hook_params = destination_hook_params
self.preoperator = preoperator
self.insert_args = insert_args or {}
def execute(self, context: Context):
- source_hook = BaseHook.get_hook(self.source_conn_id)
- destination_hook = BaseHook.get_hook(self.destination_conn_id)
+ source_hook = BaseHook.get_hook(conn_id=self.source_conn_id,
hook_params=self.source_hook_params)
Review Comment:
This is a bit problematic. It would be fine if we did it in Airflow 2 -
because "generic_transfer" is part of the "airflow" package. But we've decided
that in Airlfow 3 those operators will be moved to "standard" providers - see
#42252 #42081
This means that "generic_transfer" will eventually land in standard
provider. And since "standard" provider will also be Airflow 2 compliant,
making it call "BaseHook.get_hook(conn_id=self.source_conn_id,
hook_params=self.source_hook_params)" will make it fail on Airflow 2 (missing
hook_params keyword arg).
It **should be** detected when we move generic_transfer to the standard
provider in compatibility tests. But it meaans that it's the person who will be
moving it, who will have to solve it in the (near) future.
I think that better course of action is that part of this PR should be:
a) moving generic_transfer to standard provider
b) making it backwards compatible with Airlfow 2 (for example by checking
airflow version or by inspecting signature of the "get_hook" method.
Could you make those changes please ?
--
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]
