Bring back min-airflow-version for preinstalled providers#31469
Merged
potiuk merged 1 commit intoMay 23, 2023
Conversation
In the last wave of providers apache#31416 we bumped min-airlfow-version to 2.4 and added mechanism to verify min-airflow version is ok while importing, but it turned out that there are cases where installing just old version of airflow (with no constraints) brings the latest version of those providers and causes new installation of airflow to fail. This is far too common to ignore or require to use constraints, unfortunately We do not have min-airlfow-version in the preinstalled providers for one reason only. For some tools that are NOT conforming to standards (such as bazel), having min-airflow-version for those providers causes circular dependency (even though technically dependencies in PyPI can - and often are - circular, because dependencies in Python are not DAG and can contain cycles. This was in response to this issue in 2021 apache#17795. However, this is really bazel issue, and on top of it - it's recognized as such and being fixed very recently in bazel-contrib/rules_python#1188 because there are other packages that have similar problems (pytorch and triton being popular couple). Also Bazel is not that popular in the Python world. Therefore, rather than trying to workaround the problem of bazel, we encourage them to merge and release the fix bazel-contrib/rules_python#1166 (comment) and call it out in our installation instructions, that bazel installation might lead to problems like that. If bazel does not fix it, this will only be a problem for Future installations of airflow in a few months most likely. It will not impact current bazel users installing old versions of Airflow (actually they might start having problems now if we do not fix it and yank the 5 providers released yesterday)
eladkal
approved these changes
May 23, 2023
hussein-awala
approved these changes
May 23, 2023
pankajkoti
reviewed
May 23, 2023
Comment on lines
-59
to
-72
| {%- if PREINSTALLED_PROVIDER %} | ||
|
|
||
| This provider package is preinstalled by default when Apache Airflow is installed. You do not need to | ||
| install it separately. You can upgrade and downgrade it independently of Apache Airflow package though. | ||
|
|
||
| .. note:: | ||
|
|
||
| The minimum Apache Airflow version for this package is {{ MIN_AIRFLOW_VERSION }} and it will fail | ||
| import at runtime if the version of Airflow is lower even if there is no requirement specified in | ||
| the dependencies - this is because the provider is preinstalled and specifying minimum Apache | ||
| Airflow version would create a dependency cycle, which confuses dependency tools. | ||
|
|
||
| {%- endif %} | ||
|
|
Member
There was a problem hiding this comment.
Do we need to regenerate the docs to remove this block from the existing docs of those 5 providers? Or it happens (by itself?) as some later step (perhaps during releasing the providers 🤔 )?
Member
Author
There was a problem hiding this comment.
It is part of the release process: https://github.com/apache/airflow/blob/main/dev/README_RELEASE_PROVIDER_PACKAGES.md#generate-release-notes
There Release Manager will regenerate the docs and increase provider versions.
Member
There was a problem hiding this comment.
Understood. I was trying to find the usage of the script and guessed that & edited my comment but did not refresh the page to see your comment before. Thank you!
10 tasks
ephraimbuddy
added
the
type:misc/internal
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In the last wave of providers #31416 we bumped min-airlfow-version to 2.4 and added mechanism to verify min-airflow version is ok while importing, but it turned out that there are cases where installing just old version of airflow (with no constraints) brings the latest version of those providers and causes new installation of airflow to fail. This is far too common to ignore or require to use constraints, unfortunately We do not have min-airlfow-version in the preinstalled providers for one reason only. For some tools that are NOT conforming to standards (such as bazel), having min-airflow-version for those providers causes circular dependency (even though technically dependencies in PyPI can - and often are - circular, because dependencies in Python are not DAG and can contain cycles.
This was in response to this issue in 2021 #17795.
However, this is really bazel issue, and on top of it - it's recognized as such and being fixed very recently
in bazel-contrib/rules_python#1188 because there are other packages that have similar problems (pytorch and triton being popular couple). Also Bazel is not that popular in the Python world.
Therefore, rather than trying to workaround the problem of bazel, we encourage them to merge and release the fix
bazel-contrib/rules_python#1166 (comment) and call it out in our installation instructions, that bazel installation might lead to problems like that.
If bazel does not fix it, this will only be a problem for Future installations of airflow in a few months most likely. It will not impact current bazel users installing old versions of Airflow (actually they might start having problems now if we do not fix it and yank the 5 providers released yesterday)
^ 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.