openedx / openedx/openedx-platform

Convert `getattr(settings, "FOO")` to `settings.FOO`

Open
#38,861 0 comments 0 reactions 0 assignees View on GitHub

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.
  • 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:

  1. Make a quick ADR which we can use as guidance for devs who reference settings in the future
  2. 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)
  3. 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
  4. Convert the getattr(settings, "FOO", <default>) call site into settings.FOO.
  5. 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.