search

softdevteam / mattermost-github-integration

GitHub integration for Mattermost
Other
79 stars 50 forks source link

Please don't use app.run() in the Dockerfile #54

Closed ThiefMaster closed 4 years ago

ThiefMaster commented 5 years ago

People might use it for production, and app.run() (same for flask run btw) is not suitable for production; better use something like gunicorn or uwsgi (both can directly serve http requests).

ptersilie commented 5 years ago

I'm not very familiar with Docker (all the Docker stuff was added via a PR). Is this just a matter of changing the ReadMe? What do you suggest needs changing?

ThiefMaster commented 5 years ago

Actually it's not only about docker but also the readme suggesting python server.py - which is fine for local testing, but not for actual production usage on a public server.

Check http://flask.pocoo.org/docs/1.0/deploying/wsgi-standalone/ on some examples how to run with gunicorn or uwsgi.

I think something like gunicorn -b 0.0.0.0:5000 mattermostgithub:app would be enough and work both inside and outside Docker (pip install gunicorn is needed of course).

ptersilie commented 5 years ago

I see what you mean now. We can add a small disclaimer to the ReadMe, suggesting that people should use uwsgi or gunicorn in deployment. I'd be happy to accept a PR for this.

ptersilie commented 4 years ago

Fixed in #56