Skip to content

Commit 49cbd81

Browse files
committed
fix: Exactly preserve legacy settings dicts; rm KEYS_WITH_MERGED_VALUES
In the near term, we wish to precisely preserve the existing values of all Django settings exposed by lms/envs/production.py in order to avoid breaking legacy Django plugins without a proper announcement. That includes preserving the behavior of these old, redundant dicts: * ENV_TOKENS * AUTH_TOKENS * ENV_FEATURES * ENV_CELERY_QUEUES * ALTERNATE_QUEUE_ENVS Particularly, it means we need to ensure that updates to Django settings are reflected in these dicts. The most reliable way to do that is to change the yaml-loading logic so that these values are aliased to the corresponding values in the global namespace rather than deep-copied. Finally, we remove KEYS_WITH_MERGED_VALUES from the global namespace, and inline the remaining list. We have modified the list (specifically, we dropped the no-op MKTG_URL_OVERRIDES). Plugins should not be counting on the value of the list, so we remove it.
1 parent 5323ff7 commit 49cbd81

File tree

1 file changed

+40
-36
lines changed

1 file changed

+40
-36
lines changed

lms/envs/production.py

Lines changed: 40 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818

1919
import codecs
20-
import copy
2120
import datetime
2221
import os
2322

@@ -128,40 +127,40 @@ def get_env_setting(setting):
128127

129128
#######################################################################################################################
130129

131-
# A file path to a YAML file from which to load all the configuration for the edx platform
130+
# A file path to a YAML file from which to load configuration overrides for LMS.
132131
CONFIG_FILE = get_env_setting('LMS_CFG')
133132

134133
with codecs.open(CONFIG_FILE, encoding='utf-8') as f:
135-
__config__ = yaml.safe_load(f)
136-
137-
# _YAML_TOKENS contains the exact contents of the LMS_CFG YAML file.
138-
# We do splat the entirety of the LMS_CFG YAML file (except KEYS_WITH_MERGED_VALUES) into this module.
139-
# However, for precise backwards compatibility, we need to reference _YAML_TOKENS directly a few times,
140-
# particularly we need to derive Django setting values from YAML values.
141-
# This pattern is confusing and we discourage it. Rather than adding more _YAML_TOKENS references, please
142-
# consider just referencing this module's variables directly.
143-
_YAML_TOKENS = __config__
144-
145-
# Add the key/values from config into the global namespace of this module.
146-
# But don't override the FEATURES dict because we do that in an additive way.
147-
__config_copy__ = copy.deepcopy(__config__)
148-
149-
KEYS_WITH_MERGED_VALUES = [
150-
'FEATURES',
151-
'TRACKING_BACKENDS',
152-
'EVENT_TRACKING_BACKENDS',
153-
'JWT_AUTH',
154-
'CELERY_QUEUES',
155-
'MKTG_URL_LINK_MAP',
156-
'REST_FRAMEWORK',
157-
'EVENT_BUS_PRODUCER_CONFIG',
158-
]
159-
for key in KEYS_WITH_MERGED_VALUES:
160-
if key in __config_copy__:
161-
del __config_copy__[key]
162-
163-
vars().update(__config_copy__)
164134

135+
# _YAML_TOKENS starts out with the exact contents of the LMS_CFG YAML file.
136+
# Please avoid adding new references to _YAML_TOKENS. Such references make our settings logic more complex.
137+
# Instead, just reference the Django settings, which we define in the next step...
138+
_YAML_TOKENS = yaml.safe_load(f)
139+
140+
# Update the global namespace of this module with the key-value pairs from _YAML_TOKENS.
141+
# In other words: For (almost) every YAML key-value pair, define/update a Django setting with that name and value.
142+
vars().update({
143+
144+
# Note: If `value` is a mutable object (e.g., a dict), then it will be aliased between the global namespace and
145+
# _YAML_TOKENS. In other words, updates to `value` will manifest in _YAML_TOKENS as well. This is intentional,
146+
# in order to maintain backwards compatibility with old Django plugins which use _YAML_TOKENS.
147+
key: value
148+
for key, value in _YAML_TOKENS.items()
149+
150+
# Do NOT define/update Django settings for these particular special keys.
151+
# We handle each of these with its special logic (below, in this same module).
152+
# For example, we need to *update* the default FEATURES dict rather than wholesale *override* it.
153+
if key not in [
154+
'FEATURES',
155+
'TRACKING_BACKENDS',
156+
'EVENT_TRACKING_BACKENDS',
157+
'JWT_AUTH',
158+
'CELERY_QUEUES',
159+
'MKTG_URL_LINK_MAP',
160+
'REST_FRAMEWORK',
161+
'EVENT_BUS_PRODUCER_CONFIG',
162+
]
163+
})
165164

166165
try:
167166
# A file path to a YAML file from which to load all the code revisions currently deployed
@@ -554,11 +553,16 @@ def get_env_setting(setting):
554553

555554
# This is at the bottom because it is going to load more settings after base settings are loaded
556555

557-
# ENV_TOKENS and AUTH_TOKENS are included for reverse compatibility.
558-
# Removing them may break plugins that rely on them.
559-
# Please do not add new references to them... just use `django.conf.settings` instead.
560-
ENV_TOKENS = __config__
561-
AUTH_TOKENS = __config__
556+
# These dicts are defined solely for BACKWARDS COMPATIBILITY with existing plugins which may theoretically
557+
# rely upon them. Please do not add new references to these dicts!
558+
# - If you need to access the YAML values in this module, use _YAML_TOKENS.
559+
# - If you need to access to these values elsewhere, use the corresponding rendered `settings.*`
560+
# value rathering than diving into these dicts.
561+
ENV_TOKENS = _YAML_TOKENS
562+
AUTH_TOKENS = _YAML_TOKENS
563+
ENV_FEATURES = _YAML_TOKENS.get("FEATURES", {})
564+
ENV_CELERY_QUEUES = _YAML_CELERY_QUEUES
565+
ALTERNATE_QUEUE_ENVS = _YAML_ALTERNATE_WORKER_QUEUES
562566

563567
# Load production.py in plugins
564568
add_plugins(__name__, ProjectType.LMS, SettingsType.PRODUCTION)

0 commit comments

Comments
 (0)