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

Improve development environment configuration, support SELinux #257

Merged
merged 2 commits into from Oct 11, 2017

Conversation

Projects
None yet
3 participants
@jwflory
Contributor

jwflory commented Oct 2, 2017

This commit makes three changes:

  • Supports enforcing SELinux on the host computer by adding the :z annotation to the volume mount
  • Actually starts the local server on port 8000 on the host system (before, was running on port 80, contradicting the README)
  • Create docker/data instead of docker/docker/data, fixes accidental recursion and plays nicely with the .gitignore

I also added some more paths to ignore in the .gitignore, for Python virtualenvs. Ideally, most people won't have to use them, but I had to when I was playing around to get things working, so I went ahead
and added them.

This should help make it easier for first-time contributors to get started. With the current changes, I'm able to successfully create a development environment on first clone, where I had not been able
to before. Whooo!

Edit: I wrote a draft blog post on how to set up a development environment – a temporary preview link is here!

Improve development environment configuration, support SELinux
This commit makes three changes:

* Supports enforcing SELinux on the host computer by adding the `:z`
  annotation to the volume mount
* Actually starts the local server on port 8000 on the host system
  (before, was running on port 80, contradicting the README)
* Create `docker/data` instead of `docker/docker/data`, fixes
  accidental recursion and plays nicely with the .gitignore

I also added some more paths to ignore in the .gitignore, for Python
virtualenvs. Ideally, most people won't have to use them, but I had
to when I was playing around to get things working, so I went ahead
and added them.

This should help make it easier for first-time contributors to get
started. With the current changes, I'm able to successfully create
a development environment on first clone, where I had not been able
to before. Whooo!

jwflory added a commit to jwflory/listenbrainz-server that referenced this pull request Oct 2, 2017

Add documentation page explaining how to set up a development environ…
…ment

This commit adds a dedicated documentation page on how to set up a
development environment for ListenBrainz. I verified it looked the
way I expected by building it locally with `make clean html`. This
commit also expands on top of PR metabrainz#257 and assumes that the changes
there are merged in.

jwflory added a commit to jwflory/listenbrainz-server that referenced this pull request Oct 2, 2017

Add documentation page explaining how to set up a development environ…
…ment

This commit adds a dedicated documentation page on how to set up a
development environment for ListenBrainz. I verified it looked the
way I expected by building it locally with `make clean html`. This
commit also expands on top of PR metabrainz#257 and assumes that the changes
there are merged in.
@paramsingh

Looks really good to me, thanks for doing this!

I only have just the one change I'd like before merging this.

environment:
GOOGLE_APPLICATION_CREDENTIALS: '/code/credentials/bigquery-credentials.json'
ports:
- "80:80"
- "8000:80"

This comment has been minimized.

@paramsingh

paramsingh Oct 5, 2017

Member

I think we should keep this at 80 and change the docs to 80 from 8000. Other MeB projects use 80 too and it is a good idea to keep as many similarities between projects as possible.

@paramsingh

paramsingh Oct 5, 2017

Member

I think we should keep this at 80 and change the docs to 80 from 8000. Other MeB projects use 80 too and it is a good idea to keep as many similarities between projects as possible.

This comment has been minimized.

@jwflory

jwflory Oct 5, 2017

Contributor

@paramsingh What if you're running multiple projects in your environment? For the host machine, I thought port 8000 made more sense since it's an unprivileged port, and you could use a web server (e.g. nginx) to do a rewrite rule for port 8000. In this way, port 8000 made more sense for both a development environment and running in production.

If you'd like, I can still change everything to port 80. But from my point of view, port 8000 makes more sense since port 80 is likely to conflict with other services / applications.

@jwflory

jwflory Oct 5, 2017

Contributor

@paramsingh What if you're running multiple projects in your environment? For the host machine, I thought port 8000 made more sense since it's an unprivileged port, and you could use a web server (e.g. nginx) to do a rewrite rule for port 8000. In this way, port 8000 made more sense for both a development environment and running in production.

If you'd like, I can still change everything to port 80. But from my point of view, port 8000 makes more sense since port 80 is likely to conflict with other services / applications.

This comment has been minimized.

@paramsingh

paramsingh Oct 10, 2017

Member

You're right, 8000 does make more sense, I guess. I'm just hesitant because of other MeB projects using 80 (and a little, because my setup uses 80 :P). I'd be okay with 80 or 8000, but I guess we should make the port we choose here the standard for other MeB projects too.

@paramsingh

paramsingh Oct 10, 2017

Member

You're right, 8000 does make more sense, I guess. I'm just hesitant because of other MeB projects using 80 (and a little, because my setup uses 80 :P). I'd be okay with 80 or 8000, but I guess we should make the port we choose here the standard for other MeB projects too.

This comment has been minimized.

@paramsingh

paramsingh Oct 10, 2017

Member

@mayhem, any opinions on this? :P

@paramsingh

paramsingh Oct 10, 2017

Member

@mayhem, any opinions on this? :P

This comment has been minimized.

@mayhem

mayhem Oct 11, 2017

Member

I'd prefer consistency with other projects for now -- I agree with the reservations, but until we finally get some "best metabrainz docker practices" defined, lets go with 80.

Regardless of what we pick, people will complain about it. :)

@mayhem

mayhem Oct 11, 2017

Member

I'd prefer consistency with other projects for now -- I agree with the reservations, but until we finally get some "best metabrainz docker practices" defined, lets go with 80.

Regardless of what we pick, people will complain about it. :)

@paramsingh paramsingh merged commit 6ce76b7 into metabrainz:master Oct 11, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jwflory jwflory deleted the jwflory:change/fix-develop branch Oct 11, 2017

jwflory added a commit to jwflory/listenbrainz-server that referenced this pull request Nov 11, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment