kaxil commented on code in PR #74279:
URL: https://github.com/apache/airflow/pull/74279#discussion_r4556279256
##########
providers/common/sql/src/airflow/providers/common/sql/operators/sql.py:
##########
@@ -546,6 +552,7 @@ def __init__(
self.return_last = return_last
self.show_return_value_in_logs = show_return_value_in_logs
self.requires_result_fetch = requires_result_fetch
+ self.cancel_on_kill = cancel_on_kill
Review Comment:
`SnowflakeSqlApiOperator` already takes its own `cancel_on_kill` and sets
`self.cancel_on_kill` before calling
`super().__init__(conn_id=snowflake_conn_id, **kwargs)` without forwarding it.
This line then runs with the default `True`, so a user's `cancel_on_kill=False`
is silently reset and its `on_kill` cancels the queries anyway.
`test_snowflake_sql_api_on_kill_respects_cancel_on_kill_false` should fail on
this branch (I read it, didn't run it). Could the Snowflake operator pass
`cancel_on_kill=cancel_on_kill` to `super().__init__`, and the Snowflake tests
run on this PR? That is the only `SQLExecuteQueryOperator` subclass in
`providers/` that defines the name.
--
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]
