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.

Sign up for GitHub

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

Jump to bottom

Reorganise h's config files #8386

Merged
merged 1 commit into from
Jan 2, 2024
Merged

Reorganise h's config files #8386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ on:
- '.github/workflows/deploy.yml'
- '.github/workflows/redeploy.yml'
- 'bin/logger'
- 'conf/development-app.ini'
- 'app.ini'
- 'conf/development.ini'
- 'production.ini'
- 'conf/supervisord*.conf'
- 'docs/*'
- 'requirements/*.in'
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/data_tasks.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,5 @@ jobs:
Env: ${{ inputs.Environment }}
Timeout: 7200
Region: ${{ inputs.Region }}
Command: 'newrelic-admin run-program python bin/run_data_task.py --config-file conf/app.ini --task ${{ inputs.Task }}'
Command: 'newrelic-admin run-program python bin/run_data_task.py --config-file conf/production.ini --task ${{ inputs.Task }}'
secrets: inherit
2 changes: 1 addition & 1 deletion .github/workflows/deploy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ on:
- '.github/*'
- 'bin/create-testdb'
- 'bin/install-python'
- 'conf/development-app.ini'
- 'conf/development.ini'
- 'conf/supervisord-dev.conf'
- 'conf/websocket-dev.ini'
- 'docs/*'
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/report_refresh.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ jobs:
Env: 'prod'
Timeout: 3600
Region: 'all'
Command: 'newrelic-admin run-program python bin/run_data_task.py --config-file conf/app.ini --task report/refresh'
Command: 'newrelic-admin run-program python bin/run_data_task.py --config-file conf/production.ini --task report/refresh'
secrets: inherit
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ devdata: python

.PHONY: shell
shell: python
@pyenv exec tox -qe dev --run-command 'pshell conf/development-app.ini'
@pyenv exec tox -qe dev --run-command 'pshell conf/development.ini'

.PHONY: sql
sql: python
Expand Down
2 changes: 1 addition & 1 deletion bin/make_db
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def init_search(settings):


def main():
with bootstrap("conf/development-app.ini") as env:
with bootstrap("conf/development.ini") as env:
settings = env["registry"].settings
init_db(settings)
init_search(settings)
Expand Down
9 changes: 0 additions & 9 deletions conf/development-app.ini → conf/development.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,6 @@ secret_key: notverysecretafterall

sqlalchemy.url: postgresql://postgres@localhost/postgres

[server:main]
use: egg:gunicorn#main
host: 0.0.0.0
port: 5000
proc_name: web
graceful_timeout: 0
timeout: 0
errorlog: -
Comment on lines -25 to -32
Copy link
Contributor Author

@seanh seanh Dec 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Renamed development-app.ini to just development.ini, the same as our other apps.

This [server:main] section contained Gunicorn config settings. Gunicorn only reads settings from the [server:main] block of the PasteDeploy config file if Gunicorn is being run indirectly via pserve. We're no longer using pserve, so these Gunicorn settings have now moved into gunicorn-dev.conf.py.

The proc_name setting was unnecessary: the app is already called web in supervisord-dev.conf, so I didn't copy that into gunicorn-dev.conf.py.

The errorlog setting was also unnecessary: - is the default. Didn't copy that either.


[pshell]
setup = h.pshell.setup

Expand Down
3 changes: 3 additions & 0 deletions conf/gunicorn-dev.conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
bind = "0.0.0.0:5000"
graceful_timeout = 0
timeout = 0
Comment on lines +1 to +3
Copy link
Contributor Author

@seanh seanh Dec 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think it's better to have separate gunicorn*.conf.py config files for Gunicorn:

  • It's a more obvious place to put the Gunicorn config settings, rather than putting them in a [server:main] block in the PasteDeploy config file (you wouldn't think to look for Gunicorn settings there, and it's not obvious that they are Gunicorn settings even once you've found them).
  • There are some Gunicorn settings (such as server hooks) that can only go in a Gunicorn config file, not a PasteDeploy one.
  • Gunicorn only reads general Gunicorn settings from a PasteDeploy file when run it via pserve, and we're not using pserve anymore

5 changes: 5 additions & 0 deletions conf/gunicorn-websocket-dev.conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
bind = "localhost:5001"
worker_class = "h.streamer.Worker"
graceful_timeout = 0
workers = 2
worker_connections = 8
5 changes: 5 additions & 0 deletions conf/gunicorn-websocket-monolithic.conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
bind = "unix:/tmp/gunicorn-websocket.sock"
worker_class = "h.streamer.Worker"
graceful_timeout = 0
workers = 2
worker_connections = 8192
5 changes: 5 additions & 0 deletions conf/gunicorn-websocket.conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
bind = "0.0.0.0:5000"
worker_class = "h.streamer.Worker"
graceful_timeout = 0
workers = 2
worker_connections = 8192
1 change: 1 addition & 0 deletions conf/gunicorn.conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
bind = "unix:/tmp/gunicorn-web.sock"
5 changes: 0 additions & 5 deletions conf/app.ini → conf/production.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ use: call:h.app:create_app
[filter:proxy-prefix]
use: egg:PasteDeploy#prefix

