This is just an initial concept to break apart our Docker files. #30

ericholscher · 2017-03-08T20:52:53Z

This addresses #29,
and gives us a base image that just has the Python environment,
VCS tools,
and a couple other standard utilities.

Then it has a full image that adds all of the PDF/C libraries/etc.
This is where most of the time and bloat comes from,
and isn't actually required for most testing,
HTML builds,
conda builds,
and lots of other use cases.

This would allow us to interate and test base builds faster,
while keeping the environment the same in production.

NOTE: I'm mostly looking for feedback on this concept,
not specific nitpicking.

agjohnson

Not sure if you meant to merge to releases/2.x or not, but there are a number of regressions here. Also missing: readme and contributing updates. I think some of this can be simplified by not having /base/Dockerfile and /full/Dockerfile, but /Dockerfile and base/Dockerfile

agjohnson · 2017-03-08T21:50:15Z

base/Dockerfile

+# System dependencies
+RUN apt-get -y update
+RUN apt-get -y install vim software-properties-common python-setuptools \
+    python3-setuptools python3 python3-pip python-dev python3-dev


Why is this using the base python3 again? The latest image was using pyenv installed python.

agjohnson · 2017-03-08T21:50:38Z

base/Dockerfile

+
+FROM ubuntu:16.04
+MAINTAINER Read the Docs <support@readthedocs.com>
+LABEL version="latest"


This shouldn't be latest

agjohnson · 2017-03-08T21:51:04Z

base/Dockerfile

+WORKDIR /home/docs
+RUN curl -O https://repo.continuum.io/miniconda/Miniconda2-4.3.11-Linux-x86_64.sh
+RUN bash Miniconda2-4.3.11-Linux-x86_64.sh -b -p /home/docs/miniconda2/
+env PATH $PATH:/home/docs/miniconda2/bin


This also removes recent commits to repair the conda path

agjohnson · 2017-03-08T21:52:27Z

full/Dockerfile

+RUN apt-get -y install libpq-dev libxml2-dev libxslt-dev libxslt1-dev postgresql-client libmysqlclient-dev
+
+# from readthedocs.build
+RUN apt-get -y install libfreetype6 g++ sqlite libevent-dev libffi-dev \


Lumping all dependencies into one image is a bad idea. If there is a problem installing any one of these, the entire image is hosed and needs to be restarted. It's not common, but in development I hit this a number of times.

agjohnson · 2017-03-08T21:53:06Z

full/Dockerfile

+# Read the Docs - Environment base
+FROM readthedocs/base:latest
+MAINTAINER Read the Docs <support@readthedocs.com>
+LABEL version="2.0"


This is being merged to master, the version is not 2.0 there.

ericholscher · 2017-03-08T22:00:59Z

As I noted above, I was looking for feedback on the approach, not specific issues. Is it worth spending the time to get everything set up and working, or is it the wrong avenue?

humitos

I like the idea of having a base image as simple as it could be to just handle the most common build scenarios and some other images with full support for different things, maybe they can be also split into pdf and c/c++ libraries

agjohnson · 2017-03-08T22:15:47Z

I think it's close, there shouldn't be much conflict in have the images split like this. I mentioned above, besides the inconsistencies in the Dockerfile, I think a better pattern is that the main Dockerfile stays in the root path, and base/Dockerfile is the base image. This aligns with the pattern for extra images already.

ericholscher · 2017-03-08T22:16:35Z

The base Dockerfile will have more things in it though? So the root Dockerfile would inherent from base, but be the full concept?

This addresses #29, and gives us a `base` image that just has the Python environment, VCS tools, and a couple other standard utilities. Then it has a `full` image that adds all of the PDF/C libraries/etc. This is where most of the time and bloat comes from, and isn't actually required for most testing, HTML builds, conda builds, and lots of other use cases. This would allow us to interate and test base builds faster, while keeping the environment the same in production. NOTE: I'm mostly looking for feedback on this PR, not specific nitpicking.

humitos · 2017-03-08T22:17:55Z

@ericholscher yeah, I think the root Dockerfile should be the latest full (with everything)

agjohnson · 2017-03-08T22:18:22Z

Yeah 👍 . That way, it's very obvious which Dockerfile is The One. Development could happen from base/Dockerfile or readthedocs/build:base in the case you don't want latex and friends.

agjohnson · 2017-03-08T22:21:29Z

