Skip to content

Commit a1dd86b

Browse files
feanilclaude
andcommitted
temp: report FEATURES-as-dict warning at the real caller
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>
1 parent 20636c5 commit a1dd86b

1 file changed

Lines changed: 33 additions & 8 deletions

File tree

openedx/core/lib/features_setting_proxy.py

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""
22
Features Proxy Implementation
33
"""
4+
import traceback
45
import warnings
56
from collections.abc import Mapping, MutableMapping
67

@@ -57,14 +58,32 @@ def __getitem__(self, key):
5758
return value
5859
return self.ns[key]
5960

60-
def __setitem__(self, key, value):
61-
"""Sets a key-value pair while emitting a deprecation warning about using FEATURES as a dict."""
61+
# Frames to skip when walking the stack for the [caller: ...] suffix.
62+
# mock.patch.dict's machinery is several frames deep, so simple stacklevel
63+
# arithmetic can't reach the user test code on its own.
64+
_STACK_SKIP_PATTERNS = ('features_setting_proxy.py', '/unittest/mock.py', '/contextlib.py')
65+
66+
def _warn_dict_access(self, key, value):
67+
"""Emit the FEATURES-as-dict deprecation warning, attributed to the caller of __setitem__/update."""
68+
# stacklevel=3 walks past warnings.warn -> _warn_dict_access -> caller-of-_warn_dict_access,
69+
# so the warning is reported from the line that actually mutated FEATURES (test code or app code).
70+
stack = traceback.extract_stack()
71+
user_frame = next(
72+
(f for f in reversed(stack[:-1]) if not any(p in f.filename for p in self._STACK_SKIP_PATTERNS)),
73+
None,
74+
)
75+
suffix = f" [caller: {user_frame.filename}:{user_frame.lineno}]" if user_frame else ""
6276
warnings.warn(
6377
f"Accessing FEATURES as a dict is deprecated. "
64-
f"Add '{key} = {value!r}' to your Django settings module instead of modifying FEATURES.",
78+
f"Add '{key} = {value!r}' to your Django settings module instead of modifying FEATURES."
79+
f"{suffix}",
6580
DeprecationWarning,
66-
stacklevel=2
81+
stacklevel=3,
6782
)
83+
84+
def __setitem__(self, key, value):
85+
"""Sets a key-value pair while emitting a deprecation warning about using FEATURES as a dict."""
86+
self._warn_dict_access(key, value)
6887
self.ns[key] = value
6988

7089
def __delitem__(self, key):
@@ -114,21 +133,27 @@ def update(self, other=(), /, **kwds):
114133
proxy.update({'FEATURE_A': True}, FEATURE_B=False)
115134
-> other={'FEATURE_A': True}; kwds = {'FEATURE_B': False}
116135
"""
136+
# We bypass __setitem__ and emit the warning here so that stacklevel
137+
# points at the caller of update() rather than at this method's body.
117138
if isinstance(other, Mapping):
118139
# Handles objects that formally conform to the Mapping interface
119140
# Mapping-like types: defaultdict, OrderedDict, Counter
120141
for key in other:
121-
self[key] = other[key]
142+
self._warn_dict_access(key, other[key])
143+
self.ns[key] = other[key]
122144
elif hasattr(other, "keys"):
123145
# Fallback for objects that implement a .keys() method but
124146
# may not formally subclass Mapping
125147
for key in other.keys():
126-
self[key] = other[key]
148+
self._warn_dict_access(key, other[key])
149+
self.ns[key] = other[key]
127150
else:
128151
for key, value in other:
129-
self[key] = value
152+
self._warn_dict_access(key, value)
153+
self.ns[key] = value
130154
for key, value in kwds.items():
131-
self[key] = value
155+
self._warn_dict_access(key, value)
156+
self.ns[key] = value
132157

133158
def copy(self):
134159
"""

0 commit comments

Comments
 (0)