Skip to content

Add get_airflow_version helper#44607

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.

Sign up for GitHub

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

Closed
dstandish wants to merge 1 commit into from
Closed

Add get_airflow_version helper #44607

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions airflow/__init__.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -58,10 +58,20 @@
# very easily cause import cycles in the conf init/validate code (since downstream code from
# those functions likely import settings).
# configuration is therefore initted early here, simply by importing it.

from packaging.version import Version

from airflow import configuration, settings


@potiukpotiukDec 3, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think - as discussed in slack - this is a nice proposal. that we might be able to use in some 8 months (if we back-port it and release in 2.11, but we need to figure out - as part of it - what we do now and how we do it for the rest of the code.

I proposed a solution there (this needs to be fixed) - #43773 (comment) - not sure if we can figure out something better. I think it's a good idea to discuss it there.

@dstandishdstandishDec 3, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

so the only real material problem with the constants is if someone imports one of these constants across providers. otherwise, it's mostly a non-issue. in any case this will be a generally helpful helper func.

if we want to reduce the likelihood of cross provider imports of these kinds of version constants, we could remove the version_references module from the standard provider. i've done that in latest commit. take a look and lemme know what you think.

@potiukpotiukDec 4, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Actually, after thinking a bit and seeing it - I think we can have cale And wat it too. Maybe we can generate the constants via __init__.py template in all providers?

This way each procider could have automatically generated own set of constant (possibly even named differently to avoid confusion where to importit from.

I am at family funeral today with my mum and two aunt's but I can propose a PR when I am back if that seems appealing

@potiukpotiukDec 5, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See #44686

def get_airflow_version() -> Version:

@potiukpotiukDec 7, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
defget_airflow_version() ->Version:
defget_base_airflow_version() ->Version:

"""Return packaging Version object representing the base version."""
return Version(Version(__version__).base_version)

@potiukpotiukDec 7, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we make sure (add pre-commit?) that this method is not used until min-airflow-version is set to 2.11 in all providers?

@potiukpotiukDec 7, 2024•
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Likely it could just be a RUFF https://docs.astral.sh/ruff/settings/#lint_flake8-import-conventions_banned-from if instead adding it to airflow/__init__.py you will move it to airflow/version_compat.py module for example (following what I am proposing in #44686 as a solution until 2.11 is set as minimum version.

Implementing identical module will make it as simple as removing all version_compat.py from providers and replacing local provider's imports with imports from airflow.version_compat.



__all__ = [
"__version__",
"get_airflow_version",
"DAG",
"Asset",
"XComArg",
Expand Down