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

Update snowflake hook to not use extra prefix #26764

Merged
merged 8 commits into from
Oct 22, 2022

Conversation

dstandish
Copy link
Contributor

@dstandish dstandish commented Sep 29, 2022

As of Airflow 2.3 it's no longer necessary to use the extra__<conn_type>__ prefix for connection extras.

We're bumping min airflow version to 2.3 now, so we can remove the prefix.

We handle back compat with method get_field.

And in version 2.5 we will support no prefix in get_ui_field_behaviors (pr #26995), so we prepare for that here. What we do is update the placeholders to remove the prefix but add in the logic (which in 2.5 is added to providers manager) to add the prefix. This way all we need to do when min airflow version is 2.5 is remove the decorator. And in the meantime, we don't ever have to see the prefix.

@@ -41,6 +42,18 @@ def _try_to_boolean(value: Any):
return value


def _maybe_add_prefix(val):
"""
From Airflow 2.3 onward, there is no longer a need to add the `extra__<conn type>__`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Next provider release cycle is going to set min Airflow version 2.3 for all providers:
https://github.com/apache/airflow#release-process-for-providers

Since the next cycle is on October any way maybe we should hold this PR till we adjust the min Airflow version?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, that's fine

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when do you think we will bump the min version? will there be time to add this after bumping and before release?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bump can happen only after 11-Oct because we might need to make ad-hoc release to fix something urgently. Right after 11-Oct we can bump all providers and then proceed.

The reason I suggested it is because we usually do 1 release per month so the next release is expected around the end of October thus the current "backport" code for 2.2 will never reach to the user anyway.

We can also merge this first and do cleanup after.. That is OK.

@dstandish dstandish force-pushed the snowflake-to-abandon-the-prefix branch from 1567e1d to a17f798 Compare October 12, 2022 06:11
@dstandish dstandish changed the title Update snowflake hook to not use extra prefix unless necessary. Update snowflake hook to not use extra prefix Oct 12, 2022
@dstandish dstandish force-pushed the snowflake-to-abandon-the-prefix branch 2 times, most recently from d4d27a8 to dd2cf78 Compare October 12, 2022 21:19
@dstandish dstandish force-pushed the snowflake-to-abandon-the-prefix branch from 3b31d3c to 136fb39 Compare October 18, 2022 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:snowflake Issues related to Snowflake provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants