Skip to content

Commit

Permalink
feat!: A Better API for Derived Settings (#36192)
Browse files Browse the repository at this point in the history 
The Python API for declaring derived settings was confusing to the uninitiated
reader, and also prone to spelling mistakes. This replaces the API with one
that is more readable and more concise, and updates the implementation of
`derive_settings` to properly derive settings declared using the new API.

BREAKING CHANGE: The `derived` and `derived_collection_entry` function are
replaced with the `Derived` class. We do not expect those functions to have
been used outside of edx-platform, but if they are, this commit will cause them
to loudly ImportError.

Note that there should be NO change in behavior to the `derive_settings`
function, which we DO know to be used by some external edx-platform plugins.

Part of: #36215
  • Loading branch information
@kdmccormick
kdmccormick authored Feb 4, 2025
1 parent 237e16f commit 3227566
Show file tree
Hide file tree
Showing 7 changed files with 135 additions and 149 deletions.
55 changes: 14 additions & 41 deletions cms/envs/common.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@
get_theme_base_dirs_from_settings
)
from openedx.core.lib.license import LicenseMixin
from openedx.core.lib.derived import derived, derived_collection_entry
from openedx.core.lib.derived import Derived
from openedx.core.release import doc_version

# pylint: enable=useless-suppression
Expand Down Expand Up @@ -740,7 +740,7 @@
# Don't look for template source files inside installed applications.
'APP_DIRS': False,
# Instead, look for template source files in these dirs.
'DIRS': _make_mako_template_dirs,
'DIRS': Derived(_make_mako_template_dirs),
# Options specific to this backend.
'OPTIONS': {
'loaders': (
Expand All @@ -759,7 +759,7 @@
'NAME': 'mako',
'BACKEND': 'common.djangoapps.edxmako.backend.Mako',
'APP_DIRS': False,
'DIRS': _make_mako_template_dirs,
'DIRS': Derived(_make_mako_template_dirs),
'OPTIONS': {
'context_processors': CONTEXT_PROCESSORS,
'debug': False,
Expand All @@ -778,8 +778,6 @@
}
},
]
derived_collection_entry('TEMPLATES', 0, 'DIRS')
derived_collection_entry('TEMPLATES', 1, 'DIRS')
DEFAULT_TEMPLATE_ENGINE = TEMPLATES[0]

#################################### AWS #######################################
Expand Down Expand Up @@ -825,8 +823,7 @@
# Warning: Must have trailing slash to activate correct logout view
# (auth_backends, not LMS user_authn)
FRONTEND_LOGOUT_URL = '/logout/'
FRONTEND_REGISTER_URL = lambda settings: settings.LMS_ROOT_URL + '/register'
derived('FRONTEND_REGISTER_URL')
FRONTEND_REGISTER_URL = Derived(lambda settings: settings.LMS_ROOT_URL + '/register')

LMS_ENROLLMENT_API_PATH = "/api/enrollment/v1/"
ENTERPRISE_API_URL = LMS_INTERNAL_ROOT_URL + '/enterprise/api/v1/'
Expand Down Expand Up @@ -1316,8 +1313,7 @@
STATICI18N_FILENAME_FUNCTION = 'statici18n.utils.legacy_filename'
STATICI18N_ROOT = PROJECT_ROOT / "static"

LOCALE_PATHS = _make_locale_paths
derived('LOCALE_PATHS')
LOCALE_PATHS = Derived(_make_locale_paths)

# Messages
MESSAGE_STORAGE = 'django.contrib.messages.storage.session.SessionStorage'
Expand Down Expand Up @@ -2087,10 +2083,9 @@
# See annotations in lms/envs/common.py for details.
RETIRED_EMAIL_DOMAIN = 'retired.invalid'
# See annotations in lms/envs/common.py for details.
RETIRED_USERNAME_FMT = lambda settings: settings.RETIRED_USERNAME_PREFIX + '{}'
RETIRED_USERNAME_FMT = Derived(lambda settings: settings.RETIRED_USERNAME_PREFIX + '{}')
# See annotations in lms/envs/common.py for details.
RETIRED_EMAIL_FMT = lambda settings: settings.RETIRED_EMAIL_PREFIX + '{}@' + settings.RETIRED_EMAIL_DOMAIN
derived('RETIRED_USERNAME_FMT', 'RETIRED_EMAIL_FMT')
RETIRED_EMAIL_FMT = Derived(lambda settings: settings.RETIRED_EMAIL_PREFIX + '{}@' + settings.RETIRED_EMAIL_DOMAIN)
# See annotations in lms/envs/common.py for details.
RETIRED_USER_SALTS = ['abc', '123']
# See annotations in lms/envs/common.py for details.
Expand Down Expand Up @@ -2367,13 +2362,12 @@
############## Settings for Studio Context Sensitive Help ##############

