-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Fix SnowflakeSqlApiHook backwards compatibility #49482
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 SnowflakeSqlApiHook backwards compatibility #49482
Conversation
providers/snowflake/src/airflow/providers/snowflake/hooks/snowflake_sql_api.py
Outdated
Show resolved
Hide resolved
2a35f71 to
d815ad5
Compare
amoghrajesh
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.
Approach looks ok, one case needs handling. Also can you add test to check for region and account?
providers/snowflake/src/airflow/providers/snowflake/hooks/snowflake_sql_api.py
Outdated
Show resolved
Hide resolved
providers/snowflake/src/airflow/providers/snowflake/hooks/snowflake.py
Outdated
Show resolved
Hide resolved
|
BTW thanks for getting this fix. |
a4bc510 to
fba4a3d
Compare
|
Thanks a lot for the reviews! |
providers/snowflake/src/airflow/providers/snowflake/hooks/snowflake_sql_api.py
Outdated
Show resolved
Hide resolved
fba4a3d to
80a33d3
Compare
providers/snowflake/src/airflow/providers/snowflake/hooks/snowflake_sql_api.py
Outdated
Show resolved
Hide resolved
eladkal
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.
LGTM
amoghrajesh
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.
Yeah nice!
6b76c0a to
40accb8
Compare
…d add test case for connection params for oauth
40accb8 to
315d643
Compare
…ethod (apache#49482) * Fix SnowflakeSqlApiHook backwards compatibility * Update providers/snowflake/src/airflow/providers/snowflake/hooks/snowflake_sql_api.py --------- Co-authored-by: Elad Kalif <45845474+eladkal@users.noreply.github.com>
Follow Up: #49449
^ 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 airflow-core/newsfragments.