You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/05/17 21:29:26 UTC
[GitHub] [airflow] kazanzhy opened a new pull request, #23767: Fix error when SnowflakeHook take empty list in `sql` param
kazanzhy opened a new pull request, #23767:
URL: https://github.com/apache/airflow/pull/23767
Fixes `UnboundLocalError: local variable 'execution_info' referenced before assignment` if the SQL parameter is an empty list in SnowflakeHook.
closes: #23623
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #23767: Fix error when SnowflakeHook take empty list in `sql` param
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #23767:
URL: https://github.com/apache/airflow/pull/23767#issuecomment-1130666300
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a diff in pull request #23767: Fix error when SnowflakeHook take empty list in `sql` param
Posted by GitBox <gi...@apache.org>.
turbaszek commented on code in PR #23767:
URL: https://github.com/apache/airflow/pull/23767#discussion_r875510449
##########
airflow/providers/snowflake/hooks/snowflake.py:
##########
@@ -290,7 +290,7 @@ def run(
autocommit: bool = False,
parameters: Optional[Union[Sequence[Any], Dict[Any, Any]]] = None,
handler: Optional[Callable] = None,
- ):
+ ) -> Optional[List]:
Review Comment:
Is there any use case for empty `sql` argument? Maybe it would be better to raise `ValueError` instead of doing unnecessary connection opening. WDYT?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr merged pull request #23767: Fix error when SnowflakeHook take empty list in `sql` param
Posted by GitBox <gi...@apache.org>.
uranusjr merged PR #23767:
URL: https://github.com/apache/airflow/pull/23767
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kazanzhy commented on pull request #23767: Fix error when SnowflakeHook take empty list in `sql` param
Posted by GitBox <gi...@apache.org>.
kazanzhy commented on PR #23767:
URL: https://github.com/apache/airflow/pull/23767#issuecomment-1130634796
@turbaszek
As I understand this method overrides `DbApiHook.run()`.
I see that this possible error (`... referenced before assignment`) might be in DbApiHook, DatabricksSqlHook, **SnowflakeHook**, and ExasolHook.
TrinoHook and PrestoHook also override `DbApiHook.run()` but take a string as an SQL parameter.
So, here we could decide how we could handle the situation when `some_hook.run(sql=[])`. Therefore `ValueError` seems to be a good solution.
For example, if we trying to run an empty string using `psycopg2` it raises `ProgrammingError: can't execute an empty query`.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org