Add support for Docker #350

afirth · 2015-04-09T19:07:18Z

./Dockerfile, which builds the server with mod_wsgi-express. Data and config.py are not placed.
demo/Dockerfile, which adds the example data and config.py to 1)
demo_light/Dockerfile, which builds only the python, as the demo in
existing docs

afirth · 2015-04-09T19:12:07Z

Again referencing #305, and in the spirit of #344, this adds support for deployment through mod_wsgi-express, leveraging upstream code as much as possible. The mod_wsgi author publishes mod_wsgi-docker at https://github.com/GrahamDumpleton/mod_wsgi-docker, which this is built on top of. I would welcome further input or pulls. Images are available on dockerhub:

afirth/ga4gh_server_apache:core
afirth/ga4gh_server_apache:demo
afirth/ga4gh_centos_demo

I will add docs shortly. @hershman, what do you think? I didn't see your PR until I had these mostly done last night.

jeromekelleher · 2015-04-10T15:27:58Z

This looks really good @afirth, thanks for putting this together. As you've noticed, it's unfortunate that @hershman has already opened a PR with a working Docker recipe. I think the fairest thing to do is to merge @hershman's changes first, and perhaps you could extend on this afterwards? I like the idea of using the upstream images, this is a good idea.

afirth · 2015-04-10T17:16:08Z

I'd love to hear from @hershman, I'd love to collaborate rather than squashing each other's work. I'm sure there are some design considerations he's thought of that I haven't. I'll drop a comment on #344.

afirth · 2015-04-13T18:12:58Z

Added documentation. Now supports using environment variables to set data_source and debug. Automated builds available at https://registry.hub.docker.com/u/afirth/ga4gh_server_apache/
Updated documentation. Demo and production behind apache can now be run with one command. Support use of host-side dataset. Pull in @hershman's Dockerfile as well

pgrosu · 2015-04-13T20:35:26Z

Neat automation and integration with documentation - thanks for the nice work.

~p

dcolligan · 2015-04-13T22:09:13Z

docs/source/installation.rst

+
+  $ VBoxManage controlvm boot2docker-vm natpf1 "ga4gh,tcp,127.0.0.1,8000,,8000"
+
+For more info on port forwarding see https://www.virtualbox.org/manual/ch06.html#natforward and https://github.com/CenturyLinkLabs/panamax-ui/wiki/How-To%3A-Port-Forwarding-on-VirtualBox


these links should use the sphinx link syntax

afirth · 2015-04-13T23:00:28Z

@dcolligan I'm not sure what you mean. I didn't think link text added anything here so I left them plain. No markup is required for links without alt-text. Is there a guideline that all links should be hidden? Res: fixed

dcolligan · 2015-04-14T01:36:47Z

@afirth It breaks up the flow of reading to see long gnarly URLs in the midst of a text block. "... see here and here." would be easier for a reader to parse, for instance.

afirth · 2015-04-14T03:27:33Z

@dcolligan updated, thanks for the review

dcolligan · 2015-04-14T17:18:57Z

To give a proper review, I think we should have someone that actually knows something about docker review this. But if everyone else is just as inexperienced with it as I am, I suppose I can look at it a bit more closely after we resolve which parts of which of the 2 PRs we want to merge.

afirth · 2015-04-14T17:27:16Z

As I said, I've merged #344 into here as there's been no movement on that build, and it was/is failing. I'd like to get this in so I can switch the autobuilds over to upstream instead of my branch

afirth · 2015-04-14T17:31:19Z

Oops, looks like it was rebased a few minutes ago. I can rebase on top of it and fix the doc merge conflicts if that's the way others want to go.

hershman · 2015-04-14T17:38:31Z

I @afirth - this is really great. I'm not expert enough on Docker to comment on all your changes. Given that #344 has been reviewed and that the discussion here may continue, I'd prefer to get that pulled in and have this squash it when ready.

jeromekelleher · 2015-04-15T07:48:55Z