[server:main]
use: egg:gunicorn#main
bind: unix:/tmp/gunicorn-web.sock
proc_name: web
Comment on lines -12 to -15
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed app.ini to production.ini as in our other apps, and again moved the [server:main] section into gunicorn.conf.py. Again the proc_name setting was unnecessary here.


[loggers]
keys = root, alembic, gunicorn.error, h

Expand Down
4 changes: 2 additions & 2 deletions conf/supervisord-dev.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ nodaemon = true
silent = true

[program:web]
command = pserve --reload conf/development-app.ini
command = gunicorn --paste conf/development.ini --config conf/gunicorn-dev.conf.py
Copy link
Contributor Author

@seanh seanh Dec 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

h was the only one of our apps that was running Gunicorn indirectly via pserve. I've changed it to run gunicorn directly like our other apps do and to use gunicorn's --paste option so that the Pyramid settings get read (again like we do in other apps).

This is unusual for a Pyramid app: the Pyramid docs tell you to use pserve. But we've always run gunicorn directly (in all apps but h) and I'm not aware of anything that we're losing by not using pserve. May as well remove an unnecessary layer?

I've gone for a gunicorn --paste conf/*.ini --config conf/gunicorn*.conf.py command format that gives us both a PasteDeploy config file and a Gunicorn config file. Even though we (rightly) tend to use envvars most often for config settings, I do think we want both config files. Examples in the Pyramid docs and many Pyramid extensions tell you to use the PasteDeploy config file, and it's where we put our logging config. And the Gunicorn config file is the only Gunicorn config source that supports all Gunicorn config settings. There are separate config files for dev because it tends to differ a lot, but QA/staging/production share the same config file (so any differences need to go in envvars).

