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/10 14:37:38 UTC

[GitHub] [airflow] krisvaz opened a new issue, #23623: SnowflakeHook.run() raises UnboundLocalError exception if sql argument is empty

krisvaz opened a new issue, #23623:
URL: https://github.com/apache/airflow/issues/23623

   ### Apache Airflow Provider(s)
   
   snowflake
   
   ### Versions of Apache Airflow Providers
   
   apache-airflow-providers-snowflake==2.3.0
   
   ### Apache Airflow version
   
   2.2.2
   
   ### Operating System
   
   Amazon Linux AMI
   
   ### Deployment
   
   MWAA
   
   ### Deployment details
   
   _No response_
   
   ### What happened
   
   If the sql parameter is an empty list, the execution_info list variable is attempted to be returned when it hasn't been initialized.
   
   The execution_info variable is [defined](https://github.com/apache/airflow/blob/2.3.0/airflow/providers/snowflake/hooks/snowflake.py#L330) only within parsing through each sql query, so if the sql queries list is empty, it never gets defined.
   
   ```
   [...]
     snowflake_hook.run(sql=queries, autocommit=True)
     File "/usr/local/airflow/.local/lib/python3.7/site-packages/airflow/providers/snowflake/hooks/snowflake.py", line 304, in run
       return execution_info
   UnboundLocalError: local variable 'execution_info' referenced before assignment
   ```
   
   ### What you think should happen instead
   
   The function could either return an empty list or None.
   
   Perhaps the `execution_info` variable definition could just be moved further up in the function definition so that returning it at the end doesn't raise issues.
   
   Or, there should be a check in the `run` implementation to see if the `sql` argument is empty or not, and appropriately handle what to return from there.
   
   
   
   ### How to reproduce
   
   Pass an empty list to the sql argument when calling `SnowflakeHook.run()`.
   
   ### Anything else
   
   My script that utilizes the `SnowflakeHook.run()` function is automated in a way where there isn't always a case that there are sql queries to run. 
   
   Of course, on my end I would update my code to first check if the sql queries list is populated before calling the hook to run.
   
   However, it would save for unintended exceptions if the hook's `run()` function also appropriately handles what gets returned in the event that the `sql` argument is empty.
   
   ### Are you willing to submit PR?
   
   - [X] Yes I am willing to submit a PR!
   
   ### Code of Conduct
   
   - [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
   


-- 
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.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] uranusjr closed issue #23623: SnowflakeHook.run() raises UnboundLocalError exception if sql argument is empty

Posted by GitBox <gi...@apache.org>.
uranusjr closed issue #23623: SnowflakeHook.run() raises UnboundLocalError exception if sql argument is empty
URL: https://github.com/apache/airflow/issues/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] uranusjr commented on issue #23623: SnowflakeHook.run() raises UnboundLocalError exception if sql argument is empty

Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #23623:
URL: https://github.com/apache/airflow/issues/23623#issuecomment-1123869619

   Then the solution is probably to just add an additional check and return something else (probably `None`?) when `sql` is empty.


-- 
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] eladkal commented on issue #23623: SnowflakeHook.run() raises UnboundLocalError exception if sql argument is empty

Posted by GitBox <gi...@apache.org>.
eladkal commented on issue #23623:
URL: https://github.com/apache/airflow/issues/23623#issuecomment-1122973826

   > Not sure if that’s intended though.
   
   It doesnt make sense to collect return values of all queries. Only value returned from last query should be considered.
   


-- 
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 commented on issue #23623: SnowflakeHook.run() raises UnboundLocalError exception if sql argument is empty

Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #23623:
URL: https://github.com/apache/airflow/issues/23623#issuecomment-1122758517

   I think the `execution_info = []` line should be outside of the `for sql_statement in sql:` loop. This also seems to expose a potential bug: currently, if there are multiple SQL statements in `sql`, `execution_info` only contains result from the _last_ SQL statement, not all results from all staements. Not sure if that’s intended though.


-- 
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] boring-cyborg[bot] commented on issue #23623: SnowflakeHook.run() raises UnboundLocalError exception if sql argument is empty

Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #23623:
URL: https://github.com/apache/airflow/issues/23623#issuecomment-1122484355

   Thanks for opening your first issue here! Be sure to follow the issue template!
   


-- 
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