HELP_TOKENS_INI_FILE = REPO_ROOT / "cms" / "envs" / "help_tokens.ini"
HELP_TOKENS_LANGUAGE_CODE = lambda settings: settings.LANGUAGE_CODE
HELP_TOKENS_VERSION = lambda settings: doc_version()
HELP_TOKENS_LANGUAGE_CODE = Derived(lambda settings: settings.LANGUAGE_CODE)
HELP_TOKENS_VERSION = Derived(lambda settings: doc_version())
HELP_TOKENS_BOOKS = {
'learner': 'https://edx.readthedocs.io/projects/open-edx-learner-guide',
'course_author': 'https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course',
}
derived('HELP_TOKENS_LANGUAGE_CODE', 'HELP_TOKENS_VERSION')

# Used with Email sending
RETRY_ACTIVATION_EMAIL_MAX_ATTEMPTS = 5
Expand Down Expand Up @@ -2876,15 +2870,15 @@ def _should_send_learning_badge_events(settings):
},
'org.openedx.content_authoring.xblock.published.v1': {
'course-authoring-xblock-lifecycle':
{'event_key_field': 'xblock_info.usage_key', 'enabled': _should_send_xblock_events},
{'event_key_field': 'xblock_info.usage_key', 'enabled': Derived(_should_send_xblock_events)},
},
'org.openedx.content_authoring.xblock.deleted.v1': {
'course-authoring-xblock-lifecycle':
{'event_key_field': 'xblock_info.usage_key', 'enabled': _should_send_xblock_events},
{'event_key_field': 'xblock_info.usage_key', 'enabled': Derived(_should_send_xblock_events)},
},
'org.openedx.content_authoring.xblock.duplicated.v1': {
'course-authoring-xblock-lifecycle':
{'event_key_field': 'xblock_info.usage_key', 'enabled': _should_send_xblock_events},
{'event_key_field': 'xblock_info.usage_key', 'enabled': Derived(_should_send_xblock_events)},
},
# LMS events. These have to be copied over here because lms.common adds some derived entries as well,
# and the derivation fails if the keys are missing. If we ever remove the import of lms.common, we can remove these.
Expand All @@ -2899,38 +2893,17 @@ def _should_send_learning_badge_events(settings):
"org.openedx.learning.course.passing.status.updated.v1": {
"learning-badges-lifecycle": {
"event_key_field": "course_passing_status.course.course_key",
"enabled": _should_send_learning_badge_events,
"enabled": Derived(_should_send_learning_badge_events),
},
},
"org.openedx.learning.ccx.course.passing.status.updated.v1": {
"learning-badges-lifecycle": {
"event_key_field": "course_passing_status.course.ccx_course_key",
"enabled": _should_send_learning_badge_events,
"enabled": Derived(_should_send_learning_badge_events),
},
},
}


derived_collection_entry('EVENT_BUS_PRODUCER_CONFIG', 'org.openedx.content_authoring.xblock.published.v1',
'course-authoring-xblock-lifecycle', 'enabled')
derived_collection_entry('EVENT_BUS_PRODUCER_CONFIG', 'org.openedx.content_authoring.xblock.duplicated.v1',
'course-authoring-xblock-lifecycle', 'enabled')
derived_collection_entry('EVENT_BUS_PRODUCER_CONFIG', 'org.openedx.content_authoring.xblock.deleted.v1',
'course-authoring-xblock-lifecycle', 'enabled')

derived_collection_entry(
"EVENT_BUS_PRODUCER_CONFIG",
"org.openedx.learning.course.passing.status.updated.v1",
"learning-badges-lifecycle",
"enabled",
)
derived_collection_entry(
"EVENT_BUS_PRODUCER_CONFIG",
"org.openedx.learning.ccx.course.passing.status.updated.v1",
"learning-badges-lifecycle",
"enabled",
)

################### Authoring API ######################

# This affects the Authoring API swagger docs but not the legacy swagger docs under /api-docs/.
Expand Down
47 changes: 13 additions & 34 deletions lms/envs/common.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
get_themes_unchecked,
get_theme_base_dirs_from_settings
)
from openedx.core.lib.derived import derived, derived_collection_entry
from openedx.core.lib.derived import Derived
from openedx.core.release import doc_version
from lms.djangoapps.lms_xblock.mixin import LmsBlockMixin

Expand Down Expand Up @@ -1395,7 +1395,7 @@ def _make_mako_template_dirs(settings):
# Don't look for template source files inside installed applications.
'APP_DIRS': False,
# Instead, look for template source files in these dirs.
'DIRS': _make_mako_template_dirs,
'DIRS': Derived(_make_mako_template_dirs),
# Options specific to this backend.
'OPTIONS': {
'context_processors': CONTEXT_PROCESSORS,
Expand All @@ -1404,7 +1404,6 @@ def _make_mako_template_dirs(settings):
}
},
]
derived_collection_entry('TEMPLATES', 1, 'DIRS')
DEFAULT_TEMPLATE_ENGINE = TEMPLATES[0]
DEFAULT_TEMPLATE_ENGINE_DIRS = DEFAULT_TEMPLATE_ENGINE['DIRS'][:]