OK, we've merged @hershman's PR, and you should be able to build on that now @afirth. Can you rebase this PR off upstream/develop please? We can review the changes (to the degree that we're able to --- I agree with @dcolligan's point above, I'm also clueless in terms of Docker) then.

afirth · 2015-04-15T17:40:33Z

rebased and squashed

jeromekelleher · 2015-04-16T12:49:15Z

.whiskey/action_hooks/pre-build

@@ -0,0 +1,7 @@
+#!/usr/bin/env bash


What does this file do?

It is a pre-build hook which adds the source of this repository to requirements.txt. Basically as doing pip install -e ./ Previous build instructions pull from github rather than building from source. If the user chooses the automated builds, that will still happen, but this way it is possible to build from source locally for development. I will expand the comments here to indicate the file's purpose. Res: fixed

jeromekelleher · 2015-04-16T12:53:11Z

deploy/config.py

+#TODO this logic could move to frontend.configure() or BaseConfig
+# For docker, if/when a default is set in serverconfig.py, run -v /localdata:/default-path
+# If the env variable GA4GH_DATA_SOURCE is set, use that path. Otherwise, use the default path
+DATA_SOURCE = os.getenv('GA4GH_DATA_SOURCE', "/ga4gh-example-data")


I think we support configuration from environment variables already --- I don't quite get what we're trying to do here.

The only environment variable imported is GA4GH_CONFIGURATION, the path of the config file. I agree we should support this in frontend, but I thought that would be out of scope of adding support for Docker. This allows setting the location of the datasource by passing an env variable to the docker container. os.getenv returns arg2 if arg1 is unset. This logic could be discarded when frontend.py is updated with production oriented argument parsing. Res: won't fix

jeromekelleher · 2015-04-16T13:19:18Z

I've had a look through this @afirth, and it's generally great. I don't quite understand how it all fits together though (which is my fault as a Docker noob), so it would be really helpful if you could write a quick description of how it all works, perhaps in a README in the deploy directory.

The documentation looks good, but I wonder if we're mixing the message of the page a little bit with all the details of running demos, fixing DNS problems and so on. This page is intended to be the definitive source of information for production deployments, but there's an awful lot of stuff here for getting things working on your laptop. Perhaps this sort of information would also be better in the Docker README?

Anyway, this all excellent, so thanks a lot!

afirth · 2015-04-20T16:10:53Z

@jeromekelleher Thanks a lot for your comments. I've fixed those that I can and will squash the commits after review.

Regarding the README, I worry that this is akin to explaining how apache and mod_wsgi work in this repo. It would duplicate the documentation at https://github.com/GrahamDumpleton/mod_wsgi-docker which is the base of this development (from the mod_wsgi author), and on docs.docker.com. I've added an explicit link to the upstream project to the advanced section of the docs.

jeromekelleher · 2015-04-21T15:55:18Z

Thanks very much for the responses @afirth, and for your patience with stupid questions! I like this a lot, I think we should merge and see how it goes as we get more experience. We can streamline things a bit later if needs be.

+1 after squashing.

afirth · 2015-04-21T18:45:35Z

squashed

jeromekelleher · 2015-04-23T14:29:27Z

ga4gh/application.wsgi

@@ -0,0 +1,3 @@
+from ga4gh.frontend import app as application


Sorry, I just spotted this now as I was about to merge it. I don't think this file should be in the ga4gh directory, as it's not part of the Python package. Could you move this to deploy (or somewhere else) please?

Absolutely. I'll leave that question for another day :)
Moved the file and copied it in place with the pre-build hook. Diff here, squashed into this PR

@hershman

1) ./Dockerfile, which builds the server with mod_wsgi-express 2) deploy/variants/demo/, which adds the example data to 1) 3) deploy/variants/centos-demo-noapache/, which builds only the python, as the demo in existing docs 4) deploy/variants/ubunto-demo-apache/, by @hershman, which builds apache demo on ubuntu Add maintainer line to @hershman's Dockerfile Support GA4GH_DEBUG and GA4GH_DATA_SOURCE to set values in config.py Update installation.rst for Docker Add link text for URLs Update comments in pre-build hook Fix broken docker installation link Expand documentation for docker run Add link to ports section and fix typo Add link to upstream mod_wsgi-docker project Move application.wsgi to deploy dir and place with pre-build hook

afirth · 2015-04-23T18:02:21Z

@jeromekelleher No problem. Squashed on top, dockerhub builds passed.

jeromekelleher · 2015-04-27T15:55:39Z

Merging. Thanks @afirth!

Add support for Docker

afirth force-pushed the 2015-04-09_docker_305 branch 2 times, most recently from f468100 to 10b49c3 Compare April 9, 2015 21:31

afirth mentioned this pull request Apr 10, 2015

added Dockerfile #344

Merged

afirth force-pushed the 2015-04-09_docker_305 branch 6 times, most recently from b04b15d to 5003d38 Compare April 13, 2015 18:54

afirth mentioned this pull request Apr 13, 2015

Dockerize ga4gh server + example data #305

