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

[WIP] docker-py: use pip to install dependencies #38526

Open
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@thaJeztah
Copy link
Member

thaJeztah commented Jan 10, 2019

follow-up to #38524

This makes sure that we have all the requirements installed and that they have the right version.

Moved the installation earlier in the Dockerfile, as docker-py is not so frequently updated in this repository, so optimizing for caching.

Removed all the apt-packages that are not needed.

@thaJeztah thaJeztah force-pushed the thaJeztah:use_pip_for_docker_py branch from 182fe8e to 9b6fd2a Jan 10, 2019

@codecov

This comment has been minimized.

Copy link

codecov bot commented Jan 10, 2019

Codecov Report

Merging #38526 into master will increase coverage by 0.07%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #38526      +/-   ##
==========================================
+ Coverage   36.64%   36.71%   +0.07%     
==========================================
  Files         608      608              
  Lines       45173    45230      +57     
==========================================
+ Hits        16553    16608      +55     
+ Misses      26339    26333       -6     
- Partials     2281     2289       +8
@olljanat

This comment has been minimized.

Copy link
Contributor

olljanat commented Jan 10, 2019

PowerPC failure looks like related.

@olljanat

This comment has been minimized.

Copy link
Contributor

olljanat commented Jan 10, 2019

(reserved for my derek commands)

@derek derek bot added the status/failing-ci label Jan 10, 2019

@cpuguy83

This comment has been minimized.

Copy link
Contributor

cpuguy83 commented Jan 10, 2019

Nice, I wonder if we can go even further and install docker-py deps into their own image and then COPY them in at the end.

pip install --target would allow us to customize the path at least or can just use the default path.

thaJeztah added some commits Jan 9, 2019

Update docker-py to 3.7.0
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
docker-py: use pip to install dependencies
This makes sure that we have all the requirements installed
and that they have the right version.

Moved the installation earlier in the Dockerfile, as
docker-py is not so frequently updated in this repository,
so optimizing for caching.

Removed all the apt-packages that are not needed.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

@thaJeztah thaJeztah force-pushed the thaJeztah:use_pip_for_docker_py branch from 9b6fd2a to 62878ae Jan 11, 2019

Update to Python 3
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

@thaJeztah thaJeztah force-pushed the thaJeztah:use_pip_for_docker_py branch from a3eaf17 to 141d867 Jan 11, 2019

Skip python installation on non-x64
We only run these tests on x86, so skip for other architectures

This is a dirty hack, until we start using BuildKit, at whith time
we'll be able to skip stages.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@kolyshkin

This comment has been minimized.

Copy link
Contributor

kolyshkin commented Jan 11, 2019

I would argue that using distro packages should be preferred to using pip -- distro packages are faster to install and (supposedly?) better maintained so more stable. The current approach is to install as much as we can from the distro packages, and use pip for the rest.

The only downside I see is packages to be installed are spread between two places in Dockerfile, and it's not bad enough per se to switch to pip entirely. @thaJeztah perhaps I'm missing something -- what is your motivation to prefer pip to apt install?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment