fixing circular import error in providers caused by airflow version check

[Lee-W]

We recently worked on testing our project with the latest providers' version, #31322 but encountered a circular import error. Thanks, @ash, for suggesting to me how to solve this on https://github.com/apache/airflow/pull/30994/files#r1197635148

---

^ 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.rst or {issue_number}.significant.rst, in newsfragments.

[ashb]

We should do this on all providers please.

[ashb]

Actually I worry if this will cause a problem on 2.4 and 2.5 specifically:

v2.4's airflow/__init__.py

if not os.environ.get("_AIRFLOW__AS_LIBRARY", None):
    settings.initialize()

# Things to lazy import in form {local_name: ('target_module', 'target_name')}
__lazy_imports: dict[str, tuple[str, str]] = {
    'DAG': ('.models.dag', 'DAG'),
    'Dataset': ('.datasets', 'Dataset'),
    'XComArg': ('.models.xcom_arg', 'XComArg'),
    'AirflowException': ('.exceptions', 'AirflowException'),
    'version': ('.version', ''),
    '__version__': ('.version', 'version'),
}


def __getattr__(name: str):
    ... # Lazy load code

So on 2.4, the logging import will be triggered by the settings.initalize() function, but the getattr for lazy import hasn't yet been defined/parsed, so trying to import airflow.__version__ will fail :(

2.5 is the same.

[Lee-W]

then maybe we can try something like the following?

try:
    airflow_version = airflow.__version__
except ImportError:
    airflow_version = airflow.version.version

if packaging.version.parse(airflow_version) < packaging.version.parse("2.4.0"):
... 

[potiuk]

As dicussed in the https://github.com/apache/airflow/pull/30994/files#r1197635148 - we need to also update the JINJA template in dev/provider_packages once we figure out the best approach also for 2.4 (just in case I add request changes so that it is not merged accidentally).

[ashb]

then maybe we can try something like the following?

try:
    airflow_version = airflow.__version__
except ImportError:
    airflow_version = airflow.version.version

if packaging.version.parse(airflow_version) < packaging.version.parse("2.4.0"):
... 

I think it's AttributeError (at least by your original error message) but yes.

[potiuk]

    airflow_version = airflow.version.version

I thnk that is not going to work - it will trigger the very same recursive import error. I think the only "good" solution is to query airflow package metadata if __version__ import fails.

It will be slower but it's not going to get into any danger of resursive error.

[ashb]

Actually, maybe catch both AttributeError and ImportError? I can't say which will be thrown? (flip, flop. Sorry)

[potiuk]

Actually, maybe catch both AttributeError and ImportError? I can't say which will be thrown? (flip, flop. Sorry)

catch Exception ?

[Lee-W]

I thnk that is not going to work - it will trigger the very same recursive import error. I think the only "good" solution is to query airflow package metadata if __version__ import fails.

Do you mean something like

from importlib.metadata import version

version("airflow")

[Lee-W]

Hi @ashb @potiuk , I tried to address all the comments. The following is the latest change

try:
    airflow_version = airflow.__version__
except Exception:
    from importlib.metadata import version

    version("airflow")

if packaging.version.parse(airflow_version) < packaging.version.parse("2.4.0"):

[potiuk]

Almost but not quite:

airflow_version = version("airflow")

Also we still need to support Python 3.7 (till the end of June) so we need to do it even more complicated:

try:
    from importlib.metadata import version
except ImportError:
    from importlib_metadata import version

In Python 3.7 importlib.metadata does not have version - it only comes with the importlib_metadata compatiblity package.

Am I right (@uranusjr @ashb ?)

[Lee-W]

@potiuk Thanks! Just updated it

[ashb]

@potiuk I think try to import airflow.__version__ and fall back to airflow.version.version should work fine -- we can catch the circular error import just like any other error can't we?

[potiuk]

@potiuk I think try to import airflow.__version__ and fall back to airflow.version.version should work fine -- we can catch the circular error import just like any other error can't we?

And what shoudl we do then :) ? It will not serve its purpose, becaue it will also fail with the same recursion error on 2.3 or 2.2 likely. so we will not be able to detect what we wanted to detect (specifically if someone tries to import new http or smtp provider on Airflow < 2.4 (that was the reason of this change to detect this case)

[ashb]

My point is the recursion error is catchable, and if that happens the airflow.version.version won't cause a recursion error

Wei and I will test what works on 2.3 and 2.4 - cos not having to have the importlib mess run in each provider would be good (otherwise it would slow things down a lot in 2.4/2.5)

[potiuk]

My point is the recursion error is catchable, and if that happens the airflow.version.version won't cause a recursion error

Wei and I will test what works on 2.3 and 2.4 - cos not having to have the importlib mess run in each provider would be good (otherwise it would slow things down a lot in 2.4/2.5)

Do we have other (faster) way to check which version of airflow are we in < 2.4 then (without triggering the recursion)?

[potiuk]

Because the whole point of this check is to check it - if we catch recursion error - we still do not know which airflow version we are at - and this is the whole point of the check.

[potiuk]

Because the whole point of this check is to check it - if we catch recursion error - we still do not know which airflow version we are at - and this is the whole point of the check.

Potentially, we could do the fallback only for those preinstalled providers if we have no other way. For all others, this check is a bit superfluous (it is only needed if someone forced installation of provider, without looking at the requiremenst. For the preinstalled provider, this runtime check is our only protection.

[potiuk]

We could make "if preinstalled_provider" in JINJA template - we already have this in context I think.

[ashb]

I tested this on 2.3.0 and 2.4.0 and it works -- ran pip install --no-deps apache-airflow-providers-amazon==8.1.0rc1 just to "force" it in to a bad state.

try:
    form airflow import version as airflow_version
except ImportError:
    form airflow.version import version as airflow_version

if packaging.version.parse(airflow_version) < packaging.version.parse("2.4.0"):

I even set the min dep to 2.4.1 to double check, all good.

AttributeError is never thrown if we do the from ... import -- the python import machinery handles that and throws ImportError: cannot import name '__version__' from 'airflow'.

[potiuk]

That's cool.

[potiuk]

And with #31385 we are going to double check it - because the test is going to check it all that also on Airflow 2.4 version.

[potiuk]

Rebased #31385 to cross-check it.

[ashb]

Oh sorry -- might have been a bit eager in merging this then.