mik-laj commented on a change in pull request #20495:
URL: https://github.com/apache/airflow/pull/20495#discussion_r775205385
##########
File path: airflow/providers/snowflake/hooks/snowflake.py
##########
@@ -310,14 +309,14 @@ def run(
self.log.info("Rows affected: %s", cur.rowcount)
self.log.info("Snowflake query id: %s", cur.sfqid)
- self.query_ids.append(cur.sfqid)
+ query_ids.append(cur.sfqid)
# If autocommit was set to False for db that supports autocommit,
# or if db does not supports autocommit, we do a manual commit.
if not self.get_autocommit(conn):
conn.commit()
- return execution_info
+ return execution_info, query_ids
Review comment:
According to the method description of the `DBAPiHook` base class,
`DBAPiHook.run` should return the results of the operation. It shouldn't return
a tuple, one of the elements of which is the result of the operation.
https://github.com/apache/airflow/blob/b00ce4592fb21bd4cf4ff6227fa5a498eb357faf/airflow/hooks/dbapi.py#L192
Besides, it is a breaking change that we should avoid.
If we really want to introduce this feature, then we should do it as a new
method.
--
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]