Closed

dcolligan reviewed Apr 13, 2015
View reviewed changes

afirth force-pushed the 2015-04-09_docker_305 branch 3 times, most recently from cd1f908 to 06509ed Compare April 15, 2015 17:30

jeromekelleher reviewed Apr 16, 2015
View reviewed changes

afirth force-pushed the 2015-04-09_docker_305 branch 5 times, most recently from fcfa615 to 375e8de Compare April 20, 2015 15:39

afirth force-pushed the 2015-04-09_docker_305 branch from 4f9a169 to 757756e Compare April 20, 2015 17:45

afirth force-pushed the 2015-04-09_docker_305 branch from 757756e to 9857264 Compare April 21, 2015 16:46

jeromekelleher added the mergable label Apr 22, 2015

jeromekelleher reviewed Apr 23, 2015
View reviewed changes

jeromekelleher removed the mergable label Apr 23, 2015

afirth force-pushed the 2015-04-09_docker_305 branch from 9857264 to 1e627b5 Compare April 23, 2015 17:27

afirth force-pushed the 2015-04-09_docker_305 branch from 1e627b5 to b6ded9d Compare April 23, 2015 17:52

jeromekelleher added a commit that referenced this pull request Apr 27, 2015

Merge pull request #350 from UMMS-Biocore/2015-04-09_docker_305

7e946e7

Add support for Docker

jeromekelleher merged commit 7e946e7 into ga4gh:develop Apr 27, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Docker #350

Add support for Docker #350

afirth commented Apr 9, 2015

afirth commented Apr 9, 2015

jeromekelleher commented Apr 10, 2015

afirth commented Apr 10, 2015

afirth commented Apr 13, 2015

pgrosu commented Apr 13, 2015

dcolligan Apr 13, 2015

afirth commented Apr 13, 2015

dcolligan commented Apr 14, 2015

afirth commented Apr 14, 2015

dcolligan commented Apr 14, 2015

afirth commented Apr 14, 2015

afirth commented Apr 14, 2015

hershman commented Apr 14, 2015

jeromekelleher commented Apr 15, 2015

afirth commented Apr 15, 2015

jeromekelleher Apr 16, 2015

afirth Apr 20, 2015

jeromekelleher Apr 16, 2015

afirth Apr 20, 2015

jeromekelleher commented Apr 16, 2015

afirth commented Apr 20, 2015

jeromekelleher commented Apr 21, 2015

afirth commented Apr 21, 2015

jeromekelleher Apr 23, 2015

afirth Apr 23, 2015

afirth commented Apr 23, 2015

jeromekelleher commented Apr 27, 2015


		$ VBoxManage controlvm boot2docker-vm natpf1 "ga4gh,tcp,127.0.0.1,8000,,8000"

		For more info on port forwarding see https://www.virtualbox.org/manual/ch06.html#natforward and https://github.com/CenturyLinkLabs/panamax-ui/wiki/How-To%3A-Port-Forwarding-on-VirtualBox

		@@ -0,0 +1,3 @@
		from ga4gh.frontend import app as application

Add support for Docker #350

Add support for Docker #350

Conversation

afirth commented Apr 9, 2015

afirth commented Apr 9, 2015

jeromekelleher commented Apr 10, 2015

afirth commented Apr 10, 2015

afirth commented Apr 13, 2015

pgrosu commented Apr 13, 2015

dcolligan Apr 13, 2015

Choose a reason for hiding this comment

afirth commented Apr 13, 2015

dcolligan commented Apr 14, 2015

afirth commented Apr 14, 2015

dcolligan commented Apr 14, 2015

afirth commented Apr 14, 2015

afirth commented Apr 14, 2015

hershman commented Apr 14, 2015

jeromekelleher commented Apr 15, 2015

afirth commented Apr 15, 2015

jeromekelleher Apr 16, 2015

Choose a reason for hiding this comment

afirth Apr 20, 2015

Choose a reason for hiding this comment

jeromekelleher Apr 16, 2015

Choose a reason for hiding this comment

afirth Apr 20, 2015

Choose a reason for hiding this comment

jeromekelleher commented Apr 16, 2015

afirth commented Apr 20, 2015

jeromekelleher commented Apr 21, 2015

afirth commented Apr 21, 2015

jeromekelleher Apr 23, 2015

Choose a reason for hiding this comment

afirth Apr 23, 2015

Choose a reason for hiding this comment

afirth commented Apr 23, 2015

jeromekelleher commented Apr 27, 2015