Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SnowflakeHook.run() raises UnboundLocalError exception if sql argument is empty #23623

Closed
2 tasks done
krisvaz opened this issue May 10, 2022 · 4 comments · Fixed by #23767
Closed
2 tasks done

SnowflakeHook.run() raises UnboundLocalError exception if sql argument is empty #23623

krisvaz opened this issue May 10, 2022 · 4 comments · Fixed by #23767

Comments

@krisvaz
Copy link

krisvaz commented May 10, 2022

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

  • Yes I am willing to submit a PR!

Code of Conduct

@krisvaz krisvaz added area:providers kind:bug This is a clearly a bug labels May 10, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented May 10, 2022

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

@uranusjr
Copy link
Member

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.

@eladkal
Copy link
Contributor

eladkal commented May 10, 2022

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.

@uranusjr
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants