josh-fell commented on a change in pull request #20212:
URL: https://github.com/apache/airflow/pull/20212#discussion_r767088454
##########
File path: airflow/providers/snowflake/hooks/snowflake.py
##########
@@ -135,7 +137,7 @@ def __init__(self, *args, **kwargs) -> None:
self.schema = kwargs.pop("schema", None)
self.authenticator = kwargs.pop("authenticator", None)
self.session_parameters = kwargs.pop("session_parameters", None)
- self.query_ids = []
+ self.query_ids: List[str] = []
Review comment:
Is the `self.query_ids = []` needed in the `run()` method?
##########
File path: airflow/providers/snowflake/hooks/snowflake.py
##########
@@ -135,7 +137,7 @@ def __init__(self, *args, **kwargs) -> None:
self.schema = kwargs.pop("schema", None)
self.authenticator = kwargs.pop("authenticator", None)
self.session_parameters = kwargs.pop("session_parameters", None)
- self.query_ids = []
+ self.query_ids: List[str] = []
Review comment:
Is the `self.query_ids = []` needed in the `run()` method if it's
already defined here?
##########
File path: airflow/providers/snowflake/hooks/snowflake.py
##########
@@ -64,6 +64,8 @@ class SnowflakeHook(DbApiHook):
:param session_parameters: You can set session-level parameters at
the time you connect to Snowflake
:type session_parameters: Optional[dict]
+ :param query_ids:
Review comment:
There should be a description here. The API docs for this hook are built
from the docstring and we shouldn't have a parameter without a description.
##########
File path: airflow/providers/snowflake/hooks/snowflake.py
##########
@@ -239,6 +247,8 @@ def run(self, sql: Union[str, list], autocommit: bool =
False, parameters: Optio
:type autocommit: bool
:param parameters: The parameters to render the SQL query with.
:type parameters: dict or iterable
+ :param handler: The result handler which is called with the result of
each statement.
+ :type handler: dict or iterable
Review comment:
The docstring doesn't match the typing here.
##########
File path: airflow/providers/snowflake/hooks/snowflake.py
##########
@@ -64,6 +64,8 @@ class SnowflakeHook(DbApiHook):
:param session_parameters: You can set session-level parameters at
the time you connect to Snowflake
:type session_parameters: Optional[dict]
+ :param query_ids:
Review comment:
Oh duh, you’re right, that was silly of me to even suggest.
--
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]
