Skip to content
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

Use only-if-needed upgrade strategy to install deps for PRs #11363

Merged
merged 1 commit into from
Oct 9, 2020

Conversation

kaxil
Copy link
Member

@kaxil kaxil commented Oct 8, 2020

Currently, upgrading dependencies in setup.py still run with previous versions of the package for the PR which fails.

This PR will change it to only upgrade the package if a requirement is not satisfied (i.e. if we change the upper or lower bound of dependency in setup.py).

This will avoid the situation like #11020 and #11313 where we can't test upgrading the package in the PR itself. And we were "deadlocked" even after merging the PR as the constraints won't allow installing the new version of the package, I had to generate new constraints myself and push to constraints-master branch (e6e840f), this is not ideal and won't be possible for just contributors.

Using only-if-needed upgrade strategy - packages are only upgraded if they are named in the pip command or a requirement file (i.e, they are direct requirements), or an upgraded parent needs a later version of the dependency than is currently installed.

Example:

Using only-if-needed:

root@822984cd3cf4:/opt/airflow# pip install -e . --upgrade --upgrade-strategy only-if-needed
Obtaining file:///opt/airflow
Requirement already satisfied, skipping upgrade: alembic<2.0,>=1.2 in /usr/local/lib/python3.7/site-packages (from apache-airflow==2.0.0.dev0) (1.4.3)
Requirement already satisfied, skipping upgrade: jinja2<2.12.0,>=2.10.1 in /usr/local/lib/python3.7/site-packages (from apache-airflow==2.0.0.dev0) (2.11.2)
Requirement already satisfied, skipping upgrade: json-merge-patch==0.2 in /usr/local/lib/python3.7/site-packages (from apache-airflow==2.0.0.dev0) (0.2)
Requirement already satisfied, skipping upgrade: tabulate<0.9,>=0.7.5 in /usr/local/lib/python3.7/site-packages (from apache-airflow==2.0.0.dev0) (0.8.7)
Requirement already satisfied, skipping upgrade: graphviz>=0.12 in /usr/local/lib/python3.7/site-packages (from apache-airflow==2.0.0.dev0) (0.14.1)
Collecting tenacity~=6.2.0
  Downloading tenacity-6.2.0-py2.py3-none-any.whl (24 kB)
...
...

vs

Using eager:

root@822984cd3cf4:/opt/airflow# pip install -e . --upgrade --upgrade-strategy eager
Obtaining file:///opt/airflow
Requirement already up-to-date: alembic<2.0,>=1.2 in /usr/local/lib/python3.7/site-packages (from apache-airflow==2.0.0.dev0) (1.4.3)
Requirement already up-to-date: argcomplete~=1.10 in /usr/local/lib/python3.7/site-packages (from apache-airflow==2.0.0.dev0) (1.12.1)
Requirement already up-to-date: cryptography>=0.9.3 in /usr/local/lib/python3.7/site-packages (from apache-airflow==2.0.0.dev0) (3.1.1)
Requirement already up-to-date: dill<0.4,>=0.2.2 in /usr/local/lib/python3.7/site-packages (from apache-airflow==2.0.0.dev0) (0.3.2)
Requirement already up-to-date: funcsigs<2.0.0,>=1.0.0 in /usr/local/lib/python3.7/site-packages (from apache-airflow==2.0.0.dev0) (1.0.2)
Collecting graphviz>=0.12
  Downloading graphviz-0.14.2-py2.py3-none-any.whl (18 kB)
....
....

^ 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.

Currently, upgrading dependencies in setup.py still runs with previous versions of the package for the PR which fails.

This will change to upgrade only the package that is required for the PRs
@kaxil kaxil changed the title Use only-if-needed upgrade strategy for PRs Use only-if-needed upgrade strategy to install deps for PRs Oct 8, 2020
@kaxil
Copy link
Member Author

kaxil commented Oct 8, 2020

#11020 is blocked without this PR

cc @baolsen

@potiuk
Copy link
Member

potiuk commented Oct 9, 2020

Cool!

@potiuk potiuk merged commit 7f674c6 into apache:master Oct 9, 2020
@kaxil kaxil deleted the upgrade-only-if-needed branch October 9, 2020 08:50
@potiuk potiuk added this to the Airflow 1.10.13 milestone Oct 11, 2020
potiuk pushed a commit that referenced this pull request Oct 11, 2020
Currently, upgrading dependencies in setup.py still runs with previous versions of the package for the PR which fails.

This will change to upgrade only the package that is required for the PRs

(cherry picked from commit 7f674c6)
RaviTezu pushed a commit to RaviTezu/airflow that referenced this pull request Oct 25, 2020
Currently, upgrading dependencies in setup.py still runs with previous versions of the package for the PR which fails.

This will change to upgrade only the package that is required for the PRs

(cherry picked from commit 7f674c6)
kaxil added a commit that referenced this pull request Nov 12, 2020
Currently, upgrading dependencies in setup.py still runs with previous versions of the package for the PR which fails.

This will change to upgrade only the package that is required for the PRs

(cherry picked from commit 7f674c6)
kaxil added a commit that referenced this pull request Nov 13, 2020
Currently, upgrading dependencies in setup.py still runs with previous versions of the package for the PR which fails.

This will change to upgrade only the package that is required for the PRs

(cherry picked from commit 7f674c6)
@potiuk potiuk added the type:misc/internal Changelog: Misc changes that should appear in change log label Nov 14, 2020
cfei18 pushed a commit to cfei18/incubator-airflow that referenced this pull request Mar 5, 2021
Currently, upgrading dependencies in setup.py still runs with previous versions of the package for the PR which fails.

This will change to upgrade only the package that is required for the PRs

(cherry picked from commit 7f674c6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dev-tools type:misc/internal Changelog: Misc changes that should appear in change log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants