Add Docker image building, testing and pushing to Travis #920
Conversation
lucassz
changed the title
|
@lucassz this is awesome. A couple thoughts
This PR in addition to ospc-org/pb_deploy#1 is going to substantially increase PB quality and efficiency (and my happiness as principal deployer). Thanks again for the high quality work you've done so far this summer. What's left to complete this PR? |
lucassz
force-pushed
the
travis
branch
18 times, most recently
from
7668308
to
9302731
Compare
|
@hdoupe The latest version of the PR should now be functional, as shown on my build here. When it is merged, the relevant branch in The new approach corresponds to the latter part on your reflection on Docker, as Docker is now used for all steps of testing. I think this is more straightforward, as it avoids "works on my machine"-style issues where some edge case could pass on Travis' workers but not on the Docker containers that are actually pushed. |
|
Semi-related note: perhaps it would be helpful for the test site to show (through an environment variable) the commit it's running (e.g. in the footer)? This would make it easier to verify that this new feature is working as expected. |
| -e DROPQ_WORKERS=127.0.0.1:5050 | ||
| -e DATABASE_URL=postgresql://postgres@localhost/mypb | ||
| -it opensourcepolicycenter/web:$TAG | ||
| /bin/bash -c "pip install -q pytest-django && py.test webapp/apps" |
There was a problem hiding this comment.
Let's move pytest-django into the requirements.txt file. That way people can run these tests locally with ease.
There was a problem hiding this comment.
It's already in requirements_dev.txt, which gets installed by the python_env_build.sh script which is run by end-users. So I think it actually makes sense the current way, right? It probably shouldn't be in the image that gets deployed to e.g. Heroku.
| services: docker | ||
|
|
||
| # Also need OSPC_ANACONDA_TOKEN, AWS_ACCOUNT_ID, AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY to be defined in settings | ||
| env: TAG=${TRAVIS_COMMIT::6} |
There was a problem hiding this comment.
Doe this grab the first six characters of the git commit?
There was a problem hiding this comment.
Yes, exactly. This doesn't actually get copied anywhere beyond the Travis instance as far as I know, so it's not too important, but the 6-character git commit hash seemed good to use.
Ok, that seems sensible to me but with one condition: we should make sure that it's possible to at least run the tests without docker locally. The image build and run loop adds about 10 or 15 seconds to the feedback loop when you're making changes. Believe it or not that makes a big difference when you are used to a 2 to 5 second feedback loop when you're targeting a single test or group of tests. |
I agree. We used to show a commit hash, but this was removed in PR #622. I still don't think we should show a commit hash on the prod site, but I think it would be useful to do so on the test site. To do so, we would have to:
However, I'll knock this out in a follow up PR. |
I understand, though unless I'm misunderstanding what you're saying, wouldn't the present PR be neutral as to that ability, since it's not altering the possibility of running them locally? Or do you just mean that we need to keep that in mind as a supported feature more generally?
Right. Alternatively, it could still not be shown to the user on either one, but included in a HTML comment on both for devs to use. |
|
I just mean that we need to keep that ability available in general. That's something to consider. I'll keep that in mind (and you can bring it back up) when this is implemented in a follow up PR. |
A proof of concept for Docker image building and pushing within Travis. Currently working well in the CI system on my fork. Still some work to do; ideally, Travis would build images, push them and refresh the server instances for the test environment for any change on master, and do the same for the production environment for any new tag, which shouldn't be too hard to do. This also adds support for testing the Docker image.