Only use flask for debug #1607
Only use flask for debug #1607
Conversation
david4096
force-pushed
the
1548_gunicorn_optional
branch
2 times, most recently
from
ea9874e
to
5fac2e3
Compare
Codecov Report
@@ Coverage Diff @@
## master #1607 +/- ##
==========================================
- Coverage 86.65% 86.57% -0.08%
==========================================
Files 33 33
Lines 7471 7494 +23
Branches 926 928 +2
==========================================
+ Hits 6474 6488 +14
- Misses 805 813 +8
- Partials 192 193 +1
Continue to review full report at Codecov.
|
david4096
force-pushed
the
1548_gunicorn_optional
branch
from
5fac2e3
to
9164eef
Compare
|
To better demonstrate what this gives us, I ran the locust load testing suite with this locustfile against this PR in two modes. This test is a little better isolated than the previous which used curl. Locust uses gevent to spawn workers at a lower cost on the system. First with the
And then with the gunicorn server (running without
Note the failures with the builtin flask server, which doesn't handle simultaneous requests well. |
|
If we merge this we should follow it with a PR that improves test coverage under gunicorn mode in test_gestalt. Tests that involve ensuring an announce request is sent and received will be easier to write, I believe. |
|
I don't want to rush this into the release, but I would love to get feedback! |
| parser = common_cli.createArgumentParser("GA4GH reference server") | ||
| addServerOptions(parser) | ||
| return parser | ||
|
|
||
|
|
||
| def number_of_workers(): | ||
| return (multiprocessing.cpu_count() * 2) + 1 |
There was a problem hiding this comment.
should probably have a comment here justifying this formula
There was a problem hiding this comment.
Lifted from here http://docs.gunicorn.org/en/latest/custom.html?highlight=multiprocessing I'll add a link
| help="Don't use the flask reloader") | ||
| help="Don't use the Flask reloader (for Flask debug)") | ||
| parser.add_argument( | ||
| "--debug", "-d", action='store_true', default=False, |
There was a problem hiding this comment.
the docs say the -g flag is to run gunicorn
There was a problem hiding this comment.
Thanks, I started by making running under gunicorn a flag, but I believe the default functionality when running ga4gh_server should be to run in a multiworker environment. The -d will run using the Flask debug server (werkzeug).
| @@ -194,6 +194,7 @@ def getCmdLine(self): | |||
| python server_dev.py | |||
| --dont-use-reloader | |||
| --disable-urllib-warnings | |||
| -d | |||
There was a problem hiding this comment.
best to use the non-shorthand version of the flag here, for clarity in reading this file
| host=parsedArgs.host, port=parsedArgs.port, | ||
| use_reloader=not parsedArgs.dont_use_reloader, | ||
| ssl_context=sslContext) | ||
| if parsedArgs.debug: |
There was a problem hiding this comment.
I don't get why this is called debug ... seems like this should be run_guincorn or something...
This PR provides an experimental "run under gunicorn" option toga4gh_server.This PR runs the server behind the gunicorn WSGI server when running via
ga4gh_server. This makes the software much easier to use and more practical for people who don't know what "Apache" or "nginx" are.This spawns multiple worker processes that can each handle their own requests. This standalone WSGI HTTP Server is well tested with Flask, however, our implementation includes some challenges. The first is that TLS mode won't work directly, and logging needs to be handled differently.
I'd like to get this experimental feature out there for folks to test, I think that by default when running
ga4gh_serverwe would like to run behindgunicornas opposed to the SimpleHTTPServer. It would allow us to simplify the Apache instructions, and the dockerfile. For apache, one simply sets up a proxy and doesn't need to use mod_wsgi.Close #1548
TODO
Verify if auth works in this configurationSet up logging to stdout (where are worker logs redirected to?)Make gunicorn default production run setting