Skip to content
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

Add gunicorn to web server #9

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add gunicorn to web server #9

wants to merge 2 commits into from

Conversation

joaojunior
Copy link

Motivation

Closes #8
I only copy the idea of this pr cognoma/core-service#48 here.

API changes

I don't make any change in API.

Implementation Notes

Add gunicorn to web server in production.

Functional Tests

Verify if is possible access API after start a container from docker run.

@dcgoss
Copy link
Member

dcgoss commented Apr 21, 2017

Quick question: why port 8001?

@joaojunior
Copy link
Author

Hi @dcgoss!
If you verify the file docker-compose.yml, you will see that I don't change the port. The port already is 8001. I only change in docs, because docs say port that port is 8080, but this port isn't use.

@dhimmel
Copy link
Member

dhimmel commented Apr 23, 2017

@dcgoss do you want to take the lead reviewing / approving / merging this PR?

@@ -0,0 +1,4 @@
#!/bin/bash -x

manage.py migrate -v3 --no-input
Copy link
Member

@dcgoss dcgoss Apr 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does -v3 do? Also, this command must be prefaced with python, like line 4

command=/usr/local/bin/gunicorn task_service.wsgi:application -c /code/config/prod/gunicorn.conf
directory=/code
; User account with minimal privileges
user=nobody
Copy link
Member

@dcgoss dcgoss Apr 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the nobody user need to be created in the Dockerfile (something like: RUN useradd nobody), or is that not needed?


# Application is configured to run under Supervisord, which in turn monitors
# the Gunicorn web server
supervisord -c /code/config/prod/supervisord.conf
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an idea, we could add a -n here to make supervisord run in the foreground since it is a container. Might make it easier to debug startup problems.

worker_class = 'eventlet'

# Rule of thumb for workers is (2 * ncpus) + 1
# http://docs.gunicorn.org/en/stable/design.html#how-many-workers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gunicorn also supports supplying a Python file for configuration, here's an example from one of my projects. One benefit of this is you can dynamically compute the worker count when Gunicorn starts regardless of server, by using Python's multiprocessing.cpu_count

@dhimmel
Copy link
Member

dhimmel commented Apr 26, 2017

@kurtwheeler will this PR break anything in cognoma/infrastructure?

@kurtwheeler
Copy link
Member

I'm not familiar with gunicorn so I'm not too sure. As long as this doesn't change the way the app is started or what ports need to be available it should be fine though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants