Lee-W commented on code in PR #34041: URL: https://github.com/apache/airflow/pull/34041#discussion_r1314751981
########## airflow/providers/vertica/hooks/vertica.py: ########## @@ -17,9 +17,24 @@ # under the License. from __future__ import annotations +from typing import Any, Callable, Iterable, Mapping, overload + from vertica_python import connect -from airflow.providers.common.sql.hooks.sql import DbApiHook +from airflow.providers.common.sql.hooks.sql import DbApiHook, fetch_all_handler + + +def vertica_fetch_all_handler(cursor) -> list[tuple] | None: + """Replace the default DbApiHook fetch_all_handler .""" + to_return = fetch_all_handler(cursor) + # loop on all statement result sets to get errors + if cursor.description is not None: + while cursor.nextset(): + if cursor.description is not None: + row = cursor.fetchone() + while row: + row = cursor.fetchone() + return to_return Review Comment: Will this value be changed during the execution of lines 31-36? It seems this variable is not touched after it is assigned. But I guess this is something related to cursor? ########## airflow/providers/vertica/hooks/vertica.py: ########## @@ -17,9 +17,24 @@ # under the License. from __future__ import annotations +from typing import Any, Callable, Iterable, Mapping, overload + from vertica_python import connect -from airflow.providers.common.sql.hooks.sql import DbApiHook +from airflow.providers.common.sql.hooks.sql import DbApiHook, fetch_all_handler + + +def vertica_fetch_all_handler(cursor) -> list[tuple] | None: + """Replace the default DbApiHook fetch_all_handler .""" + to_return = fetch_all_handler(cursor) Review Comment: Could we rename `to_return`? Does `rows` make sense as the name of this value? or `result` might be a bit better than `to_return` -- 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]
