Removes provider-imposed requirements from setup.cfg #13409
Conversation
potiuk
added
upgrade to newer dependencies
full tests needed
|
I believe I managed to improve the "eager constraint update" mechanism in the way that it removes all provider-imposed requirements out of the setup.cfg / install_requires (!). It works exactly as before when it comes to constraints installation and automated constraint upgrade, but this time those requirements are not present in the install_requires section of the I simply add the imposed requirements in the images build steps where eager upgrade strategy is used. This seems to keep the same properties as previous solution where those limits were added to "install-requires" but those limits are only used during the image build and airflow package does not have those. Also with --upgrade-to-newer-dependencies switch of breeze you can run the very same "eager upgrade" locally and it makes it much easier to locally test new constraints without the need to push them as PR. It now becomes rather straightforward how to fix any future similar conflict. |
potiuk
force-pushed
the
move-provider-imposed-constraints-out-of-setup-cfg
branch
from
1f65d17
to
673d85d
Compare
|
CC: @r-richmond --> I think this is much better solution of the problem you wanted to solve in #13333 |
potiuk
force-pushed
the
move-provider-imposed-constraints-out-of-setup-cfg
branch
from
673d85d
to
fac79e8
Compare
|
cc: @mjpieters -> I think this is the ultimate solution to many of the problems we had with providers messing with the apache-airflow package. |
github-actions
bot
removed
the
full tests needed
potiuk
force-pushed
the
move-provider-imposed-constraints-out-of-setup-cfg
branch
from
fac79e8
to
e5ebcd9
Compare
| @@ -245,6 +245,10 @@ ENV INSTALL_PROVIDERS_FROM_SOURCES=${INSTALL_PROVIDERS_FROM_SOURCES} | |||
| ARG UPGRADE_TO_NEWER_DEPENDENCIES="false" | |||
| ENV UPGRADE_TO_NEWER_DEPENDENCIES=${UPGRADE_TO_NEWER_DEPENDENCIES} | |||
|
|
|||
| # Those are additional constraints that are needed for some extras but we do not want to | |||
There was a problem hiding this comment.
We need to embed those and use pre-commit because we want to handle the situation when somebody updates it in PR . IN this case the scripts from CI will not be used (only master version) so we need to make sure that Dockerfile's
default value is the same as the ARG passed in this case.
The nice thing is that you can have different requirements for PROD image (less providers that are messing with the dependencies) and different for CI image (there we have all providers).
potiuk
force-pushed
the
move-provider-imposed-constraints-out-of-setup-cfg
branch
2 times, most recently
from
efd6274
to
41b6813
Compare
|
Actually, this one is even better now - we have a separate set of those "provider-imposed" requirements in Dockerfile.ci and in Dockerfile. This means that even our production docker imaage will not have limitations (like |
|
The Workflow run is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason. |
potiuk
force-pushed
the
move-provider-imposed-constraints-out-of-setup-cfg
branch
from
41b6813
to
7752339
Compare
potiuk
force-pushed
the
move-provider-imposed-constraints-out-of-setup-cfg
branch
7 times, most recently
from
e41b6fa
to
1027401
Compare
|
The Workflow run is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason. |
potiuk
force-pushed
the
move-provider-imposed-constraints-out-of-setup-cfg
branch
2 times, most recently
from
62e7432
to
c38fd26
Compare
This change removes the provider-imposed requirements from the airflow's setup.cfg to additional configuration in the breeze/CI scripts. This does not change constraint apprach when installing airflow, the constraints to those versions remain as they were, but airflow package does not have to have the limits in 'install_requires' section which makes it much more "standalone. We can add more requirements there as needed or remove them when provider's dependencies change. Also thanks to using --upgrade-to-newer-dependencies flag in Breeze, the instructions on what to do when there is a problem with conflicting dependencies are much simpler. You do not need any more to set the label in PR to test how upgrade to newer dependencies will look like, you can test it yourself locally. This is a final step of making airflow package fully independent from the provider's dependencies.
potiuk
force-pushed
the
move-provider-imposed-constraints-out-of-setup-cfg
branch
from
c38fd26
to
1c0c58e
Compare
The recent change apache#13409 introduced common installation script but one case turned out to be not working. Installing Airflow in editable mode with eager upgrade fails on building production image (the CI image effectively did not have airflow installed in editable mode, which it should). This PR fixes that.
The recent change #13409 introduced common installation script but one case turned out to be not working. Installing Airflow in editable mode with eager upgrade fails on building production image (the CI image effectively did not have airflow installed in editable mode, which it should). This PR fixes that.
This PR improves building production image from local packages, in preparation for moving provider requirements out of setup.cfg. Previously `pip download` step was executed in the CI scripts in order to download all the packages that were needed. However this had two problems: 1) PIP download was executed outside of Dockerfile in CI scripts which means that any change to requirements there could not be executed in 'workflow_run' event - because main branch version of CI scripts is used there. We want to add extra requirements when installing airflow so in order to be able to change it, those requirements should be added in Dockerfile. This will be done in the follow-up #13409 PR. 2) Packages downloaded with PIP download have a "file" version rather than regular == version when you run pip freeze/check. This looks weird and while you can figure out the version from file name, when you `pip install` them, they look much more normal. The airflow package and provider package will still get the "file" form but this is ok because we are building those packages from sources and they are not yet available in PyPI. Example: adal==1.2.5 aiohttp==3.7.3 alembic==1.4.3 amqp==2.6.1 apache-airflow @ file:///docker-context-files/apache_airflow-2.1.0.dev0-py3-none-any.whl apache-airflow-providers-amazon @ file:///docker-context-files/apache_airflow_providers_amazon-1.0.0-py3-none-any.whl apache-airflow-providers-celery @ file:///docker-context-files/apache_airflow_providers_celery-1.0.0-py3-none-any.whl ... With this PR, we do not `pip download` all packages, but instead we prepare airflow + providers packages as .whl files and install them from there (all the dependencies are installed from PyPI) (cherry picked from commit e436883)
This change removes the provider-imposed requirements from the airflow's setup.cfg to additional configuration in the breeze/CI scripts. This does not change constraint apprach when installing airflow, the constraints to those versions remain as they were, but airflow package does not have to have the limits in 'install_requires' section which makes it much more "standalone. We can add more requirements there as needed or remove them when provider's dependencies change. Also thanks to using --upgrade-to-newer-dependencies flag in Breeze, the instructions on what to do when there is a problem with conflicting dependencies are much simpler. You do not need any more to set the label in PR to test how upgrade to newer dependencies will look like, you can test it yourself locally. This is a final step of making airflow package fully independent from the provider's dependencies. (cherry picked from commit f49f36b)
The recent change #13409 introduced common installation script but one case turned out to be not working. Installing Airflow in editable mode with eager upgrade fails on building production image (the CI image effectively did not have airflow installed in editable mode, which it should). This PR fixes that. (cherry picked from commit 543194d)
Based on #13556
Please take a look at the last commit only as it is based on #13422
This change removes the provider-imposed requirements from the
airflow's setup.cfg to additional configuration in the
breeze/CI scripts. This does not change constraint apprach
when installing airflow, the constraints to those versions
remain as they were, but airflow package does not have to
have the limits in 'install_requires' section which makes
it much more "standalone.
We can add more requirements there as needed or remove
them when provider's dependencies change.
Also thanks to using --upgrade-to-newer-dependencies flag in
Breeze, the instructions on what to do when there is
a problem with conflicting dependencies are much simpler.
This is a final step of making airflow package fully
independent from the provider's dependencies.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.