We'd probably also want this pattern for releases/2.x branch and master, so there will be some duplication of effort here.

agjohnson · 2017-03-08T22:24:55Z

Oh and because the full image will install a huge amount of dependencies last, this should point out that it's easier to develop and test the base image alone, before building the full image. Trying to develop on the full image will be very painful, as any changes up front will require reinstalling GB of latex and friends.

ericholscher · 2017-03-08T22:31:13Z

Hmm -- is there a way to build a base-full image or something with all the reqs, and merge that in the root one before we do the RTD specific stuff? That might be too complicated tho.

humitos · 2017-03-08T22:32:32Z

Dockerfile

 # Install requirements
-RUN apt-get -y install \
-    bzr subversion git-core mercurial libpq-dev libxml2-dev libxslt-dev \
+RUN apt-get -y install libpq-dev libxml2-dev libxslt-dev \


I think this will be a problem since we are already as a docs user from the base image :)

agjohnson · 2017-03-08T22:37:13Z

I really wish dockerfile had an include statement :/

I think what you're asking is going to be hard to accomplish, you might be best off just resetting some of the necessary bits like USER or WORKDIR in the /Dockerfile. That is:

FROM readthedocs/build:base

USER root

$STUFF

USER docs

ericholscher · 2017-03-09T19:14:39Z

I've run into a bunch of issues around the installing of scentific packages into the python 3.3 pyenv. Do we really expect people to be building docs with Python 3.3 for some reason? I don't believe it is widely supported, and I can't think of a use case for it.

ericholscher · 2017-03-09T19:18:41Z

Nevermind, I got it working now, with a few dependencies.

humitos · 2017-03-09T19:38:26Z

I've run into a bunch of issues around the installing of scentific packages into the python 3.3 pyenv. Do we really expect people to be building docs with Python 3.3 for some reason?

I have another more general question: why do you want to support something different than 2.7.x (for Python2) and lower than 3.6 (for Python3)?

I mean, why would someone use 3.x instead of 3.6 now? Is there any documentation that builds correctly in 3.x and not in 3.6?

agjohnson · 2017-03-09T20:32:09Z

@humitos i don't think we can be sure of what will or will not build, so it will be the best user experience to let the user decide this

humitos · 2017-03-09T20:37:35Z

@agjohnson ok, so I will ask how did you choose the versions that you support?

Why 3.5.2 instead of 3.5.3?
Why 3.4.5 instead 3.4.6?
Why not also 3.2.6?
Why not others?
Why not 3.7.0a0 so people can start testing strange things?

agjohnson · 2017-03-09T20:51:53Z

The versions off by a bug fix release need to be bumped. This will be regular maintenance. I don't have a strong opinion of 3.2 either way, same goes for 3.3 really. We could offer 3.2, but it is a rather old release at this point. 3.7 alpha release might be interesting, but would require more frequent releases of this image, so for that I'd avoid it until it's an RC. There's some amount of diminishing returns here when selecting versions, i think deciding on the last 4 minor versions (or whatever that number is) should be enough.

ericholscher · 2017-03-09T21:27:02Z

AFAIK almost nobody uses anything before 3.5 in a real production environment, so we can probably drop them.

ericholscher · 2017-03-09T21:28:04Z

Even Django only supports 3.4 and above: https://docs.djangoproject.com/en/dev/releases/1.10/

agjohnson · 2017-03-09T21:40:48Z

I think 3.4 at minimum then, it was the the 3.x on the last build image.

ericholscher · 2017-03-09T22:09:56Z

I might shelve this for now, as it seems like it isn't really going to give us the intended value of having a much smaller docker image. I guess we could ship base with 1 python2 and 1 python3, but that's still gonna end up probably closer to 2GB, so not really saving too much.

agjohnson · 2017-03-09T22:39:28Z

I do think it might be worth exploring some form of dynamic configuration to get around the jank of Dockerfiles. Salt would make sense here and could be kept pretty basic.

ericholscher · 2017-03-18T19:44:55Z

Just some interesting stats about python versions: pypa/pip#4343

…

On Thursday, March 9, 2017, Anthony ***@***.***> wrote: I do think it might be worth exploring some form of dynamic configuration to get around the jank of Dockerfiles. Salt would make sense here and could be kept pretty basic. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#30 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABjprOSU0aeNG8KYGjSShMaufK5HaBQks5rkH-ggaJpZM4MXS4I> .

