openedx / openedx/openedx-platform
Convert `getattr(settings, "FOO")` to `settings.FOO`
Open
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 8.2k
- Forks
- 4.4k
- Avg merge
- 6d 18h
- Merged PRs (30d)
- 42
Description
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.
- With
- 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
- With
- Analysis
getattrinvocations 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>), ...
- the definition, if there is one (
- With
settings., there's exactly one default: the definition (FOO = "<default1>")
- With
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 intosettings.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
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by cataloging getattr(settings, ...) invocations across the repository and review the existing definitions in openedx, lms, and cms/envs/common.py. Draft the ADR, identify which services use each setting, and run the relevant tests after converting settings references in batches. Done means required settings have behavior-preserving defaults in the appropriate common.py files and the test suite passes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100