Make Snowflake Hook conform to semantics of DBApi #28006
Conversation
boring-cyborg
bot
added
area:providers
provider:snowflake
|
If this one will be ok for others, I will fix all failing tests (if there will be some) and add a set of tests like I did for Databricks Hook: https://github.com/apache/airflow/blob/main/tests/providers/databricks/hooks/test_databricks_sql.py |
|
cc: @kazanzhy @denimalpaca . This is hopefull the LAST (🤞 ) of standardising Hook's run() behaviour. And I hope later on we might be even able to switch to using the common.sql one for both Snowflake and Exasol (they are the two remaining Hooks with custom run() - but after this change, they are conforming to the same semantics, so we should be able to replace the custom run() methods easily. |
|
Ah... Tests passed so we REALLY need to add more tests, because we do not test what we should :) |
potiuk
force-pushed
the
standardize-snowflake-hook-run-results
branch
from
e1503d0 to
931d08f
Compare
|
I addeed million tests - similar as we have in Databricks case (and it's good I did - I found few issues). |
vincbeck
left a comment
vincbeck
left a comment
There was a problem hiding this comment.
One minor comment, otherwise LGTM
potiuk
force-pushed
the
standardize-snowflake-hook-run-results
branch
from
931d08f to
13b60d5
Compare
potiuk
force-pushed
the
standardize-snowflake-hook-run-results
branch
from
13b60d5 to
58065aa
Compare
potiuk
force-pushed
the
standardize-snowflake-hook-run-results
branch
from
58065aa to
09c4957
Compare
This is a breaking change, that changes return value of the Hook, however it keeps compatibility with regards of the value returned by the SnowflakeOperator (so DAGs using the operator should be unaffected). You can also switch back to the Dictionary returned by Hooks by specifying ``return_dictionaries`` parameter to run method of the Hook. At the same time SnowflakeHook can be used in a number of generic operators and sensors such as: * SQLColumnCheckOperator * SQLSensor Fixes: apache#27978 Fixes: apache#27976
potiuk
force-pushed
the
standardize-snowflake-hook-run-results
branch
from
09c4957 to
4ff1099
Compare
Co-authored-by: Tzu-ping Chung <[email protected]>
Co-authored-by: Tzu-ping Chung <[email protected]>
* Make Snowflake Hook conform to semantics of DBApi This is a breaking change, that changes return value of the Hook, however it keeps compatibility with regards of the value returned by the SnowflakeOperator (so DAGs using the operator should be unaffected). You can also switch back to the Dictionary returned by Hooks by specifying ``return_dictionaries`` parameter to run method of the Hook. At the same time SnowflakeHook can be used in a number of generic operators and sensors such as: * SQLColumnCheckOperator * SQLSensor Fixes: apache#27978 Fixes: apache#27976
5 participants
This is a breaking change, that changes return value of the Hook, however it keeps compatibility with regards of the value returned by the SnowflakeOperator (so DAGs using the operator should be unaffected).
At the same time SnowflakeHook can be used in a number of generic operators and sensors such as:
Fixes: #27978
Fixes: #27976
^ 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.