From b1d4e466d5bca150edbb11c0d5a1692c75889437 Mon Sep 17 00:00:00 2001 From: "Kyle D. McCormick" Date: Wed, 5 Feb 2025 13:00:16 -0500 Subject: [PATCH 1/9] docs: Settings Simplification ADR --- .../0022-settings-simplification.rst | 298 ++++++++++++++++++ 1 file changed, 298 insertions(+) create mode 100644 docs/decisions/0022-settings-simplification.rst diff --git a/docs/decisions/0022-settings-simplification.rst b/docs/decisions/0022-settings-simplification.rst new file mode 100644 index 00000000000..32409fb30bf --- /dev/null +++ b/docs/decisions/0022-settings-simplification.rst @@ -0,0 +1,298 @@ +Django settings simplification +############################## + +Status +****** + +Accepted + +Implementation tracked by: https://github.com/openedx/edx-platform/issues/36215 + +Context +******* + +OEP-45 declares that sites will configure each IDA's (indepently-deployable +application's) Django settings with an ``_CFG`` yaml file, parsed and +loaded by a single upstream-provided ``DJANGO_SETTINGS_MODULE``. this contrasts +with the django convention, which is that sites override Django settings using +a their own ``DJANGO_SETTINGS_MODULE``. The rationale is that all Open edX +customization can be reasonably specified in YAML; therefore, it is +operationally safer to avoid using a custom ``DJANGO_SETTINGS_MODULE``, and it +is operationally desirable for all operation modes to execute the same Python +module for configuration. This was `briefly discussed in the oep-45 review +`_. + +For example, in theory, the upstream production LMS config might be named +``lms/settings/settings.py`` and work like this: + +* import ``lms/settings/required.py``, which declares settings that must be + overridden. +* import ``lms/settings/defaults.py``, which defines reasonable defaults for + all other settings. +* load ``/openedx/config/lms.yml``, which should override every setting + declared in required.py and override some settings defined in defaults.py. +* apply some minimal merging and/or conditional logic to handle yaml values + which are not simple overrides (e.g., ``FEATURES``, which needs to be + merged). + +The upstream production CMS config would exist in parallel. + +However, as of Sumac, we do not know of any site other than edx.org that +successfully uses only YAML files for configuration. Furthermore, +upstream-provided ``DJANGO_SETTINGS_MODULE`` which loads these yaml files +(``lms/envs/production.py``) is not simple: it declares defaults, imports from +other Django settings modules, sets more defaults, handles dozens of special +cases, and has a special Open-edX-specific "derived settings" mechanism to +handle settings that depend on other settings. + +Tutor does provide YAML files, but *it also has custom production and +development settings files*! The result is that we have multiple layers of +indirection setting between edx-platform's common base settings, and the Django +settings rendered into the actual community-supported Open edX distribution +(Tutor). Specifically, production edx-platform configuration currently works +like this: + +* ``lms/envs/tutor/production.py``... + + * is generated by Tutor from the template + ``tutor/templates/apps/openedx/settings/lms/production.py``, + + * which derives + ``tutor/templates/apps/openedx/settings/partials/common_lms.py``, + + * which derives + ``tutor/templates/apps/openedx/settings/partials/common_all.py``; + + * and uses templates vars from Tutor configuration (``config.yml``), + + * and invokes hooks from any enable Tutor plugins; + + * it imports ``lms/envs/production.py``, + + * which imports ``lms/envs/common.py``, + + * which sets production-inappropriate defaults; + + * it sets more defaults, some of the edX.org-specific; + + * it loads ``/openedx/config/lms.yml``... + + * which is generated by Tutor from template + ``tutor/templates/apps/openedx/config/lms.env.yml`` + + * which derives + ``tutor/templates/apps/openedx/config/partials/auth.yml``; + + * it reverts some of ``lms.yml`` with new "defaults"; + + * and it uses certain values ``/openedx/config/lms.yml`` to conditionally + override more settings and update certain dictionary settings, in a way + which is not documented. + +* ``cms/envs/tutor/production.py``... + + * is generated by Tutor from the template + ``tutor/templates/apps/openedx/settings/cms/production.py``, + + * which derives + ``tutor/templates/apps/openedx/settings/partials/common_cms.py``, + + * which derives + ``tutor/templates/apps/openedx/settings/partials/common_all.py``; + + * and uses templates vars from Tutor configuration (``config.yml``), + + * and invokes hooks from any enable Tutor plugins; + + * it imports ``cms/envs/production.py``, + + * it imports ``cms/envs/common.py``, which sets production-inappropriate + defaults, + + * and which imports ``lms/envs/common.py``, which also sets + production-inappropriate defaults; + + * it sets more defaults, some of the edX.org-specific; + + * it loads ``/openedx/config/cms.yml``... + + * which is generated by Tutor from template + ``tutor/templates/apps/openedx/config/cms.env.yml`` + + * which derives + ``tutor/templates/apps/openedx/config/partials/auth.yml``; + + * it reverts some of ``/openedx/config/cms.yml`` with new "defaults"; + + * and it uses certain values ``/openedx/config/cms.yml`` to conditionally + override more settings and update certain dictionary settings, in a way + which is not documented. + +This is very difficult to reason about. Configuration complexity is frequently +cited as a chief area of pain for Open edX developers and operators. +Discussions in the Named Release Planning and Build-Test-Release Working Groups +frequently are encumbered with confusion and uncertainty of what the default +settings are in edx-platform, how they differ from Tutor's default settings, +what settings can be overriden, and how to do so. Only a minority of developers +and operators fully understand the configuration logic described above +end-to-end; even for those that do, following this override chain for any given +Django setting is time-consuming and error-prone. CAT-1 bugs and high-severity +security vulnerabilities have arisen due to misunderstanding of how +edx-platform Django settings are rendered. + +Developers are frequently instructed that if they need to override a Django +setting, the preferred way to do so is to "make a Tutor plugin". This is a +large amount of prior knowledge, boilerplate, and indirection, all required +to simply do something which Django provides out-of-the-box via a custom +``DJANGO_SETTINGS_MODULE``. + +Finally, it is worth nothing that all the complexity and toil exists alongside +other edx-platform configuration methods, such as Waffle, configuration models, +site configuration, XBlock configuration, and entry points. Those configuration +pathways are outside of the scope of this ADR, but are mentioned to demonstrate +the distressing level of complexity that developers and operators face when +working with the platform. + +Decision +******** + +This is our target edx-platform settings module structure: + +* ``openedx/envs/common.py``: Defaults shared between LMS and CMS (new!). + + * ``lms/envs/common.py``: LMS default settings. Wherever possible, + prod-ready values should be used; where not possible, obviously-incorrrect + values should be used. + + * ``lms/envs/test.py``: Override LMS settings for unit tests. Should work + in a local venv as well as in CI. + + * ``$THIRD_PARTY/lms/production.py``: Third-party providers (like edx.org) and + tools (like Tutor) will need to provide a custom setting settings file + derived from common.py in order to deploy LMS. + + * ``lms/envs/yaml.py``: (Possibly) An alternative to third-party + production.py. Loads overrides from a YAML file at ``LMS_CFG``, + plus some well-defined special handling for mergable values like + ``FEATURES``. This is adapted from and replaces lms/envs/production.py. + + * ``lms/envs/development.py``: Override LMS settings so that it can run + "bare metal" directly on a developer's local machine using debug-friendly + settings. Will use ``local.openedx.io`` (which resolves to 127.0.0.1) as + a base domain, which should be suitable for third-party tools as well. + + * ``$THIRD_PARTY/lms/dev.py``: Third-party tools (like Tutor, and 2U's + Devstack) will need to provide a custom settings file derived from + development.py in orer to serve a local LMS. + + * ``cms/envs/common.py`` + + * ``cms/envs/test.py`` + + * ``$THIRD_PARTY/cms/production.py`` + + * ``cms/envs/yaml.py`` (Possibly) + + * ``cms/envs/development.py`` + + * ``$THIRD_PARTY/cms/dev.py`` + + +Consequences +************ + +Moving to the target structure will take several steps. The steps are +non-breaking unless noted. + +* Introduce a dump_settings management command so that we can more easily + validate changes (or lack thereof) to the terminal edx-platform settings + modules. + +* BREAKING (minor, all settings modules): Improve edx-platform's API for + deriving settings, as we are about to depend on it significantly more than we + currently do. + +* Remove redundant overrides in (cms,lms)/envs/production.py. Use Derived + settings defaults to further simplify the module without changing its output. + +* Create openedx/envs/common.py, ensuring that toggle and setting annotations + are loaded from it. Move settings which are shared between + (cms,lms)/envs/common.py into openedx/envs/common.py. This may be iteratively + done across multiple PRs. + +* BREAKING (major, just common.py): Find the best production-ready defaults + between both (lms,cms)/envs/production.py and Tutor's production.pys, and + "bubble" them up to (openedx,cms,lms)/common.py. Keep + (lms,cms)/envs/production.py unchanged through this process. + +* Develop (cms,lms)/envs/development based off of (cms,lms)/envs/common.py. + Iterate until we can run "bare metal" development server for LMS and CMS + using these settings. + +* BREAKING (major): Deprecate and remove (cms,lms)/envs/devstack.py. + Tools (like Tutor and 2U's devstack) will either need to maintain local + copies of these modules, or "rebase" themselves onto + (lms,cms)/envs/development.py. + +* Propose and, if accepted, implement an update to OEP-45 (Configuring and + Operating Open edX). `Progress on this update is tracked here`_. + Based on community feedback, the update will be either to: + + 1. BREAKING (major): Revoke the OEP-45 sections regarding YAML. Deprecate and + remove (cms,lms)/envs/production.py. Tools and providers that use + these settings modules will either need to maintain local copies of these + modules, or "rebase" their internal settings modules onto + (cms,lms)/envs/common.py. Update operator documenation as needed. + + 2. BREAKING (major): Update OEP-45 to clarify that YAML configuration is + optional. Operators can opt out of YAML by deriving directly from + (cms,lms)/envs/common.py, or opt into YAML by using + (cms,lms)/envs/yaml.py. Document a simplified YAML schema in OEP-45. Issue + DEPR(s) explaining that (cms,lms)/envs/production.py is renamed to + (cms,lms)/envs/yaml.py, and that several breaking behavior changes are + happening in order to achieve the documented schema. + +* Create tickets to achieve a similar OEP-45-compliant settings structure in + any IDAs (independently-deployable applications) which exist in the openedx + GitHub organization, such as the Credentials service. + +.. _Progress on this update is tracked here: https://github.com/openedx/open-edx-proposals/issues/587 + +Alternatives Considered +*********************** + +One alternative settings structure +---------------------------------- + +Here is an alternate structure would de-dupe any shared LMS/CMS dev & test +logic by creating more shared modules within openedx/envs folder. Although +DRYer, this structure would increase the total number of edx-platform files and +potentially encourage more LMS-CMS coupling. So, will not pursue this +structure, but will keep it in mind as an alternative if we enounter +difficulties with the plan laid out in this ADR. + +* ``openedx/envs/common.py`` + + * ``lms/envs/prod.py`` + + * ``$THIRD_PARTY/lms/production.py`` + + * ``cms/envs/prod.py`` + + * ``$THIRD_PARTY/cms/production.py`` + + * ``openedx/envs/test.py`` + + * ``lms/envs/test.py`` + + * ``cms/envs/test.py`` + + * ``openedx/envs/dev.py`` + + * ``lms/envs/dev.py`` + + * ``$THIRD_PARTY/lms/dev.py`` + + * ``cms/envs/dev.py`` + + * ``$THIRD_PARTY/cms/dev.py`` From ba7703ef6d8c7762da7e35e409b2b34fcd819aaa Mon Sep 17 00:00:00 2001 From: "Kyle D. McCormick" Date: Wed, 12 Feb 2025 13:49:16 -0500 Subject: [PATCH 2/9] docs: Feanil's review comments --- .../0022-settings-simplification.rst | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/docs/decisions/0022-settings-simplification.rst b/docs/decisions/0022-settings-simplification.rst index 32409fb30bf..0739c471de0 100644 --- a/docs/decisions/0022-settings-simplification.rst +++ b/docs/decisions/0022-settings-simplification.rst @@ -47,7 +47,7 @@ handle settings that depend on other settings. Tutor does provide YAML files, but *it also has custom production and development settings files*! The result is that we have multiple layers of -indirection setting between edx-platform's common base settings, and the Django +indirection between edx-platform's common base settings, and the Django settings rendered into the actual community-supported Open edX distribution (Tutor). Specifically, production edx-platform configuration currently works like this: @@ -73,7 +73,7 @@ like this: * which sets production-inappropriate defaults; - * it sets more defaults, some of the edX.org-specific; + * it sets more defaults, some of them edX.org-specific; * it loads ``/openedx/config/lms.yml``... @@ -161,15 +161,20 @@ This is our target edx-platform settings module structure: * ``openedx/envs/common.py``: Defaults shared between LMS and CMS (new!). * ``lms/envs/common.py``: LMS default settings. Wherever possible, - prod-ready values should be used; where not possible, obviously-incorrrect - values should be used. + prod-ready values should be used. Where not possible, defaults should be + omitted (if the settings come from third-party libraries like Django) or be + set to obviously-wrong values (if edx-platform is defining them). For all + edx-platform-defined settings, code annotations should exist, either in + this file or in ``openedx/envs/common.py``. * ``lms/envs/test.py``: Override LMS settings for unit tests. Should work in a local venv as well as in CI. - * ``$THIRD_PARTY/lms/production.py``: Third-party providers (like edx.org) and - tools (like Tutor) will need to provide a custom setting settings file - derived from common.py in order to deploy LMS. + * ``/lms_production.py`` (example path): In order to + deploy the LMS, third-party providers (like edx.org) and tools (like + Tutor) will need to separately maintain their own custom settings module + derived from ``lms/envs/common.py``, and point their + ``DJANGO_SETTINGS_MODULE`` environment variable at this module. * ``lms/envs/yaml.py``: (Possibly) An alternative to third-party production.py. Loads overrides from a YAML file at ``LMS_CFG``, @@ -181,21 +186,23 @@ This is our target edx-platform settings module structure: settings. Will use ``local.openedx.io`` (which resolves to 127.0.0.1) as a base domain, which should be suitable for third-party tools as well. - * ``$THIRD_PARTY/lms/dev.py``: Third-party tools (like Tutor, and 2U's - Devstack) will need to provide a custom settings file derived from - development.py in orer to serve a local LMS. + * ``/lms_development.py`` (example path): In order to + run the LMS, third-party tools (like Tutor, and 2U's devstack) will + need to separately maintain their own custom settings module derived + from ``lms/envs/development.py``, and point their + ``DJANGO_SETTINGS_MODULE`` environment variable at this module. * ``cms/envs/common.py`` * ``cms/envs/test.py`` - * ``$THIRD_PARTY/cms/production.py`` + * ``/cms_production.py`` (example path) * ``cms/envs/yaml.py`` (Possibly) * ``cms/envs/development.py`` - * ``$THIRD_PARTY/cms/dev.py`` + * ``/cms_development.py`` (example path) Consequences From e32b5c8b4a1c367f1fc15461756aec45a82acd5f Mon Sep 17 00:00:00 2001 From: "Kyle D. McCormick" Date: Thu, 13 Feb 2025 13:19:01 -0500 Subject: [PATCH 3/9] docs: Clarify defaults and derived settings --- .../0022-settings-simplification.rst | 60 ++++++++++++------- 1 file changed, 40 insertions(+), 20 deletions(-) diff --git a/docs/decisions/0022-settings-simplification.rst b/docs/decisions/0022-settings-simplification.rst index 0739c471de0..abcd2ba19ce 100644 --- a/docs/decisions/0022-settings-simplification.rst +++ b/docs/decisions/0022-settings-simplification.rst @@ -158,52 +158,72 @@ Decision This is our target edx-platform settings module structure: -* ``openedx/envs/common.py``: Defaults shared between LMS and CMS (new!). - - * ``lms/envs/common.py``: LMS default settings. Wherever possible, - prod-ready values should be used. Where not possible, defaults should be - omitted (if the settings come from third-party libraries like Django) or be - set to obviously-wrong values (if edx-platform is defining them). For all - edx-platform-defined settings, code annotations should exist, either in - this file or in ``openedx/envs/common.py``. +* ``openedx/envs/common.py``: Define as much shared configuration between LMS + and CMS as possible, including: (a) where possible, annotated definitions of + edx-platform-specific settings with *reasonable, production-ready* defaults; + (b) otherwise, annotated definitions of edx-platform-specific settings (like + secrets) with *obviously-wrong* defaults, ensuring they aren't used in + production; and (c) un-annotated reasonable production-ready overrides of + third-party settings. When a particular setting's default should depend on + the *final* value of another setting, the former should be assigned to a + ``Derived(...)`` value, where ``...`` is a computation based on the latter. + + * ``lms/envs/common.py``: Extend ``openedx/envs/common.py`` to create, as + much as possible, a production-ready settings file for the LMS. These + extension may include: (a) annotated definitions of LMS-specific settings + with production-ready defaults; (b) annotated definitions of LMS-specific + settings with obviously-wrong defaults; (c) un-annotated LMS-specific + overrides of settings defined in ``openedx/envs/common.py``; and (d) + un-annotated overrides of third-party settings. Again, ``Derived`` settings + can be used as appropriate. * ``lms/envs/test.py``: Override LMS settings for unit tests. Should work - in a local venv as well as in CI. + in a local venv as well as in CI. Needs to invoke ``derive_settings`` in + order to render all previously-defined ``Derived`` settings. - * ``/lms_production.py`` (example path): In order to + * ``/lms_prod.py`` (example path): In order to deploy the LMS, third-party providers (like edx.org) and tools (like Tutor) will need to separately maintain their own custom settings module derived from ``lms/envs/common.py``, and point their - ``DJANGO_SETTINGS_MODULE`` environment variable at this module. + ``DJANGO_SETTINGS_MODULE`` environment variable at this module. It is + important that this module both (i) replaces the obviously-wrong settings + with appropriate production settings, and (ii) invokes + ``derive_settings`` to render all previously-defined ``Derived`` settings. * ``lms/envs/yaml.py``: (Possibly) An alternative to third-party production.py. Loads overrides from a YAML file at ``LMS_CFG``, plus some well-defined special handling for mergable values like ``FEATURES``. This is adapted from and replaces lms/envs/production.py. + It will invoke ``derive_settings``. - * ``lms/envs/development.py``: Override LMS settings so that it can run + * ``lms/envs/dev.py``: Override LMS settings so that it can run "bare metal" directly on a developer's local machine using debug-friendly settings. Will use ``local.openedx.io`` (which resolves to 127.0.0.1) as - a base domain, which should be suitable for third-party tools as well. + a base domain, which should be suitable for third-party tools as well. It + will invoke ``derive_settings``. - * ``/lms_development.py`` (example path): In order to + * ``/lms_dev.py`` (example path): In order to run the LMS, third-party tools (like Tutor, and 2U's devstack) will need to separately maintain their own custom settings module derived - from ``lms/envs/development.py``, and point their + from ``lms/envs/dev.py``, and point their ``DJANGO_SETTINGS_MODULE`` environment variable at this module. * ``cms/envs/common.py`` * ``cms/envs/test.py`` - * ``/cms_production.py`` (example path) + * ``/cms_prod.py`` (example path) * ``cms/envs/yaml.py`` (Possibly) - * ``cms/envs/development.py`` + * ``cms/envs/dev.py`` + + * ``/cms_dev.py`` (example path) - * ``/cms_development.py`` (example path) +Notes on defaults: +* When the default value of setting X depends on the *final* value of setting + Y, ``openedx.core Consequences ************ @@ -232,14 +252,14 @@ non-breaking unless noted. "bubble" them up to (openedx,cms,lms)/common.py. Keep (lms,cms)/envs/production.py unchanged through this process. -* Develop (cms,lms)/envs/development based off of (cms,lms)/envs/common.py. +* Develop (cms,lms)/envs/dev based off of (cms,lms)/envs/common.py. Iterate until we can run "bare metal" development server for LMS and CMS using these settings. * BREAKING (major): Deprecate and remove (cms,lms)/envs/devstack.py. Tools (like Tutor and 2U's devstack) will either need to maintain local copies of these modules, or "rebase" themselves onto - (lms,cms)/envs/development.py. + (lms,cms)/envs/dev.py. * Propose and, if accepted, implement an update to OEP-45 (Configuring and Operating Open edX). `Progress on this update is tracked here`_. From 108d3ff25db0580a2ffa57bd3372291d9667a919 Mon Sep 17 00:00:00 2001 From: "Kyle D. McCormick" Date: Thu, 13 Feb 2025 13:21:07 -0500 Subject: [PATCH 4/9] docs: Clarify default for ./manage.py --- docs/decisions/0022-settings-simplification.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/decisions/0022-settings-simplification.rst b/docs/decisions/0022-settings-simplification.rst index abcd2ba19ce..cd59e9e91eb 100644 --- a/docs/decisions/0022-settings-simplification.rst +++ b/docs/decisions/0022-settings-simplification.rst @@ -175,7 +175,9 @@ This is our target edx-platform settings module structure: settings with obviously-wrong defaults; (c) un-annotated LMS-specific overrides of settings defined in ``openedx/envs/common.py``; and (d) un-annotated overrides of third-party settings. Again, ``Derived`` settings - can be used as appropriate. + can be used as appropriate. This will be the default settings file for + running LMS management commands, although tools can override this (as + usual) by specifying a ``DJANGO_SETTINGS_MODULE``. * ``lms/envs/test.py``: Override LMS settings for unit tests. Should work in a local venv as well as in CI. Needs to invoke ``derive_settings`` in From f9432666c59fb2b163ec1877e80e92315902f032 Mon Sep 17 00:00:00 2001 From: "Kyle D. McCormick" Date: Thu, 20 Feb 2025 13:32:01 -0500 Subject: [PATCH 5/9] docs: Remove dangling draft paragraph --- docs/decisions/0022-settings-simplification.rst | 5 ----- 1 file changed, 5 deletions(-) diff --git a/docs/decisions/0022-settings-simplification.rst b/docs/decisions/0022-settings-simplification.rst index cd59e9e91eb..fc992df3695 100644 --- a/docs/decisions/0022-settings-simplification.rst +++ b/docs/decisions/0022-settings-simplification.rst @@ -222,11 +222,6 @@ This is our target edx-platform settings module structure: * ``/cms_dev.py`` (example path) -Notes on defaults: - -* When the default value of setting X depends on the *final* value of setting - Y, ``openedx.core - Consequences ************ From 321f3ac0ec4b35aec560d94bad06b1504d001455 Mon Sep 17 00:00:00 2001 From: "Kyle D. McCormick" Date: Thu, 20 Feb 2025 15:48:13 -0500 Subject: [PATCH 6/9] docs: Rework parts about YAML and OEP-45 for easier reading --- .../0022-settings-simplification.rst | 102 +++++++++++------- 1 file changed, 66 insertions(+), 36 deletions(-) diff --git a/docs/decisions/0022-settings-simplification.rst b/docs/decisions/0022-settings-simplification.rst index fc992df3695..97e59faff14 100644 --- a/docs/decisions/0022-settings-simplification.rst +++ b/docs/decisions/0022-settings-simplification.rst @@ -153,10 +153,34 @@ pathways are outside of the scope of this ADR, but are mentioned to demonstrate the distressing level of complexity that developers and operators face when working with the platform. -Decision -******** +Decision & Consequences +*********************** + +Overview +======== + +We orient edx-platform towards using standard Django settings configuration +patterns. Specifically, we will make it easy for operators to override settings +by supplying a custom ``DJANGO_SETTINGS_MODULE``. + +Moving towards this goals will need to be an iterative and careful process, +and it's likely that some aspects of the target structure or plan (described +below) will need to updated along the way. Nonetheless, once it becomes clear +that we are landing on a solid settings structure for edx-platform, we'll +propose an OEP-45 update to generalize the structure to all deployable Open edX +Django applications. + +Finally, based on what we learn throughout this process, our OEP-45 propsal +will either recommend to: + +1. Drop support for the ``_CFG`` YAML files, or + +2. Simplify the ``_CFG`` YAML schema, document it, and clarify that it + is an optional alternative to ``DJANGO_SETTINGS_MODULE`` rather than the + required/preferred configuration method. -This is our target edx-platform settings module structure: +Target settings structure for edx-platform +========================================== * ``openedx/envs/common.py``: Define as much shared configuration between LMS and CMS as possible, including: (a) where possible, annotated definitions of @@ -192,11 +216,12 @@ This is our target edx-platform settings module structure: with appropriate production settings, and (ii) invokes ``derive_settings`` to render all previously-defined ``Derived`` settings. - * ``lms/envs/yaml.py``: (Possibly) An alternative to third-party - production.py. Loads overrides from a YAML file at ``LMS_CFG``, - plus some well-defined special handling for mergable values like - ``FEATURES``. This is adapted from and replaces lms/envs/production.py. - It will invoke ``derive_settings``. + * ``lms/envs/yaml.py`` (only if we decide to retain YAML support): + An upstream-maintained alternative to + ``/lms_repo.py>``. Loads overrides from a YAML file at + ``LMS_CFG``, plus some well-defined special handling for mergable values + like ``FEATURES``. This is adapted from and replaces + lms/envs/production.py. It will invoke ``derive_settings``. * ``lms/envs/dev.py``: Override LMS settings so that it can run "bare metal" directly on a developer's local machine using debug-friendly @@ -216,25 +241,25 @@ This is our target edx-platform settings module structure: * ``/cms_prod.py`` (example path) - * ``cms/envs/yaml.py`` (Possibly) + * ``cms/envs/yaml.py`` (only if we decide to retain YAML support) * ``cms/envs/dev.py`` * ``/cms_dev.py`` (example path) -Consequences -************ +Plan of action +============== -Moving to the target structure will take several steps. The steps are -non-breaking unless noted. +These steps are non-breaking unless noted. * Introduce a dump_settings management command so that we can more easily validate changes (or lack thereof) to the terminal edx-platform settings modules. -* BREAKING (minor, all settings modules): Improve edx-platform's API for +* Improve edx-platform's API for deriving settings, as we are about to depend on it significantly more than we - currently do. + currently do. This is a potentially BREAKING CHANGE to any third-party + settings files which imported from ``openedx.core.lib.derived``. * Remove redundant overrides in (cms,lms)/envs/production.py. Use Derived settings defaults to further simplify the module without changing its output. @@ -244,37 +269,42 @@ non-breaking unless noted. (cms,lms)/envs/common.py into openedx/envs/common.py. This may be iteratively done across multiple PRs. -* BREAKING (major, just common.py): Find the best production-ready defaults - between both (lms,cms)/envs/production.py and Tutor's production.pys, and - "bubble" them up to (openedx,cms,lms)/common.py. Keep - (lms,cms)/envs/production.py unchanged through this process. +* Find the best production-ready defaults between both + (lms,cms)/envs/production.py and Tutor's production.pys, and "bubble" them up + to (openedx,cms,lms)/common.py. Keep (lms,cms)/envs/production.py unchanged + through this process. This is a BREAKING CHANGE for any operator that derives + from (lms,cms)/envs/common.py directly. Most operators derive from + (lms,cms)/envs/production.py, so we do not expect this to affect many sites, + if any. * Develop (cms,lms)/envs/dev based off of (cms,lms)/envs/common.py. Iterate until we can run "bare metal" development server for LMS and CMS using these settings. -* BREAKING (major): Deprecate and remove (cms,lms)/envs/devstack.py. - Tools (like Tutor and 2U's devstack) will either need to maintain local - copies of these modules, or "rebase" themselves onto - (lms,cms)/envs/dev.py. +* Deprecate and remove (cms,lms)/envs/devstack.py. This is a BREAKING CHANGE to + third-party development tools (like Tutor and 2U's devstack), as they will + now either need to maintain local copies of these modules, or "rebase" + themselves onto (lms,cms)/envs/dev.py. * Propose and, if accepted, implement an update to OEP-45 (Configuring and - Operating Open edX). `Progress on this update is tracked here`_. - Based on community feedback, the update will be either to: + Operating Open edX). `Progress on this update is tracked here`_. As mentioned + in the Decision section, this update will either: - 1. BREAKING (major): Revoke the OEP-45 sections regarding YAML. Deprecate and - remove (cms,lms)/envs/production.py. Tools and providers that use - these settings modules will either need to maintain local copies of these - modules, or "rebase" their internal settings modules onto - (cms,lms)/envs/common.py. Update operator documenation as needed. + 1. Revoke the OEP-45 sections regarding YAML. Deprecate and remove + (cms,lms)/envs/production.py. This is a BREAKING CHANGE for tools and + providers that use these settings modules, as they will either need to + maintain local copies of these modules, or "rebase" their internal + settings modules onto (cms,lms)/envs/common.py. Update operator + documenation as needed. - 2. BREAKING (major): Update OEP-45 to clarify that YAML configuration is + 2. Update OEP-45 to clarify that YAML configuration is optional. Operators can opt out of YAML by deriving directly from - (cms,lms)/envs/common.py, or opt into YAML by using - (cms,lms)/envs/yaml.py. Document a simplified YAML schema in OEP-45. Issue - DEPR(s) explaining that (cms,lms)/envs/production.py is renamed to - (cms,lms)/envs/yaml.py, and that several breaking behavior changes are - happening in order to achieve the documented schema. + (cms,lms)/envs/common.py, or they can opt into YAML by using + (cms,lms)/envs/yaml.py. Document a simplified YAML schema in OEP-45. + There will be several well-communicated BREAKING CHANGES in YAML behavior + in order to achieve the simplified schema. Furthermore, the rename of + (cms,lms)/envs/production.py to (cms,lms)/envs/yaml.py will be a BREAKING + CHANGE. * Create tickets to achieve a similar OEP-45-compliant settings structure in any IDAs (independently-deployable applications) which exist in the openedx From 8da96ecb46d530c81c29150b1b84742697bcbcfe Mon Sep 17 00:00:00 2001 From: "Kyle D. McCormick" Date: Thu, 20 Feb 2025 15:55:43 -0500 Subject: [PATCH 7/9] docs: Comment 3rd party settings but don't annotate them --- .../0022-settings-simplification.rst | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/docs/decisions/0022-settings-simplification.rst b/docs/decisions/0022-settings-simplification.rst index 97e59faff14..819efa0d06f 100644 --- a/docs/decisions/0022-settings-simplification.rst +++ b/docs/decisions/0022-settings-simplification.rst @@ -65,7 +65,7 @@ like this: * and uses templates vars from Tutor configuration (``config.yml``), - * and invokes hooks from any enable Tutor plugins; + * and invokes hooks from any enabled Tutor plugins; * it imports ``lms/envs/production.py``, @@ -187,21 +187,23 @@ Target settings structure for edx-platform edx-platform-specific settings with *reasonable, production-ready* defaults; (b) otherwise, annotated definitions of edx-platform-specific settings (like secrets) with *obviously-wrong* defaults, ensuring they aren't used in - production; and (c) un-annotated reasonable production-ready overrides of - third-party settings. When a particular setting's default should depend on - the *final* value of another setting, the former should be assigned to a + production; and (c) reasonable production-ready overrides of third-party + settings, ideally with explanatory comments (but not annotations). When a + particular setting's default should depend on the *final* value of another + setting, the former should be assigned to a ``Derived(...)`` value, where ``...`` is a computation based on the latter. * ``lms/envs/common.py``: Extend ``openedx/envs/common.py`` to create, as much as possible, a production-ready settings file for the LMS. These extension may include: (a) annotated definitions of LMS-specific settings with production-ready defaults; (b) annotated definitions of LMS-specific - settings with obviously-wrong defaults; (c) un-annotated LMS-specific - overrides of settings defined in ``openedx/envs/common.py``; and (d) - un-annotated overrides of third-party settings. Again, ``Derived`` settings - can be used as appropriate. This will be the default settings file for - running LMS management commands, although tools can override this (as - usual) by specifying a ``DJANGO_SETTINGS_MODULE``. + settings with obviously-wrong defaults; and (c) LMS-specific + overrides of settings defined in ``openedx/envs/common.py`` and of + third-party settings, ideally with explanatory comments (but not + annotations). Again, ``Derived`` settings can be used as appropriate. This + will be the default settings file for running LMS management commands, + although tools can override this (as usual) by specifying a + ``DJANGO_SETTINGS_MODULE``. * ``lms/envs/test.py``: Override LMS settings for unit tests. Should work in a local venv as well as in CI. Needs to invoke ``derive_settings`` in From 9e5c5da64ae619fdbfd40fc577127bae7b14987e Mon Sep 17 00:00:00 2001 From: "Kyle D. McCormick" Date: Thu, 20 Feb 2025 15:56:54 -0500 Subject: [PATCH 8/9] docs: fix headings --- docs/decisions/0022-settings-simplification.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/decisions/0022-settings-simplification.rst b/docs/decisions/0022-settings-simplification.rst index 819efa0d06f..87573e52024 100644 --- a/docs/decisions/0022-settings-simplification.rst +++ b/docs/decisions/0022-settings-simplification.rst @@ -318,7 +318,8 @@ Alternatives Considered *********************** One alternative settings structure ----------------------------------- +================================== + Here is an alternate structure would de-dupe any shared LMS/CMS dev & test logic by creating more shared modules within openedx/envs folder. Although From 8a21bf37addc8b88bf405fc2fc95bafc59ada4cf Mon Sep 17 00:00:00 2001 From: "Kyle D. McCormick" Date: Thu, 20 Feb 2025 16:04:28 -0500 Subject: [PATCH 9/9] docs: the rest of robrap's review --- .../0022-settings-simplification.rst | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/docs/decisions/0022-settings-simplification.rst b/docs/decisions/0022-settings-simplification.rst index 87573e52024..b19c3a9b83b 100644 --- a/docs/decisions/0022-settings-simplification.rst +++ b/docs/decisions/0022-settings-simplification.rst @@ -12,11 +12,11 @@ Context ******* OEP-45 declares that sites will configure each IDA's (indepently-deployable -application's) Django settings with an ``_CFG`` yaml file, parsed and -loaded by a single upstream-provided ``DJANGO_SETTINGS_MODULE``. this contrasts -with the django convention, which is that sites override Django settings using -a their own ``DJANGO_SETTINGS_MODULE``. The rationale is that all Open edX -customization can be reasonably specified in YAML; therefore, it is +application's) Django settings with an ``_CFG`` YAML file, parsed and +loaded by a single upstream-provided ``DJANGO_SETTINGS_MODULE``. This contrasts +with the Django convention, which is that sites override Django settings using +their own ``DJANGO_SETTINGS_MODULE``. The rationale was that all Open edX +setting customization can be reasonably specified in YAML; therefore, it is operationally safer to avoid using a custom ``DJANGO_SETTINGS_MODULE``, and it is operationally desirable for all operation modes to execute the same Python module for configuration. This was `briefly discussed in the oep-45 review @@ -38,7 +38,7 @@ For example, in theory, the upstream production LMS config might be named The upstream production CMS config would exist in parallel. However, as of Sumac, we do not know of any site other than edx.org that -successfully uses only YAML files for configuration. Furthermore, +successfully uses only YAML files for configuration. Furthermore, the upstream-provided ``DJANGO_SETTINGS_MODULE`` which loads these yaml files (``lms/envs/production.py``) is not simple: it declares defaults, imports from other Django settings modules, sets more defaults, handles dozens of special @@ -85,9 +85,9 @@ like this: * it reverts some of ``lms.yml`` with new "defaults"; - * and it uses certain values ``/openedx/config/lms.yml`` to conditionally - override more settings and update certain dictionary settings, in a way - which is not documented. + * and it uses certain values from ``/openedx/config/lms.yml`` to + conditionally override more settings and update certain dictionary + settings, in a way which is not documented. * ``cms/envs/tutor/production.py``... @@ -102,7 +102,7 @@ like this: * and uses templates vars from Tutor configuration (``config.yml``), - * and invokes hooks from any enable Tutor plugins; + * and invokes hooks from any enabled Tutor plugins; * it imports ``cms/envs/production.py``, @@ -124,9 +124,9 @@ like this: * it reverts some of ``/openedx/config/cms.yml`` with new "defaults"; - * and it uses certain values ``/openedx/config/cms.yml`` to conditionally - override more settings and update certain dictionary settings, in a way - which is not documented. + * and it uses certain values from ``/openedx/config/cms.yml`` to + conditionally override more settings and update certain dictionary + settings, in a way which is not documented. This is very difficult to reason about. Configuration complexity is frequently cited as a chief area of pain for Open edX developers and operators. @@ -173,9 +173,9 @@ Django applications. Finally, based on what we learn throughout this process, our OEP-45 propsal will either recommend to: -1. Drop support for the ``_CFG`` YAML files, or +1. Drop support for the ``_CFG`` YAML files, or -2. Simplify the ``_CFG`` YAML schema, document it, and clarify that it +2. Simplify the ``_CFG`` YAML schema, document it, and clarify that it is an optional alternative to ``DJANGO_SETTINGS_MODULE`` rather than the required/preferred configuration method. @@ -266,10 +266,10 @@ These steps are non-breaking unless noted. * Remove redundant overrides in (cms,lms)/envs/production.py. Use Derived settings defaults to further simplify the module without changing its output. -* Create openedx/envs/common.py, ensuring that toggle and setting annotations - are loaded from it. Move settings which are shared between - (cms,lms)/envs/common.py into openedx/envs/common.py. This may be iteratively - done across multiple PRs. +* Create openedx/envs/common.py, ensuring that any annotations defined in it + are included in the edx-platform reference docs build. Move settings which + are shared between (cms,lms)/envs/common.py into openedx/envs/common.py. This + may be iteratively done across multiple PRs. * Find the best production-ready defaults between both (lms,cms)/envs/production.py and Tutor's production.pys, and "bubble" them up @@ -321,10 +321,10 @@ One alternative settings structure ================================== -Here is an alternate structure would de-dupe any shared LMS/CMS dev & test +Here is an alternate structure that would de-dupe any shared LMS/CMS dev & test logic by creating more shared modules within openedx/envs folder. Although DRYer, this structure would increase the total number of edx-platform files and -potentially encourage more LMS-CMS coupling. So, will not pursue this +potentially encourage more LMS-CMS coupling. So, we will not pursue this structure, but will keep it in mind as an alternative if we enounter difficulties with the plan laid out in this ADR.