The cookiecutter will put these gunicorn --paste conf/*.ini --config conf/gunicorn*.conf.py commands in the supervisord[-dev].conf files and then apps will be able to customise the configs by overriding the conf/* files and/or by using environment variables (including the GUNICORN_CMD_ARGS envvar).

Notes on Gunicorn configuration

To understand Gunicorn configuration you need to read the Running Gunicorn, Configuration Overview and Settings pages in the Gunicorn docs and combine them in your head. Here's a short summary:

Gunicorn reads configuration from five locations and merges them. You can use all five of these at once, each successive location overriding earlier ones:

  1. Certain specific environment variables for specific settings. For example PORT (for the port part of the bind setting), WEB_CONCURRENCY (for the workers setting), and a few others. We use some of these (e.g. WEB_CONCURRENCY) in our QA and production environments. Not all settings have environment variables, only a small handful do. The environment variables aren't all listed in one place in the Gunicorn docs, mentions of them are spread throughout the Settings page.

  2. The PasteDeploy config file (if any), e.g. development.ini or production.ini. There are two variations of this:

    1. You can run a command like pserve *.ini which uses Pyramid's pserve to run Gunicorn indirectly. The [server:main] block in the *.ini PasteDeploy config file tells Pyramid to use Gunicorn (instead of Waitress, which is Pyramid's default):

      [server:main]
use = egg:gunicorn#main
bind = unix:/tmp/gunicorn-web.sock
proc_name = web
workers = 3

      In this case Gunicorn can read some Gunicorn config settings from the [server:main] block in the PasteDeploy config file (such as proc_name and workers = 3 in the example above). Some Gunicorn settings (e.g. server hooks) can't go in a PasteDeploy config file.

      Some Gunicorn features such as hot-reloading don't work when running Gunicorn via pserve (you have to use pserve's reloading instead).

      See Running the Project Application in Pyramid's docs and Paste Deployment in Gunicorn's docs and also Paster Applications in the Gunicorn docs.

    2. You can run Gunicorn with the --paste or --paster option:

      gunicorn --paste development.ini [GUNICORN_OPTIONS]

      In this case you're running Gunicorn directly and not using pserve. Gunicorn will read certain settings from the PasteDeploy config file such as the path to the WSGI application function and the logging config. But when run in this way Gunicorn will not read general Gunicorn settings from the PasteDeploy config file's [server:main] section, so you have to put your Gunicorn settings somewhere else. Gunicorn hot reloading will work.

  3. The Gunicorn config file (if any). By default this is gunicorn.conf.py in the current working directory but it can be changed with the -c / --config argument to the gunicorn command. A Gunicorn config file is the only source that gives you access to all Gunicorn settings.

  4. The GUNICORN_CMD_ARGS environment variable which can contain gunicorn command line arguments, e.g. GUNICORN_CMD_ARGS="--bind=127.0.0.1 --workers=3".

  5. Command line arguments passed directly to the gunicorn command itself. These override everything else. Not all Gunicorn settings have command line arguments.

stdout_events_enabled=true
stderr_events_enabled=true
stopsignal = KILL
stopasgroup = true
autostart = %(ENV_ENABLE_WEB)s

[program:websocket]
command = pserve --reload conf/websocket-dev.ini
command = gunicorn --paste conf/websocket-dev.ini --config conf/gunicorn-websocket-dev.conf.py
stdout_events_enabled=true
stderr_events_enabled=true
stopsignal = KILL
Expand Down
13 changes: 11 additions & 2 deletions conf/supervisord.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,30 @@ stderr_events_enabled=true
autostart = %(ENV_ENABLE_NGINX)s

[program:web]
command=newrelic-admin run-program pserve conf/app.ini
command=newrelic-admin run-program gunicorn --paste conf/production.ini --config conf/gunicorn.conf.py
stdout_logfile=NONE
stderr_logfile=NONE
stdout_events_enabled=true
stderr_events_enabled=true
autostart = %(ENV_ENABLE_WEB)s

[program:websocket]
command=pserve %(ENV_WEBSOCKET_CONFIG)s
command=newrelic-admin run-program gunicorn --paste conf/websocket.ini --config conf/gunicorn-websocket.conf.py
stdout_logfile=NONE
stderr_logfile=NONE
stdout_events_enabled=true
stderr_events_enabled=true
autostart = %(ENV_ENABLE_WEBSOCKET)s

[program:websocket-monolithic]
command=newrelic-admin run-program gunicorn --paste conf/websocket-monolithic.ini --config conf/gunicorn-websocket-monolithic.conf.py
stdout_logfile=NONE
stderr_logfile=NONE
stdout_events_enabled=true
stderr_events_enabled=true
autostart = %(ENV_ENABLE_WEBSOCKET_MONOLITHIC)s
process_name = websocket
Comment on lines +31 to +38
Copy link
Contributor Author

@seanh seanh Dec 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Previously there was a single websocket "program" in supervisord.conf and a %(ENV_WEBSOCKET_CONFIG)s envvar was used to make the websocket config file vary between different QA/staging/production environments: conf/websocket-separate.ini (now renamed to just websocket.ini) or conf/websocket-monolithic.ini.

This will no longer work now that we have separate PasteDeploy and Gunicorn config files. You'd need two separate envvars WEBSOCKET_PASTEDEPLOY_CONFIG and WEBSOCKET_GUNICORN_CONFIG or some other scheme.

I've changed this to instead define two separate websocket and websocket-monolithic processes in supervisord.conf. We'll then set the ENABLE_WEBSOCKET and ENABLE_WEBSOCKET_MONOLITHIC envvars as necessary in each environment.

These ENABLE_{PROGRAM} envvars are a pattern that's available for all our programs/processes in all our repos. We already use ENABLE_WEBSOCKET=false in h's QA and Production (US) environments and then they have separate QA (WebSocket) and Production (WebSocket) environments that have ENABLE_WEB=false and ENABLE_WEBSOCKET=true. These ENABLE_*'s could also be used in h-periodic which needs to disable some of its beat processes in its Canada environment. Having a separate environment for Celery workers would be another example (ENABLE_WEB=false and ENABLE_WORKER=true). So it makes sense to also use the ENABLE_* config to toggle between websocket and websocket-monolithic in different environments.


[program:worker]
command=newrelic-admin run-program hypothesis celery worker --loglevel=INFO
stdout_logfile=NONE
Expand Down
11 changes: 0 additions & 11 deletions conf/websocket-dev.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,6 @@ secret_key: notverysecretafterall
# SQLAlchemy configuration -- See SQLAlchemy documentation
sqlalchemy.url: postgresql://postgres@localhost/postgres

[server:main]
use: egg:gunicorn#main
host: localhost
port: 5001
worker_class: h.streamer.Worker
graceful_timeout: 0
proc_name: websocket
# This is very low so you can see what happens when we run out
workers: 2
worker_connections: 8

[loggers]
keys = root, gunicorn.error, h

Expand Down
9 changes: 0 additions & 9 deletions conf/websocket-monolithic.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,6 @@
[app:main]
use: call:h.streamer:create_app

[server:main]
use: egg:gunicorn#main
bind: unix:/tmp/gunicorn-websocket.sock
worker_class: h.streamer.Worker
graceful_timeout: 0
proc_name: websocket
workers: 2
worker_connections: 8192

[loggers]
keys = root, gunicorn.error, h.streamer

Expand Down
10 changes: 0 additions & 10 deletions conf/websocket-separate.ini → conf/websocket.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,6 @@
[app:main]
use: call:h.streamer:create_app

[server:main]
use: egg:gunicorn#main
host: 0.0.0.0
port: 5000
worker_class: h.streamer.Worker
graceful_timeout: 0
proc_name: websocket
workers: 2
worker_connections: 8192

[loggers]
keys = root, gunicorn.error, h.streamer

Expand Down
2 changes: 1 addition & 1 deletion h/cli/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def bootstrap(app_url, dev=False):
else:
raise click.ClickException("the app URL must be set in production mode!")

config = "conf/development-app.ini" if dev else "conf/app.ini"
config = "conf/development.ini" if dev else "conf/production.ini"

paster.setup_logging(config)
request = Request.blank("/", base_url=app_url)
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/bin/run_data_task_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def test_reporting_tasks(self, environ):
sys.executable,
"bin/run_data_task.py",
"--config-file",
"conf/development-app.ini",
"conf/development.ini",
"--task",
task_name,
],
Expand Down