-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Fix: Pass hook parameters to SnowflakeSqlApiHook and prep them for AP… #41150
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
Fix: Pass hook parameters to SnowflakeSqlApiHook and prep them for AP… #41150
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
de89a29 to
f4db682
Compare
|
Can you please add/modify a unit test for this? |
|
@BTeclaw reminder about adding tests |
9c693fb to
588c520
Compare
|
Pushed unit test for parametrization of the API call, haven't really written any unit tests like that before so feedback is welcome! Thanks |
507a197 to
6fae2c3
Compare
Lee-W
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some nitpicks, but overall looks good
Co-authored-by: Wei Lee <weilee.rx@gmail.com>
Co-authored-by: Wei Lee <weilee.rx@gmail.com>
|
@Lee-W applied suggested changes - thank you! |
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
…I call (apache#41150) --------- Co-authored-by: Blazej Teclaw <blazej.teclaw@hlag.com> Co-authored-by: Wei Lee <weilee.rx@gmail.com>
…I call (apache#41150) --------- Co-authored-by: Blazej Teclaw <blazej.teclaw@hlag.com> Co-authored-by: Wei Lee <weilee.rx@gmail.com>
This PR is a resolution to the given issue #39622. The main bug here was that the hook parameters were not properly given to the SnowflakeSqlAPIHook and then they were not properly assigned. The query execution method of the hook statically assigned configuration from Airflow connection. In this PR it is changed so that:
closes: #39622
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.