feat: migrate callers off FEATURES-as-dict access by feanil · Pull Request #38772 · openedx/openedx-platform

feanil · 2026-06-16T18:27:40Z

Second PR in the FEATURES-as-dict migration stack. #38818 (the bridge) is merged.

Per-flag refactor: commits that move callers off settings.FEATURES['X'] / patch.dict(settings.FEATURES, …) and onto settings.X / @override_settings(X=…). The underlying flat settings already exist in lms/envs/common.py, cms/envs/common.py, and the shared openedx/envs/common.py — the env files were previously flattened, so this PR only touches call sites (plus a few flag definitions that needed to move into the shared file).

`settings.X` vs `getattr(settings, 'X', <default>)`

Most reads drop the getattr fallback in favor of a bare settings.X — but not all. The rule is: a bare settings.X requires that the setting be defined in the common.py file of every process that can reach the read.

Setting defined in openedx/envs/common.py (shared, imported by both LMS and CMS) — use bare settings.X at every caller.
Setting defined only in lms/envs/common.py — use bare settings.X only if the caller is exclusively reachable from the LMS process. If the caller lives under common/djangoapps/ or openedx/core/ and can be pulled in by CMS test collection or CMS request handling, keep getattr(settings, 'X', <default>) so CMS doesn't AttributeError.
Setting defined only in cms/envs/common.py — the mirror rule. Bare settings.X at CMS-only callers; getattr at shared/LMS-reachable callers. This deliberately keeps Studio-only concepts (e.g. ENABLE_CREATOR_GROUP, DISABLE_COURSE_CREATION) from leaking into LMS settings.
Setting not previously defined anywhere — the migration adds an explicit definition (with a proper .. toggle_name: / .. setting_name: annotation) in the smallest common file that covers all callers, then drops getattr at the call sites.

hasattr(settings, 'X') is used in a few module-load contexts (test util constants) where we specifically want "is the setting defined" rather than "what is its value" — safe against a missing setting on either process.

Stack:

feat: bridge FEATURES proxy reads to @override_settings overrides #38818 — bridge → master (merged)
(this PR) migrations → master
temp: report FEATURES-as-dict warning at the real caller #38819 — temp diagnostic (lists flags still to migrate) → migrations

kdmccormick

looks good once rebased & green.

As I mentioned before I dislike all the new getattr invocations, but I've come around that it's the right way to push through this FEATURES refactor. I wrote up a followup ticket which we can put into the config simplification epic somewhere post-FEATURES-refactor #38861

kdmccormick · 2026-07-08T13:56:10Z

 TPA_FEATURES_KEY = 'ENABLE_THIRD_PARTY_AUTH'
-TPA_FEATURE_ENABLED = TPA_FEATURES_KEY in settings.FEATURES
+TPA_FEATURE_ENABLED = hasattr(settings, TPA_FEATURES_KEY)


not your problem to fix, but 🤮🤮🤮

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Production reads use getattr(settings, ...); tests use @override_settings(ENABLE_CREATOR_GROUP=...). Mixed-flag patches keep the other flag in patch.dict until its own migration. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- Production reads use getattr(settings, ...); tests use @override_settings(CERTIFICATES_HTML_VIEW=...). - Drop now-unused FEATURES_WITH_CERTS_ENABLED/_DISABLED helpers. - FEATURES_WITH_CUSTOM_CERTS_ENABLED keeps CUSTOM_CERTIFICATE_TEMPLATES_ENABLED until that flag migrates. - Mixed-flag patches keep the other flag in patch.dict. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- test_200_for_lti_provider's post-`with` modulestore assertions relied on a state leak (the old code aliased settings.FEATURES without .copy(), so the mutation persisted). They now run inside the with-block. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- Tests that previously had @patch.dict on setUp/setUpClass move the override to the class level. @override_settings as a setUp decorator conflicts with CacheIsolationMixin's own override_settings push/pop — the wrong layer gets popped on setUp exit, leaving settings.CACHES broken for subsequent test methods. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Running `pytest openedx/core/djangoapps/embargo/tests/` in isolation fails one test because `test_redirect_if_blocked_courseware` mocks `embargo.api.check_course_access`, and the mocked call triggers the first-time import of `embargo.views`. views.py's `from .api import check_course_access` captures the MagicMock as `views.check_course_access` permanently. CI doesn't hit this because earlier shard directories preload `embargo.views` before any mock fires. Reproduces on master. Calling via the module attribute resolves at call time, so future mocks on the api function stay visible to the view. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…-dict Default flips from raising KeyError (`settings.FEATURES['…']` direct subscript, only safe inside an active patch) to False (`getattr` with default) — the flag isn't defined in any env file. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…s-dict Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Flat setting was already being assigned to via the instructor plugin's plugin_settings hook (settings.MAX_ENROLLMENT_INSTR_BUTTONS = 200) — only the readers were still going through settings.FEATURES. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…as-dict Flat setting was already being assigned in the instructor plugin's plugin_settings hook — only the two readers were still going through settings.FEATURES.get. Also fixes the annotation that read FEATURES['ENABLE_CERTIFICATES_INSTRUCTOR_MANAGE] (missing closing quote, wrong location). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…dict Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

