settings.prod: Use SWH_CONFIG_FILENAME to load & check swh config
Merge request reports
Activity
Build is green See https://jenkins.softwareheritage.org/job/DDEP/job/tox/79/ for more details.
mentioned in commit swh/infra/puppet/puppet-swh-site@8ccd4218
From what I can tell, in your
make_app_from_configfilefunction, theconfobject is never used, only checked. That feels very brittle to me and I think the checks should happen in the swh.deposit.settings.production module directly.It's standard practice in Django deployments to rely on environment variables set outside the WSGI process manager to load configuration; that's what the DJANGO_SETTINGS_MODULE environment variable is there for. I think it makes sense for our production settings module to also look at the SWH_CONFIG_FILENAME envvar to pull the settings that we don't want to publish in the main DJANGO_SETTINGS_MODULE (which is itself published in the swh.deposit module on PyPI).
I'm not too fussed about the wsgi module shipping a default value for DJANGO_SETTINGS_MODULE (as it's not supposed to be changed anyway), and frankly for SWH_CONFIG_FILENAME as well as it makes for easier deployment; but it's not too hard to set SWH_CONFIG_FILENAME explicitly on the gunicorn service declaration instead (via an
Environmentdirective in the systemd unit).
From what I can tell, in your make_app_from_configfile function, the conf object is never used, only checked. True.
That feels very brittle to me and I think the checks should happen in the swh.deposit.settings.production module directly.
ah, yeah, makes sense, that's where it's used anyway!
It's standard practice in Django deployments to rely on environment variables set outside the WSGI process manager to load configuration; that's what the DJANGO_SETTINGS_MODULE environment variable is there for. I think it makes sense for our production settings module to also look at the SWH_CONFIG_FILENAME envvar to pull the settings that we don't want to publish in the main DJANGO_SETTINGS_MODULE (which is itself published in the swh.deposit module on PyPI).
Yes, right. I think that's what i aimed at in swh/infra/puppet/puppet-swh-site!170 (closed).
I'm not too fussed about the wsgi module shipping a default value for DJANGO_SETTINGS_MODULE (as it's not supposed to be changed anyway), and frankly for SWH_CONFIG_FILENAME as well as it makes for easier deployment; but it's not too hard to set SWH_CONFIG_FILENAME explicitly on the gunicorn service declaration instead (via an Environment directive in the systemd unit).
Right, well, i already did the change in the manifest so i'll keep that ;)
Thanks, that helps a lot.
Some references in the commit message have been migrated:
- T1533 is now swh/meta#1533 (closed)
-
Build is green See https://jenkins.softwareheritage.org/job/DDEP/job/tox/84/ for more details.
mentioned in merge request swh-docker-dev!124 (closed)
