Rationale
- Documentation
- With
getattr, it is possible to ad-hoc check settings which were never defined in (lms,cms,openedx)/envs/common.py, adding hidden config knobs to the platform.
- With
.settings, there must be a common.py definition, which helps operators reason about configuration and gives us a place to document settings.
- Robustness
- With
getattr, a misspelling of FOO will silently fall to the default.
- With
settings. a misspelling of FOO will raise an AttributeError which we can find and fix
- Analysis
getattr invocations are often skipped by mypy, pylint, ruff, etc due to their dynamic nature
- Regular calls to
settings. can be statically analyzed for type correctness etc
- Defaults
- With
getattr, you can have conflicting defaults defined in 2+ different places:
- the definition, if there is one (
FOO = "<default1>")
- each call site
getattr(settings, "FOO", <default2>), getattr(settings, "FOO", <default3>), ...
- With
settings., there's exactly one default: the definition (FOO = "<default1>")
Tasks
Roughly:
- Make a quick ADR which we can use as guidance for devs who reference settings in the future
- Catalog all invocations of
getattr(settings, ...) and make a best guess as to which services use them (LMS, CMS, both -- will be hard to determine this with 100% confidence)
- Add the necessary definitions to (openedx,lms,cms)/envs/common.py.
- Define it with a default which does not change the behavior of the base system
- This will involve, in some cases, moving definitions up from lms/envs/common.py into openedx/envs/common.py
- Convert the
getattr(settings, "FOO", <default>) call site into settings.FOO.
- Tests will fail if a setting was defined in the wrong place. Fix and repeat.
It might make sense to do 3-5 in batches
Rationale
getattr, it is possible to ad-hoc check settings which were never defined in (lms,cms,openedx)/envs/common.py, adding hidden config knobs to the platform..settings, there must be a common.py definition, which helps operators reason about configuration and gives us a place to document settings.getattr, a misspelling of FOO will silently fall to the default.settings.a misspelling of FOO will raise an AttributeError which we can find and fixgetattrinvocations are often skipped by mypy, pylint, ruff, etc due to their dynamic naturesettings.can be statically analyzed for type correctness etcgetattr, you can have conflicting defaults defined in 2+ different places:FOO = "<default1>")getattr(settings, "FOO", <default2>),getattr(settings, "FOO", <default3>), ...settings., there's exactly one default: the definition (FOO = "<default1>")Tasks
Roughly:
getattr(settings, ...)and make a best guess as to which services use them (LMS, CMS, both -- will be hard to determine this with 100% confidence)getattr(settings, "FOO", <default>)call site intosettings.FOO.It might make sense to do 3-5 in batches