[CIVP-10579] move to pip only? #24

beckermr · 2017-03-10T23:20:54Z

This PR moves this image to a pip-only install. We may not do this, but let's see what happens.

beckermr · 2017-03-12T15:30:09Z

@mheilman @stephen-hoover Take a look and tell me what you think. I need to update the docs still, but a 30% look at this would be good.

One wrinkle: You have to install numpy before you can install glmnet, so I have to run the pip install -r ... command twice. :(

beckermr · 2017-03-14T18:39:03Z

(hammer) (hammer)

mheilman · 2017-03-15T15:53:42Z

does this make sure numpy is linked to the appropriate linear algebra libraries? it'd probably be good to check this, and maybe run a benchmark too.

in general, i'm skeptical that this will make our lives easier, but it also probably won't make things much worse.

beckermr · 2017-03-15T15:57:12Z

I am skeptical as well. It tests scipy, but not numpy.

https://github.com/civisanalytics/datascience-python/blob/master/circle.yml#L12

stephen-hoover

I see the warning

warning: building with the bundled copy of libffi is deprecated on this platform. It will not be distributed with Python 3.7
Let's add libffi to the packages we pull.

When I build this image locally, the size has decreased from 1.89 GB on latest to 1.32 GB here. That's a big change. If anything, I would have expected the size to increase with this PR. Any ideas why it's smaller? Smaller is great, but I want to be sure it's not signaling a problem somewhere.

Also, just a note: The Python build gives

The necessary bits to build these optional modules were not found:
_dbm _gdbm _tkinter

I think that's fine; AFAIK we don't need DBM, and I don't expect anyone to be running tkinter.

stephen-hoover · 2017-03-15T19:17:57Z

Dockerfile

+        libssl-dev \
+        make \
+        xz-utils \
+        zlib1g-dev' && \


Why not install these with the first apt-get? It seems unnecessarily obfuscated as a string here.

stephen-hoover · 2017-03-15T19:18:35Z

Dockerfile

+    curl -fSL "https://www.python.org/ftp/python/${PYTHON_VERSION%%[a-z]*}/Python-$PYTHON_VERSION.tar.xz.asc" -o python.tar.xz.asc && \
+    export GNUPGHOME="$(mktemp -d)" && \
+    gpg --keyserver pgp.mit.edu --recv-keys "$GPG_KEY" && \
+    gpg --batch --verify python.tar.xz.asc python.tar.xz && \


I get the warning

gpg: keyring /tmp/tmp.nL93R0YNBb/secring.gpg' created gpg: keyring /tmp/tmp.nL93R0YNBb/pubring.gpg' created
gpg: requesting key AA65421D from hkp server pgp.mit.edu
gpg: /tmp/tmp.nL93R0YNBb/trustdb.gpg: trustdb created
gpg: key AA65421D: public key "Ned Deily (Python release signing key) nad@python.org" imported
gpg: no ultimately trusted keys found
gpg: Total number processed: 1
gpg: imported: 1 (RSA: 1)
gpg --batch --verify python.tar.xz.asc python.tar.xz
gpg: Signature made Fri 23 Dec 2016 02:53:10 AM UTC using RSA key ID AA65421D
gpg: Good signature from "Ned Deily (Python release signing key) nad@python.org"
gpg: aka "Ned Deily nad@baybryj.net"
gpg: aka "keybase.io/nad nad@keybase.io"
gpg: aka "Ned Deily (Python release signing key) nad@acm.org"
gpg: WARNING: This key is not certified with a trusted signature!
gpg: There is no indication that the signature belongs to the owner.
Primary key fingerprint: 0D96 DF4D 4110 E5C4 3FBF B17F 2D34 7EA6 AA65 421D

Is that expected?

Privacy is hard? :)

I don't understand pgp well enough to say what is going on here though I do have an idea.

stephen-hoover · 2017-03-15T19:21:16Z

Dockerfile

+    rm python.tar.xz && \
+    \
+    cd /usr/src/python && \
+    ./configure --enable-shared --enable-unicode=ucs4 && \


This line gives the warning

configure: WARNING: unrecognized options: --enable-unicode

What is that option supposed to do? We should correct it if we need it, or remove it if we don't.

I also get

If you want a release build with all optimizations active (LTO, PGO, etc),
please run ./configure --enable-optimizations

I think we should use that option: http://stackoverflow.com/questions/41405728/what-does-enable-optimizations-do-while-compiling-python

See also discussion here: docker-library/python#160

We are getting a segfault like the issue you linked above with --enable-optimizations. I think we should pass.

stephen-hoover · 2017-03-15T19:23:30Z

Dockerfile

 RUN python -c "import matplotlib.pyplot"

-ENV VERSION=2.0.1 \
-    VERSION_MAJOR=2 \
+ENV VERSION=3.0.0 \


What does this change about the environment? Ideally, this change is only an implementation detail and should be in the next minor version.

Oh, right. Conda is gone completely. That would be a breaking change for anyone who was conda-installing.

Yep. Scary.

