Page MenuHomeSoftware Heritage

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

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

Details

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

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 added inline comments.
requirements.txt
20 ↗(On Diff #3725)

in docker, it complained not having it.

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,

This revision was automatically updated to reflect the committed changes.