swh.web.config: Allow configuration through environment variable
ClosedPublic
Actions

Authored by ardumont on Feb 23 2019, 1:53 AM.

Details

Reviewers

anlambert
douardda
vlorentz

Group Reviewers

Reviewers

Commits

rDWAPPSb347caf24c22: requirements.txt: Add missing dependency
rDWAPPS83079c93906f: swh.web.config: Allow configuration through environment variable

Summary

This is not removing the implicit configuration yet. This is just
adaptation to allow a similar behavior between other swh services.

In production, the actual environment variable is actually set (and
use the same value as before). So with this, at the next deployment,
we should see no difference.

The next step (later) would be to remove the implicit configuration
part (there are quite some ;).

Related T1533
Related D1161 (deposit, might be the nearest to the webapp)
Related D1167 D1178 D1170 rDSCHce90908cd0c32ce978ae813aa73235850cdeb4d2

Also Add missing dependency in requirements.txt (or docker build fails
without it). Docker adaptation will follow in another diff.

Test Plan

tox

Diff Detail

Repository

rDWAPPS Web applications

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Feb 23 2019, 1:53 AM

Harbormaster completed remote builds in B4416: Diff 3725.Feb 23 2019, 1:57 AM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/176/ for more details.

ardumont added a child revision: D1183: swh-web: Unify production and development start up.Feb 23 2019, 1:57 AM

ardumont mentioned this in T1533: Make sure api server uses explicit configurations.Feb 23 2019, 1:58 AM

vlorentz accepted this revision.Feb 25 2019, 10:44 AM

This revision is now accepted and ready to land.Feb 25 2019, 10:44 AM

I'm ok with this. Regarding removing the implicit configuration embedded in conf.py, I think moving it to a separate yml file into the repository
would be a good idea. This will allow to track all declared configuration keys and their default values and use this conf by default for
local development.

requirements.txt
20 ↗	(On Diff #3725)	Why is this new dependency required ?

ardumont marked an inline comment as done.Feb 25 2019, 11:24 AM

ardumont added inline comments.

requirements.txt
20 ↗	(On Diff #3725)	in docker, it complained not having it.

In D1182#25208, @anlambert wrote:

I'm ok with this. Regarding removing the implicit configuration embedded in conf.py, I think moving it to a separate yml file into the repository
would be a good idea. This will allow to track all declared configuration keys and their default values and use this conf by default for
local development.

To be clear, i wanted to do more.
As i had no time left, i kinda try to find the sweet spot between aligning swh-web with our other modules.
What you propose sounds reasonable.

I'll merge this in the mean time.

Cheers,

Closed by commit rDWAPPS83079c93906f: swh.web.config: Allow configuration through environment variable (authored by ardumont). · Explain WhyFeb 25 2019, 12:11 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

swh/

web/

config.py

26 lines

Diff 3735

View Options

swh.web.config: Allow configuration through environment variableClosedPublicActions