Docker feedback

[jarshwah]

This will be a meta issue for collecting feedback. Otherwise we can just create an issue per item and close this one off, up to you.

Regarding the extra dependencies, can you add them from the dockerfile, install them, then volume mount over the top? Don't copy the entire django folder over, just the requirements.

Try combining as many instructions as you can to minimise the number of layers. For example, you have 5 RUN instructions in a row, they could all be combined. https://github.com/orf/django-docker-box/blob/master/Dockerfile#L23

I have no other specific feedback yet because I've just glanced at some of the files, I'm waiting for the weekend before I take it for a spin.

[orf]

Thanks for this! The meta issue is fine 👍

Regarding the dependencies: this brings some trouble. I would like to be able to have Django live in a directory outside of this project. Unfortunately the Django .git directory is rather large, so if you build the project with the django root as the context it has to copy the entire 400+mb tree (including .git) to the docker daemon before building which is pretty slow. The solution is to add .git to a .dockerignore but we don't control the django source directory and so cannot do this.

A fix might be to use a symlink within this project, i.e ./django/ -> ~/projects/django/, and have django/.git in a .dockerignore here but I am unsure how or if this would work on Windows.

I've worked around this by persisting the pip cache across runs, so installing the dependencies is pretty fast. If this does get accepted into the django project we should add a .dockerignore to speed things up and remove the need for that volume.

About the Dockerfile, good points, but I'm more concerned right now about getting a green build across the board just to prove that this is viable: https://travis-ci.com/orf/django-docker-box/builds/87770210

Right now there is:

