temp: report FEATURES-as-dict warning at the real caller#38819
Draft
feanil wants to merge 25 commits into
Draft
Conversation
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>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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>
Pushed standalone first so CI surfaces the full list of caller sites before the migration commits land. FeaturesProxy.__setitem__ used stacklevel=2 and update() delegated to __setitem__, so every warning was reported at update()'s own 'self[key] = other[key]' line in this file (89) instead of the test or app code that actually mutated FEATURES. Why the helper instead of just bumping stacklevel: stacklevel is a frame count from warnings.warn upward. Bumping __setitem__'s stacklevel from 2 to 3 would fix the update()->__setitem__ path but would overshoot by one frame on direct __setitem__ calls (e.g. fp['X'] = True), reporting from the caller's caller. The two paths need different stacklevel arithmetic. Factors the emit into _warn_dict_access(stacklevel=3) so both __setitem__ and update() can call it with the same constant; each contributes exactly one intermediate frame. update() now writes to self.ns directly to avoid double-warning. No behavior change beyond the reported source location. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Jun 26, 2026
feanil
force-pushed
the
feanil/features-dict-migration
branch
from
346382f to
520006a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Third PR in the FEATURES-as-dict migration stack. Stacked on #38772.
Single
temp:commit that fixesFeaturesProxy'sstacklevelso theAccessing FEATURES as a dict is deprecatedDeprecationWarning points at the real caller instead of always reporting line 89 of the proxy itself. Used to drive the migration: each green-CI run surfaces the list of flags still left to migrate.Not for merge — this PR exists so we can iterate on the surfaced list while the upstream bridge / migration PRs land. Will be closed once every flag is migrated and
FeaturesProxyis deleted.Stack: