dstandish commented on code in PR #27514:
URL: https://github.com/apache/airflow/pull/27514#discussion_r1017032245
##########
airflow/providers/common/sql/hooks/sql.py:
##########
@@ -264,6 +266,7 @@ def run(
results = []
for sql_statement in sql:
self._run_command(cur, sql_statement, parameters)
+ self._update_query_ids(cur)
Review Comment:
> I want to avoid all providers doing:
>
> class SnowflakeExecuteQueryOperator(SqlExecuteQueryOperator):
> def on_kill():
> ...
it's a very reasonable goal. at same time, i do think that we have to start
from real, working code for the "particular" cases before it will be clear what
makes sense in the abstract, general case. and it seems that we may not have
any sql operators that use dbapi hook which properly implement on_kill.
re AsyncSqlExecuteQueryOperator it depends what you're after.
this suggests deferrable operator. you can have a synchronous sql operator
which under the hood submits and polls (asynchronously). that might be one way
to allow for on_kill to function properly but there might also be other ways
such as the snowflake example above. i do think that putting async in name of
operator should be determined by the macro operator behavior not the underlying
implementation details -- i.e. does it just submit and exit and return query
through xcom (or defer). it's also possible to have an operator param that
controls this behavior.
--
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]