feanil force-pushed the feanil/fix-deprecation-warnings branch from 1b8c4e8 to 7f5dbc2 Compare June 16, 2026 18:52

feanil force-pushed the feanil/features-dict-migration branch 3 times, most recently from 9ffdb02 to f9359f9 Compare June 17, 2026 14:27

Base automatically changed from feanil/fix-deprecation-warnings to master June 17, 2026 15:33

feanil force-pushed the feanil/features-dict-migration branch 4 times, most recently from 60dd592 to 4ecd09f Compare June 23, 2026 18:42

feanil force-pushed the feanil/features-dict-migration branch 4 times, most recently from 858d762 to 346382f Compare June 26, 2026 19:30

feanil changed the base branch from master to feanil/features-dict-bridge June 26, 2026 19:31

This was referenced Jun 26, 2026

feat: bridge FEATURES proxy reads to @override_settings overrides #38818

Merged

temp: report FEATURES-as-dict warning at the real caller #38819

Closed

feanil force-pushed the feanil/features-dict-migration branch from 346382f to 520006a Compare June 26, 2026 19:59

kdmccormick approved these changes Jul 8, 2026

View reviewed changes

feanil force-pushed the feanil/features-dict-bridge branch from 3a686d6 to 1299ef1 Compare July 9, 2026 13:57

feanil force-pushed the feanil/features-dict-migration branch from 520006a to 20636c5 Compare July 9, 2026 14:03

Base automatically changed from feanil/features-dict-bridge to master July 9, 2026 14:21

feanil force-pushed the feanil/features-dict-migration branch 4 times, most recently from df5644a to 7009266 Compare July 10, 2026 13:58

feanil and others added 3 commits July 10, 2026 16:47

refactor: migrate ENABLE_AUTHN_MICROFRONTEND off FEATURES-as-dict

7039df4

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

feanil force-pushed the feanil/features-dict-migration branch from c3b0a3b to 1515d53 Compare July 10, 2026 20:47

feanil and others added 26 commits July 10, 2026 17:05

refactor: migrate ENABLE_COURSE_OLX_VALIDATION off FEATURES-as-dict

6e92d10

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

refactor: migrate ENABLE_SPECIAL_EXAMS off FEATURES-as-dict

596fa9a

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

refactor: migrate ENABLE_INTEGRITY_SIGNATURE off FEATURES-as-dict

f7d9c16

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

refactor: migrate CUSTOM_COURSES_EDX off FEATURES-as-dict

7aa5910

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

refactor: migrate EMBARGO off FEATURES-as-dict

84a0992

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

refactor: migrate ENTRANCE_EXAMS off FEATURES-as-dict

cd0bf78

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

refactor: migrate STUDIO_REQUEST_EMAIL off FEATURES-as-dict

45c1f21

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

refactor: migrate ENABLE_UNICODE_USERNAME off FEATURES-as-dict

40c2d1f

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

refactor: migrate ENABLE_TEAMS off FEATURES-as-dict

917170f

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

refactor: migrate DISABLE_COURSE_CREATION off FEATURES-as-dict

de9584b

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

refactor: migrate ALLOW_UNICODE_COURSE_ID off FEATURES-as-dict

f5b8d7e

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

refactor: migrate XQA_SERVER off FEATURES-as-dict

f52f4b2

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

refactor: migrate LICENSING off FEATURES-as-dict

5ffca44

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

refactor: migrate ENABLE_LMS_MIGRATION off FEATURES-as-dict

7a8e369

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

refactor: migrate ENABLE_THIRD_PARTY_AUTH off FEATURES-as-dict

ad9a429

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

refactor: migrate ENABLE_LTI_PII_ACKNOWLEDGEMENT off FEATURES-as-dict

a73f635

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

refactor: migrate EXPOSE_CACHE_PROGRAMS_ENDPOINT off FEATURES-as-dict

5183c82

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

refactor: migrate CUSTOM_CERTIFICATE_TEMPLATES_ENABLED off FEATURES-a…

dee8f35

…s-dict Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

refactor: migrate ALLOW_EMAIL_ADDRESS_CHANGE off FEATURES-as-dict

0ba5029

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

refactor: migrate ENABLE_FORUM_DAILY_DIGEST off FEATURES-as-dict

cdcc8d0

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

refactor: migrate CERTIFICATES_INSTRUCTOR_GENERATION off FEATURES-as-…

873ee59

…dict Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

feanil force-pushed the feanil/features-dict-migration branch from 113517b to 873ee59 Compare July 10, 2026 21:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: migrate callers off FEATURES-as-dict access#38772

feat: migrate callers off FEATURES-as-dict access#38772
feanil wants to merge 30 commits into
masterfrom
feanil/features-dict-migration

feanil commented Jun 16, 2026 •

edited

Loading

Uh oh!

kdmccormick left a comment

Uh oh!

kdmccormick Jul 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

feanil commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

settings.X vs getattr(settings, 'X', <default>)

Uh oh!

kdmccormick left a comment

Choose a reason for hiding this comment

Uh oh!

kdmccormick Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

feanil commented Jun 16, 2026 •

edited

Loading

`settings.X` vs `getattr(settings, 'X', <default>)`