-
Notifications
You must be signed in to change notification settings - Fork 70
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
This is just an initial concept to break apart our Docker files. #30
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
base/Dockerfile
Outdated
# 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this using the base python3 again? The latest image was using pyenv installed python.
base/Dockerfile
Outdated
|
||
FROM ubuntu:16.04 | ||
MAINTAINER Read the Docs <support@readthedocs.com> | ||
LABEL version="latest" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be latest
base/Dockerfile
Outdated
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also removes recent commits to repair the conda path
full/Dockerfile
Outdated
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 \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
full/Dockerfile
Outdated
# Read the Docs - Environment base | ||
FROM readthedocs/base:latest | ||
MAINTAINER Read the Docs <support@readthedocs.com> | ||
LABEL version="2.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is being merged to master
, the version is not 2.0
there.
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? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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 |
The base Dockerfile will have more things in it though? So the root Dockerfile would inherent from base, but be the |
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.
@ericholscher yeah, I think the root Dockerfile should be the latest full (with everything) |
Yeah 👍 . That way, it's very obvious which Dockerfile is The One. Development could happen from |
We'd probably also want this pattern for |
10cd9b4
to
9a822fd
Compare
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. |
Hmm -- is there a way to build a |
Dockerfile
Outdated
# 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 \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will be a problem since we are already as a docs
user from the base image :)
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
|
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. |
Nevermind, I got it working now, with a few dependencies. |
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? |
@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 |
@agjohnson ok, so I will ask how did you choose the versions that you support?
|
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. |
AFAIK almost nobody uses anything before 3.5 in a real production environment, so we can probably drop them. |
Even Django only supports 3.4 and above: https://docs.djangoproject.com/en/dev/releases/1.10/ |
I think 3.4 at minimum then, it was the the 3.x on the last build image. |
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. |
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. |
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
|
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.