1. a persistent gis failure on Sqlite (Add a gc.collect() to make GDALBandTests.test_band_data more deterministic django/django#10501, but this isn't the correct solution)
2. two failures in master on mysql 8 (https://code.djangoproject.com/ticket/29847#ticket)
3. persistent failures regarding the database images taking too long to start, resulting in the test runner failing to connect, so we need to add a healthcheck of some kind

[jarshwah]

Regarding the requirements, what I was thinking about was something like:

ARG DJANGO_PATH
RUN mkdir /requirements
COPY $DJANGO_PATH/tests/requirements/*.txt /requirements/
COPY extra-requirements.txt /requirements/
RUN pip install -r /requirements/py3.txt -r /requirements/..etc.txt

Then you can still volume mount the django directory, and all of the requirements have been built into the image.

Which of the databases is taking too long to start? I see pg has a healthcheck but the mysql varieties do not.

Also missing a headless test runner using the chrome or firefox drivers. There's an xvfb example in the vagrant image but I'm not sure how well that works. It might be worth looking into using chrome headless directly - but I have no experience with it.

If this project can get total coverage then it might even be attractive to depend on travis.ci directly and move the django project away from Jenkins. I don't know how well that would be received, but there's at least some burden of managing those jenkins instances and the ansible scripts behind them so I'd be keen to raise that possibility.

(Sorry, I was away for most of the weekend and didn't get much chance to dive in like I wanted to)

[apollo13]

I am very much interested in moving this forward; so if there is anything where you need help please ping me on IRC.

[orf]

Thanks for your help @apollo13, much appreciated!

So, a few things about the implementation. If we go with @jarshwah's suggestion (which is more clean, for sure) we run into some issues. To do this we need to specify the Django path as a the build context. That's fine. The problem is we have a .env file specifying the default environment variables (like PYTHON_VERSION), however it seems that this file has to be relative to the build context. So it will look in $DJANGO_PATH/.env. This is quite annoying, as every single compose run needs at least two variables.

So, a quickstart would become: PYTHON_VERSION=3.6 POSTGRES_VERSION=10 docker-compose run postgres ..., which is pretty verbose. We'd also loose all default values, and to be extra annoying docker-compose fails with a pretty unobvious error in the case above (because postgres:${POSTGRES_VERSION} resolves to postgres: if POSTGRES_VERSION it's not defined, which is in invalid reference). We could work around this by defining defaults in the Dockerfile and the compose definition I guess, I will try this tonight.

Also, while it's conceptually more 'dockery' it will actually be slower on the CI run, as you cannot mount volumes during the build. So the dependencies would always need to be downloaded, whereas now they are cached between runs in a volume. I'm not too fussed about this as the CI is not the primary use case here (yet!), however even a 40 second increase in each run across a full matrix equals ~40 minutes or so added in total.

As a side note, if we are going to consider a Jenkins replacement, Travis CI gives large open source projects special accounts with more concurrent runners. The exact specifics are done on a case-by-case basis, but Django qualifies for this.

[orf]

Also, as a side note, in the future with this we can potentially test on MacOS (while not a deployment platform, it's a huge development platform) and Windows. A full run across each of these would be out of the question but a minimal test suite may not be.

[jarshwah]

I wasn't suggesting to use the Django directory as the build context by the way. I was suggesting we should copy the requirements files from the django directory, but leave the build context as-is. Does that change things in any way?

[orf]

Oh right! Unfortunately that is not possible without the build context being Django (unless we duplicate the requirements files here). Docker COPY commands are relative to the build context, so you can't do COPY /any/directory/any/file.

Making Django the build context isn't a bad idea, I've implemented it in #11 and it all seems to work OK.

[carltongibson]

Hi @orf. I've been using this now, it's great. Thank you!

I was going to mark the Trac ticket as Patch needs improvement since I don't think it's quite ready to progress, but here seems the place to discuss currently so... (we can move the discussion elsewhere if you prefer).

So, first off, a question: what's the benefit to moving the files into django/django rather than say, django/django-docker-box and setting DJANGO_PATH as we do here? The PR adds quite a lot of files, and I suspect quite frequent edits, so lots of noise — could we not just keep that separate?

Second, for me at least, having done nothing to optimise in any way, performance is quite down on running without docker: 16mins vs 4mins.

(That's with the images already built etc.)

(That's running with --parallel=2 to keep it fair, because docker is setup with the default "2 CPUs", but none of that is even mentioned on the Quickstart...)

Any thoughts on this?

I have various other little thoughts, that tie-in to the tweaks suggested here and in the other issues here and on the PR, but they really come under "frequent edits, so lots of noise" above.

Current status: we should definitely pull this into the Django org, as an official project etc, point people towards it and so on, but not include the files directly in django/django or make it the top option in the testing Quickstart.

[adamchainz]

+1 to moving into django org and pointing people towards it without including in django/django.

[orf]

Thanks for your feedback Carlton! I'll try and address a few points below.

Moving under Django: yes please! I can transfer the repo directly or we could create a new one, up to you. Let me know how to proceed.

Speed: it really should not be that slow, unless perhaps you are on Windows and using Virtual box as your docker server? I'd love to diagnose why this is, so any info you can give me would be great.

On Linux the speed should be native, on Mac it should be nearly native. With hyper-v it should still be fast.

Regarding including this inside Django: the main reason is the simplicity of running this while it's inside Django. Just clone and go (in theory 😁).

The secondary reason is it unlocks a lot of cool improvements down the line. Using docker-compose as the primary test runner for Django has some real tangible benefits - we can adapt the way we run tests via pull requests, the CI is always in sync with the developers, less reliance on Jenkins, easier to adapt other services (like the VS CI solution that's recently been proposed). Plus there is a lot less maintenance of Jenkins in general, and it's easier to expand our coverage.

In short, this repo running a single full-matrix build a day has caught a few bugs. It would be good to run it on every PR, or at least improve our coverage, and I see docker-compose as a gateway to making faster improvements to this in the future.

And the third reason is that it's a bit annoying to keep it separate. It's not really a separate project, it's intimately tied to Django and needs to be kept in sync (specifically versions, but other stuff as well). It feels natural to me to move this into core.

But, baby steps I guess. If not everyone agrees with this then I'm happy to keep it separate.

Edit: I would definitely not like to maintain the PR if I do not have to, so it would be good to get a consensus on this. I started the PR after someone suggested bringing it into core on the mailing list, so perhaps it warrants some more discussion there?

[jarshwah]

I’d like to see inclusion directly within django/django also. I see one of the major benefits as helping new contributors get up and running quickly. Pointing at a separate repo and asking users to mess with env vars really harms that onboarding process. I see the future infrastructure goals as secondary but important too. As far as performance goes, docker for mac in particular is very fsync happy. Every single write to a database within a container will be fsynced to the host volume. You can use volume mount options cached/delegated to control how often fsync will be used. Unsure if this is the problem for Carlton, but it’s a common one with docker compose and databases.

…

On Tue, 19 Feb 2019 at 06:53, Tom Forbes ***@***.***> wrote: Thanks for your feedback Carlton! I'll try and address a few points below. Moving under Django: yes please! I can transfer the repo directly or we could create a new one, up to you. Let me know how to proceed. Speed: it really should not be that slow, unless perhaps you are on Windows and using Virtual box as your docker server? I'd love to diagnose why this is, so any info you can give me would be great. On Linux the speed should be native, on Mac it should be nearly native. With hyper-v it should still be fast. Regarding including this inside Django: the main reason is the simplicity of running this while it's inside Django. Just clone and go (in theory 😁 ). The secondary reason is it unlocks a lot of cool improvements down the line. Using docker-compose as the primary test runner for Django has some real tangible benefits - we can adapt the way we run tests via pull requests, the CI is always in sync with the developers, less reliance on Jenkins, easier to adapt other services (like the VS CI solution that's recently been proposed). Plus there is a lot less maintenance of Jenkins in general, and it's easier to expand our coverage. In short, this repo running a single full-matrix build a day has caught a few bugs. It would be good to run it on every PR, or at least improve our coverage, and I see docker-compose as a gateway to making faster improvements to this in the future. And the third reason is that it's a bit annoying to keep it separate. It's not really a separate project, it's intimately tied to Django and needs to be kept in sync (specifically versions, but other stuff as well). It feels natural to me to move this into core. But, baby steps I guess. If not everyone agrees with this then I'm happy to keep it separate. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAhBm2VXS6IeXDrqqC41o36dzhXhEIB9ks5vOwScgaJpZM4XWkPT> .

[carltongibson]

I'm using Docker on macOS. Just default settings.

I think we need to go back to the mailing list...

[carltongibson]

One thought: If we had an "official" image users could just docker pull to get up and running. (???)