stephen-hoover · 2017-03-15T19:31:12Z

Dockerfile

-# Ensure UTF-8 locale.
-RUN locale-gen en_US.UTF-8
+# We need this file to install the right version of numpy.
+COPY requirements.txt /requirements.txt


This is the part of the image which will change most frequently. Let's put it near the end to avoid recompiling Python with every package version change. I think you can combine the

pip3 install --no-cache-dir `grep 'numpy' /requirements.txt` && \ + find /usr/local -depth \ + \( \ + \( -type d -a -name test -o -name tests \) \ + -o \ + \( -type f -a -name '*.pyc' -o -name '*.pyo' \) \ + \) -exec rm -rf '{}' + && \ + rm -rf /usr/src/python ~/.cache

onto the same layer with the other pip installs, rather than in the Python installation layer.

The requirements is at the top in order to install numpy right after the build process. This can be moved. I was just trying to get this to work once.

stephen-hoover · 2017-03-15T19:32:21Z

Dockerfile

-    conda clean --all -y
+# Now install the rest of the dependencies from the requirements file.
+RUN pip install --no-cache-dir -r /requirements.txt
+RUN rm -rf ~/.cache/*


The way Docker works, there's not much point to having an rm on its own line. The files would still be in the stack of layers we download. This should be combined into the previous line. (Is there anything in the cache to clear, or is this just to be sure?)

stephen-hoover · 2017-03-15T19:35:29Z

If this really does cut the size of the image by 500 MB, then I'm in favor. Compiling our own Python is a bit of pain, but we only have to do it once.

beckermr · 2017-03-16T17:33:29Z

Latest build is at 1.31 GB. Not bad. We have about ~850 MB of python packages, of which TensorFlow is the biggest at ~160 MB.

beckermr · 2017-03-16T17:37:17Z

Linear algebra in numpy seems to work, @mheilman.

beckermr · 2017-03-16T17:38:49Z

@stephen-hoover I think the latest version addresses all of your comments in various ways.

beckermr · 2017-03-17T13:53:07Z

I am closing this for now. We can revisit later if needed.

stephen-hoover

I'd add miniconda too.

stephen-hoover · 2017-03-17T13:01:16Z

Dockerfile

-      libffi-dev \
-      gfortran \
-      g++ \
-      git \


We need to keep git installed in the container. It's useful for installing new packages which aren't on pypi.

Whoops! I did not see that.

stephen-hoover · 2017-03-17T14:04:39Z

Dockerfile

-    CIVIS_CONDA_VERSION=4.3.11 \
-    CIVIS_PYTHON_VERSION=3.6.0
-
-RUN DEBIAN_FRONTEND=noninteractive apt-get update -y --no-install-recommends && \


I think we need to keep the "DEBIAN_FRONTEND=noninteractive" bit.

beckermr force-pushed the move-to-pip branch from 980add0 to c8f588b Compare March 12, 2017 15:26

beckermr changed the title ~~WIP start to move to pip only~~ [CIVP-10579] move to pip only? Mar 13, 2017

ENH move to a pure pip install

61077f3

beckermr force-pushed the move-to-pip branch from 166465e to 61077f3 Compare March 14, 2017 18:39

DEP remove old code

01798a7

stephen-hoover suggested changes Mar 15, 2017

View reviewed changes

beckermr added 7 commits March 16, 2017 10:22

ENH move to docker build straight from the source!

942d6fb

BUG silly bug

fba06f1

ENH tabs to spaces, adding libffi

8c32807

BUG maybe this is it?

994ef93

BUg add postgressql

555300d

ENH added more deps I hope

3f4488f

ENH try again...

486cd73

beckermr closed this Mar 17, 2017

stephen-hoover reviewed Mar 17, 2017

View reviewed changes

beckermr reopened this Mar 17, 2017

beckermr and others added 4 commits March 17, 2017 09:24

ENH add miniconda and git

e23b1ac

ENH more conda stuff

f17cdd3

ENH roll back condarc changes

0c770a8

ENH force conda to remove python

f711221

beckermr closed this Mar 17, 2017

salilgupta1 deleted the move-to-pip branch March 5, 2020 20:01

[CIVP-10579] move to pip only? #24

[CIVP-10579] move to pip only? #24

Uh oh!

Conversation

beckermr commented Mar 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

beckermr commented Mar 12, 2017

Uh oh!

beckermr commented Mar 14, 2017

Uh oh!

mheilman commented Mar 15, 2017

Uh oh!

beckermr commented Mar 15, 2017

Uh oh!

stephen-hoover left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stephen-hoover Mar 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stephen-hoover commented Mar 15, 2017

Uh oh!

beckermr commented Mar 16, 2017

Uh oh!

beckermr commented Mar 16, 2017

Uh oh!

beckermr commented Mar 16, 2017

Uh oh!

beckermr commented Mar 17, 2017

Uh oh!

stephen-hoover left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

beckermr commented Mar 10, 2017 •

edited

Loading

stephen-hoover Mar 15, 2017 •

edited

Loading