Skip to content

Modernize usage of PIP in Airflow images #20726

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.

Sign up for GitHub

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

Jump to bottom
Merged
potiuk merged 1 commit into from
Jan 7, 2022
Merged

Modernize usage of PIP in Airflow images #20726

potiuk merged 1 commit into from
Jan 7, 2022

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Jan 6, 2022

  • removes PIP_INSTALL_USER variable
  • upgrades PIP to 21.3.1
  • removes AIRFLOW_INSTALL_USER_FLAG as it is not needed
  • removes spurious usage of --upgrade flag for PIP
  • adds better diagnostics during the build for PIP location and version

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

@potiuk potiuk requested review from ashb and mik-laj as code owners January 6, 2022 16:50
@boring-cyborg boring-cyborg bot added the area:production-image Production image improvements and fixes label Jan 6, 2022
@potiuk potiuk requested review from eladkal, kaxil and uranusjr January 6, 2022 16:51
@potiuk
Copy link
Member Author

potiuk commented Jan 6, 2022

This is the same as previously approved #20678 (and merge-squashed in #20679) but with fixed bug that caused PROD tests to fail (the bug was that PIP_USER was missing in the build segment resulting in empty ".local" folder in final image. This made teh image much smaller (50%) but also much less useful (airflow was not installed effectively).

@potiuk potiuk marked this pull request as draft January 6, 2022 16:54
@potiuk
Copy link
Member Author

potiuk commented Jan 6, 2022

(I did not cherry-pick properly- fixing it)

@potiuk potiuk force-pushed the modernizes-pip-usage branch from 6401f0e to 73a2a98 Compare January 6, 2022 17:01
@potiuk potiuk marked this pull request as ready for review January 6, 2022 17:01
@potiuk potiuk requested a review from jedcunningham as a code owner January 6, 2022 17:01
@potiuk
Copy link
Member Author

potiuk commented Jan 6, 2022

OK. It's ready for re-review.

@potiuk
Modernize usage of PIP in Airflow images
c959186 
* remove PIP_INSTALL_USER variable
* upgrade PIP to 21.3.1
* remove AIRFLOW_INSTALL_USER_FLAG as it is not needed
* remove spurious usage of --upgrade flag for PIP
* add better diagnostics during the build for PIP location and version

Separated out from apache#20238
@potiuk potiuk force-pushed the modernizes-pip-usage branch from 73a2a98 to c959186 Compare January 6, 2022 17:04
@potiuk potiuk changed the title Modernizes usage of PIP in Airflow images Modernize usage of PIP in Airflow images Jan 6, 2022
@potiuk
Copy link
Member Author

potiuk commented Jan 6, 2022

Looks like it's good to go :) . The failure of one of the tests seems like virtual machine killed mid-flight, Not a real error.

@potiuk
Copy link
Member Author

potiuk commented Jan 6, 2022

Yep. Just one failure - coming from killing the machine.

@potiuk
Copy link
Member Author

potiuk commented Jan 6, 2022

@ashb @uranusjr -> I am waiting with the next one to avoid the confusion with multiple commits per pr

uranusjr
uranusjr approved these changes Jan 7, 2022
View reviewed changes
Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions
Copy link

github-actions bot commented Jan 7, 2022

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 main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Jan 7, 2022
@potiuk potiuk merged commit 0635b97 into apache:main Jan 7, 2022
@potiuk potiuk deleted the modernizes-pip-usage branch January 7, 2022 10:39
@potiuk potiuk added this to the Airflow 2.2.4 milestone Jan 20, 2022
@potiuk potiuk added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Jan 22, 2022
potiuk added a commit that referenced this pull request Jan 22, 2022
@potiuk
Modernize usage of PIP in Airflow images (#20726)
3fea595 
* remove PIP_INSTALL_USER variable
* upgrade PIP to 21.3.1
* remove AIRFLOW_INSTALL_USER_FLAG as it is not needed
* remove spurious usage of --upgrade flag for PIP
* add better diagnostics during the build for PIP location and version

Separated out from #20238

(cherry picked from commit 0635b97)
jedcunningham pushed a commit that referenced this pull request Jan 27, 2022
@potiuk @jedcunningham
Modernize usage of PIP in Airflow images (#20726)
a4c57e5 
* remove PIP_INSTALL_USER variable
* upgrade PIP to 21.3.1
* remove AIRFLOW_INSTALL_USER_FLAG as it is not needed
* remove spurious usage of --upgrade flag for PIP
* add better diagnostics during the build for PIP location and version

Separated out from #20238

(cherry picked from commit 0635b97)
@potiuk potiuk restored the modernizes-pip-usage branch April 26, 2022 20:47
@potiuk potiuk deleted the modernizes-pip-usage branch July 29, 2022 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@uranusjr uranusjr uranusjr approved these changes

@ashb ashb Awaiting requested review from ashb ashb is a code owner

@mik-laj mik-laj Awaiting requested review from mik-laj

@eladkal eladkal Awaiting requested review from eladkal

@kaxil kaxil Awaiting requested review from kaxil kaxil is a code owner

@jedcunningham jedcunningham Awaiting requested review from jedcunningham jedcunningham is a code owner

Assignees

No one assigned

Labels

area:production-image Production image improvements and fixes changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) full tests needed We need to run full set of tests for this PR to merge

Projects

None yet

Milestone

Airflow 2.2.4

Development

Successfully merging this pull request may close these issues.

2 participants

@potiuk @uranusjr