Expand Down Expand Up @@ -1734,7 +1733,7 @@ def _make_mako_template_dirs(settings):
'DOC_STORE_CONFIG': DOC_STORE_CONFIG,
'OPTIONS': {
'default_class': 'xmodule.hidden_block.HiddenBlock',
'fs_root': lambda settings: settings.DATA_DIR,
'fs_root': Derived(lambda settings: settings.DATA_DIR),
'render_template': 'common.djangoapps.edxmako.shortcuts.render_to_string',
}
},
Expand All @@ -1744,7 +1743,7 @@ def _make_mako_template_dirs(settings):
'DOC_STORE_CONFIG': DOC_STORE_CONFIG,
'OPTIONS': {
'default_class': 'xmodule.hidden_block.HiddenBlock',
'fs_root': lambda settings: settings.DATA_DIR,
'fs_root': Derived(lambda settings: settings.DATA_DIR),
'render_template': 'common.djangoapps.edxmako.shortcuts.render_to_string',
}
}
Expand Down Expand Up @@ -2054,8 +2053,7 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring
for locale_path in settings.COMPREHENSIVE_THEME_LOCALE_PATHS:
locale_paths += (path(locale_path), )
return locale_paths
LOCALE_PATHS = _make_locale_paths
derived('LOCALE_PATHS')
LOCALE_PATHS = Derived(_make_locale_paths)

# Messages
MESSAGE_STORAGE = 'django.contrib.messages.storage.session.SessionStorage'
Expand Down Expand Up @@ -4658,13 +4656,12 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring
############## Settings for LMS Context Sensitive Help ##############

HELP_TOKENS_INI_FILE = REPO_ROOT / "lms" / "envs" / "help_tokens.ini"
HELP_TOKENS_LANGUAGE_CODE = lambda settings: settings.LANGUAGE_CODE
HELP_TOKENS_VERSION = lambda settings: doc_version()
HELP_TOKENS_LANGUAGE_CODE = Derived(lambda settings: settings.LANGUAGE_CODE)
HELP_TOKENS_VERSION = Derived(lambda settings: doc_version())
HELP_TOKENS_BOOKS = {
'learner': 'https://edx.readthedocs.io/projects/open-edx-learner-guide',
'course_author': 'https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course',
}
derived('HELP_TOKENS_LANGUAGE_CODE', 'HELP_TOKENS_VERSION')

