-
Notifications
You must be signed in to change notification settings - Fork 15.3k
Unify selecting constraints option when installing airflow #52274
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
Unify selecting constraints option when installing airflow #52274
Conversation
I will have to test it also with "upgrade with newer dependencies" and "canary" before I merge it. |
Ok - now I have consistent at least (and failing) - and can fix it :) |
Due to the way how it historically got added - we had two ways of selecting whether we are installing airlfow dyanmically in breeze with or without constraints: * --install-airflow-with-constraints - was used in a few places * --skip-airflow-constraints - was used in other places The logic to handle those were broken at places where they contradicted each other. This PR unifies it and only uses the --install-airflow-with-constraints flag in all the places where we need to determine whether constraints are used or not and it fixes the logic. The logic of installation had been reviewed, refactored into separate methods doing smaller tasks and more diagnostics was added.
3ca47a0
to
4474d0f
Compare
I refactored the installation script and split the long and complex method into smaller, separate methods doing things in much easiert to understand way. |
Let me try with |
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.
Thanks Nice removal :)
Indeed seems like it is going to be easier to understand it now - also for me :) |
Backport failed to create: v3-0-test. View the failure log Run details
You can attempt to backport this manually by running: cherry_picker 8846bef v3-0-test This should apply the commit to the v3-0-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continue |
apache#52274) Due to the way how it historically got added - we had two ways of selecting whether we are installing airlfow dyanmically in breeze with or without constraints: * --install-airflow-with-constraints - was used in a few places * --skip-airflow-constraints - was used in other places The logic to handle those were broken at places where they contradicted each other. This PR unifies it and only uses the --install-airflow-with-constraints flag in all the places where we need to determine whether constraints are used or not and it fixes the logic. The logic of installation had been reviewed, refactored into separate methods doing smaller tasks and more diagnostics was added. (cherry picked from commit 8846bef) Co-authored-by: Jarek Potiuk <[email protected]>
apache#52274) Due to the way how it historically got added - we had two ways of selecting whether we are installing airlfow dyanmically in breeze with or without constraints: * --install-airflow-with-constraints - was used in a few places * --skip-airflow-constraints - was used in other places The logic to handle those were broken at places where they contradicted each other. This PR unifies it and only uses the --install-airflow-with-constraints flag in all the places where we need to determine whether constraints are used or not and it fixes the logic. The logic of installation had been reviewed, refactored into separate methods doing smaller tasks and more diagnostics was added. (cherry picked from commit 8846bef) Co-authored-by: Jarek Potiuk <[email protected]>
apache#52274) Due to the way how it historically got added - we had two ways of selecting whether we are installing airlfow dyanmically in breeze with or without constraints: * --install-airflow-with-constraints - was used in a few places * --skip-airflow-constraints - was used in other places The logic to handle those were broken at places where they contradicted each other. This PR unifies it and only uses the --install-airflow-with-constraints flag in all the places where we need to determine whether constraints are used or not and it fixes the logic. The logic of installation had been reviewed, refactored into separate methods doing smaller tasks and more diagnostics was added. (cherry picked from commit 8846bef) Co-authored-by: Jarek Potiuk <[email protected]>
Due to the way how it historically got added - we had two ways of selecting whether we are installing airlfow dyanmically in breeze with or without constraints:
The logic to handle those were broken at places where they contradicted each other. This PR unifies it and only uses the --install-airflow-with-constraints flag in all the places where we need to determine whether constraints are used or not and it fixes the logic.
The logic of installation had been reviewed, refactored into separate methods doing smaller tasks and more diagnostics was added.
^ 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 airflow-core/newsfragments.