-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
When installing providers from the context folder, use pip --find-links
#15673
Conversation
…nks` Without this change it is impossible for one of the providers to depend upon the "dev"/current version of Airflow -- pip instead would try and go out to PyPI to find the version (which almost certainly wont exist, as it hasn't been released yet)
/cc @uranusjr |
This is strange. It should not be needed because we are installing both providers and airflow from the "/docker-context-files" directory by simply specifying them explicitely as arguments - the .whl packages for both airflow and provider should be there, built from the current sources so it should work all right. |
Might be a bug in PIP @uranusjr Just for the record and to explicitly state the problem:, the failed installation command was like below. So as far as I understand PIP should work if amazon specifies
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, The only question I have is why it did not work in the first place :)?
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease. |
I suspect that by providing .whl feels it bypasses some part of the resolver, and thus doesn't notice that it's already "got" the 2.1.0dev0 of Airflow available. |
IIRC there is some kind of ordering involved, if you provide the wheel path before all packages depending on it[1] pip will work as expected. But installing from path is generally frawned upon anyway (it’s a non-standard pip extension) and we generally advice [1]: something like that, I don’t recall the exact details from the top of my head. One of the other pip maintainers likens the process to put your left hand on your head and turn right three times and sacrafice the heart of a goat. |
Cool. Thanks for clarification! |
Yeah the goat was definitely missing here. |
…nks` (apache#15673) Without this change it is impossible for one of the providers to depend upon the "dev"/current version of Airflow -- pip instead would try and go out to PyPI to find the version (which almost certainly wont exist, as it hasn't been released yet) (cherry picked from commit 13faa69)
Without this change it is impossible for one of the providers to depend upon the "dev"/current version of Airflow -- pip instead would try and go out to PyPI to find the version (which almost certainly wont exist, as it hasn't been released yet)
This was noticed in my draft PR #15667 and caused the prod images to fail to build.
I tested this change locally with
./breeze build-image --production-image --disable-pypi-when-building --install-from-docker-context-files
:^ 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.