############## OPEN EDX ENTERPRISE SERVICE CONFIGURATION ######################
# The Open edX Enterprise service is currently hosted via the LMS container/process.
Expand Down Expand Up @@ -4952,14 +4949,13 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring
# .. setting_description: Set the format a retired user username field gets transformed into, where {}
# is replaced with the hash of the original username. This is a derived setting that depends on
# RETIRED_USERNAME_PREFIX value.
RETIRED_USERNAME_FMT = lambda settings: settings.RETIRED_USERNAME_PREFIX + '{}'
RETIRED_USERNAME_FMT = Derived(lambda settings: settings.RETIRED_USERNAME_PREFIX + '{}')
# .. setting_name: RETIRED_EMAIL_FMT
# .. setting_default: retired__user_{}@retired.invalid
# .. setting_description: Set the format a retired user email field gets transformed into, where {} is
# replaced with the hash of the original email. This is a derived setting that depends on
# RETIRED_EMAIL_PREFIX and RETIRED_EMAIL_DOMAIN values.
RETIRED_EMAIL_FMT = lambda settings: settings.RETIRED_EMAIL_PREFIX + '{}@' + settings.RETIRED_EMAIL_DOMAIN
derived('RETIRED_USERNAME_FMT', 'RETIRED_EMAIL_FMT')
RETIRED_EMAIL_FMT = Derived(lambda settings: settings.RETIRED_EMAIL_PREFIX + '{}@' + settings.RETIRED_EMAIL_DOMAIN)
# .. setting_name: RETIRED_USER_SALTS
# .. setting_default: ['abc', '123']
# .. setting_description: Set a list of salts used for hashing usernames and emails on users retirement.
Expand Down Expand Up @@ -5447,11 +5443,11 @@ def _should_send_learning_badge_events(settings):
EVENT_BUS_PRODUCER_CONFIG = {
'org.openedx.learning.certificate.created.v1': {
'learning-certificate-lifecycle':
{'event_key_field': 'certificate.course.course_key', 'enabled': _should_send_certificate_events},
{'event_key_field': 'certificate.course.course_key', 'enabled': Derived(_should_send_certificate_events)},
},
'org.openedx.learning.certificate.revoked.v1': {
'learning-certificate-lifecycle':
{'event_key_field': 'certificate.course.course_key', 'enabled': _should_send_certificate_events},
{'event_key_field': 'certificate.course.course_key', 'enabled': Derived(_should_send_certificate_events)},
},
'org.openedx.learning.course.unenrollment.completed.v1': {
'course-unenrollment-lifecycle':
Expand Down Expand Up @@ -5513,33 +5509,16 @@ def _should_send_learning_badge_events(settings):
"org.openedx.learning.course.passing.status.updated.v1": {
"learning-badges-lifecycle": {
"event_key_field": "course_passing_status.course.course_key",
"enabled": _should_send_learning_badge_events,
"enabled": Derived(_should_send_learning_badge_events),
},
},
"org.openedx.learning.ccx.course.passing.status.updated.v1": {
"learning-badges-lifecycle": {
"event_key_field": "course_passing_status.course.ccx_course_key",
"enabled": _should_send_learning_badge_events,
"enabled": Derived(_should_send_learning_badge_events),
},
},
}
derived_collection_entry('EVENT_BUS_PRODUCER_CONFIG', 'org.openedx.learning.certificate.created.v1',
'learning-certificate-lifecycle', 'enabled')
derived_collection_entry('EVENT_BUS_PRODUCER_CONFIG', 'org.openedx.learning.certificate.revoked.v1',
'learning-certificate-lifecycle', 'enabled')

derived_collection_entry(
"EVENT_BUS_PRODUCER_CONFIG",
"org.openedx.learning.course.passing.status.updated.v1",
"learning-badges-lifecycle",
"enabled",
)
derived_collection_entry(
"EVENT_BUS_PRODUCER_CONFIG",
"org.openedx.learning.ccx.course.passing.status.updated.v1",
"learning-badges-lifecycle",
"enabled",
)

BEAMER_PRODUCT_ID = ""

Expand Down
6 changes: 2 additions & 4 deletions lms/envs/docs/README.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ For example:
for locale_path in settings.COMPREHENSIVE_THEME_LOCALE_PATHS:
locale_paths += (path(locale_path), )
return locale_paths
LOCALE_PATHS = _make_locale_paths
derived('LOCALE_PATHS')
LOCALE_PATHS = Derived(_make_locale_paths)
In this case, ``LOCALE_PATHS`` will be defined correctly at the end of the
settings module parsing no matter what ``REPO_ROOT``,
Expand Down Expand Up @@ -92,7 +91,6 @@ when nested within each other:
'NAME': 'mako',
'BACKEND': 'common.djangoapps.edxmako.backend.Mako',
'APP_DIRS': False,
'DIRS': _make_mako_template_dirs,
'DIRS': Derived(_make_mako_template_dirs),
},
]
derived_collection_entry('TEMPLATES', 1, 'DIRS')
11 changes: 0 additions & 11 deletions lms/envs/production.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -349,17 +349,6 @@ def get_env_setting(setting):
# use the one from common.py
MODULESTORE = convert_module_store_setting_if_needed(_YAML_TOKENS.get('MODULESTORE', MODULESTORE))

# After conversion above, the modulestore will have a "stores" list with all defined stores, for all stores, add the
# fs_root entry to derived collection so that if it's a callable it can be resolved. We need to do this because the
# `derived_collection_entry` takes an exact index value but the config file might have overridden the number of stores
# and so we can't be sure that the 2 we define in common.py will be there when we try to derive settings. This could
# lead to exceptions being thrown when the `derive_settings` call later in this file tries to update settings. We call
# the derived_collection_entry function here to ensure that we update the fs_root for any callables that remain after
# we've updated the MODULESTORE setting from our config file.
for idx, store in enumerate(MODULESTORE['default']['OPTIONS']['stores']):
if 'OPTIONS' in store and 'fs_root' in store["OPTIONS"]:
derived_collection_entry('MODULESTORE', 'default', 'OPTIONS', 'stores', idx, 'OPTIONS', 'fs_root')

BROKER_URL = "{}://{}:{}@{}/{}".format(CELERY_BROKER_TRANSPORT,
CELERY_BROKER_USER,
CELERY_BROKER_PASSWORD,
Expand Down
1 change: 1 addition & 0 deletions mypy.ini
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ files =
openedx/core/djangoapps/content_staging,
openedx/core/djangoapps/content_libraries,
openedx/core/djangoapps/xblock,
openedx/core/lib/derived.py,
openedx/core/types,
openedx/core/djangoapps/content_tagging,
xmodule/util/keys.py,
Expand Down
Loading

0 comments on commit 3227566

Please sign in to comment.