-- Eric Holscher Maker of the internet residing in Portland, Oregon http://ericholscher.com

ericholscher added the PR: work in progress Pull request is not ready for full review label Mar 8, 2017

ericholscher requested review from humitos and agjohnson March 8, 2017 20:53

agjohnson requested changes Mar 8, 2017

View reviewed changes

humitos reviewed Mar 8, 2017

View reviewed changes

Update to new format

9a822fd

ericholscher force-pushed the base-full-refactor branch from 10cd9b4 to 9a822fd Compare March 8, 2017 22:22

ericholscher added 2 commits March 8, 2017 14:22

Fix formatting

a6d2f2e

Remove redundent packages

ce0c044

humitos reviewed Mar 8, 2017

View reviewed changes

Fix up base image so it builds

88e06f8

Fix build instructions

b796b66

agjohnson closed this Oct 19, 2017

agjohnson removed the PR: work in progress Pull request is not ready for full review label Oct 19, 2017

humitos deleted the base-full-refactor branch September 3, 2020 09:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

This is just an initial concept to break apart our Docker files. #30

This is just an initial concept to break apart our Docker files. #30

ericholscher commented Mar 8, 2017 •

edited

Loading

agjohnson left a comment

agjohnson Mar 8, 2017

agjohnson Mar 8, 2017

agjohnson Mar 8, 2017

agjohnson Mar 8, 2017

agjohnson Mar 8, 2017

ericholscher commented Mar 8, 2017

humitos left a comment

agjohnson commented Mar 8, 2017

ericholscher commented Mar 8, 2017

humitos commented Mar 8, 2017

agjohnson commented Mar 8, 2017 •

edited

Loading

agjohnson commented Mar 8, 2017

agjohnson commented Mar 8, 2017

ericholscher commented Mar 8, 2017

humitos Mar 8, 2017

agjohnson commented Mar 8, 2017

ericholscher commented Mar 9, 2017

ericholscher commented Mar 9, 2017

humitos commented Mar 9, 2017

agjohnson commented Mar 9, 2017

humitos commented Mar 9, 2017 •

edited

Loading

agjohnson commented Mar 9, 2017

ericholscher commented Mar 9, 2017

ericholscher commented Mar 9, 2017

agjohnson commented Mar 9, 2017

ericholscher commented Mar 9, 2017

agjohnson commented Mar 9, 2017

ericholscher commented Mar 18, 2017 via email

This is just an initial concept to break apart our Docker files. #30

This is just an initial concept to break apart our Docker files. #30

Conversation

ericholscher commented Mar 8, 2017 • edited Loading

agjohnson left a comment

Choose a reason for hiding this comment

agjohnson Mar 8, 2017

Choose a reason for hiding this comment

agjohnson Mar 8, 2017

Choose a reason for hiding this comment

agjohnson Mar 8, 2017

Choose a reason for hiding this comment

agjohnson Mar 8, 2017

Choose a reason for hiding this comment

agjohnson Mar 8, 2017

Choose a reason for hiding this comment

ericholscher commented Mar 8, 2017

humitos left a comment

Choose a reason for hiding this comment

agjohnson commented Mar 8, 2017

ericholscher commented Mar 8, 2017

humitos commented Mar 8, 2017

agjohnson commented Mar 8, 2017 • edited Loading

agjohnson commented Mar 8, 2017

agjohnson commented Mar 8, 2017

ericholscher commented Mar 8, 2017

humitos Mar 8, 2017

Choose a reason for hiding this comment

agjohnson commented Mar 8, 2017

ericholscher commented Mar 9, 2017

ericholscher commented Mar 9, 2017

humitos commented Mar 9, 2017

agjohnson commented Mar 9, 2017

humitos commented Mar 9, 2017 • edited Loading

agjohnson commented Mar 9, 2017

ericholscher commented Mar 9, 2017

ericholscher commented Mar 9, 2017

agjohnson commented Mar 9, 2017

ericholscher commented Mar 9, 2017

agjohnson commented Mar 9, 2017

ericholscher commented Mar 18, 2017 via email

ericholscher commented Mar 8, 2017 •

edited

Loading

agjohnson commented Mar 8, 2017 •

edited

Loading

humitos commented Mar 9, 2017 •

edited

Loading