Skip to content

Commit 1299ef1

Browse files
feanilclaude
andcommitted
feat: bridge FEATURES proxy reads to @override_settings overrides
Lets @override_settings(KEY=val) be seen through both settings.KEY and settings.FEATURES['KEY']/.get('KEY'), so tests can migrate to the flat setting without first updating every production reader that still does settings.FEATURES['KEY']. We walk only the UserSettingsHolder chain that @override_settings pushes onto django.conf.settings._wrapped, NOT the bottom Settings layer. Django's bottom Settings is a *snapshot* of the settings module's globals taken at init time, so it doesn't reflect runtime mutations of the module's globals — which is exactly what legacy patch.dict(settings.FEATURES, ...) does via proxy.ns. Reading the snapshot would mask those mutations and break legacy tests; walking only the explicit override layers lets both paths coexist during the migration. ns kept as-is. A stateless redesign (drop ns, route everything through django.conf.settings, set FEATURES = FeaturesProxy() in each env file) was considered but rejected for this step because FEATURES.copy() and __iter__ are widely used to snapshot the full set of feature flags; without ns there is no clean source of truth for "which keys are features". Once all callers have moved off the dict pattern the proxy can be deleted outright (every settings.FEATURES['X'] becomes settings.X), making the stateless refactor unnecessary. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f999886 commit 1299ef1

1 file changed

Lines changed: 35 additions & 3 deletions

File tree

openedx/core/lib/features_setting_proxy.py

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,37 @@ def __init__(self, namespace=None):
2424
"""Store the namespace (as a dict)"""
2525
self.ns = namespace or {}
2626

27+
_NOT_OVERRIDDEN = object()
28+
29+
def _resolve(self, key):
30+
"""Return key's value if it's been overridden via @override_settings, else self._NOT_OVERRIDDEN.
31+
32+
We deliberately walk only the UserSettingsHolder chain (the layers that
33+
@override_settings pushes onto django.conf.settings._wrapped) rather
34+
than reading django.conf.settings.X directly. Django's bottom Settings
35+
layer is a *snapshot* taken at init time, so it doesn't reflect runtime
36+
mutations of the settings module's globals (which is what
37+
proxy.ns mutations and legacy patch.dict(settings.FEATURES, ...) do).
38+
Walking only the explicit override layers lets the new
39+
@override_settings(X=Y) path work while leaving the legacy patch.dict
40+
path untouched.
41+
"""
42+
from django.conf import settings as django_settings
43+
wrapped = django_settings._wrapped # pylint: disable=protected-access
44+
# UserSettingsHolder has a `default_settings` attribute and stores
45+
# explicit overrides in its own __dict__; the bottom Settings has no
46+
# default_settings, so the loop terminates there.
47+
while hasattr(wrapped, 'default_settings'):
48+
if key in wrapped.__dict__:
49+
return wrapped.__dict__[key]
50+
wrapped = wrapped.default_settings
51+
return self._NOT_OVERRIDDEN
52+
2753
def __getitem__(self, key):
28-
"""Retrieve a feature flag by key"""
54+
"""Retrieve a feature flag by key, preferring @override_settings overrides."""
55+
value = self._resolve(key)
56+
if value is not self._NOT_OVERRIDDEN:
57+
return value
2958
return self.ns[key]
3059

3160
def __setitem__(self, key, value):
@@ -49,14 +78,17 @@ def __len__(self):
4978
return len(self.ns)
5079

5180
def __contains__(self, key):
52-
return key in self.ns
81+
return self._resolve(key) is not self._NOT_OVERRIDDEN or key in self.ns
5382

5483
def clear(self):
5584
"""Remove all feature flags from the namespace."""
5685
self.ns.clear()
5786

5887
def get(self, key, default=None):
59-
"""Standard dict-style get with default"""
88+
"""Standard dict-style get with default; prefers @override_settings overrides."""
89+
value = self._resolve(key)
90+
if value is not self._NOT_OVERRIDDEN:
91+
return value
6092
return self.ns.get(key, default)
6193

6294
def update(self, other=(), /, **kwds):

0 commit comments

Comments
 (0)