-
Notifications
You must be signed in to change notification settings - Fork 4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
3003984
commit 57f550a
Showing
1 changed file
with
298 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 ``<APPNAME>_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 | ||
<https://github.com/openedx/open-edx-proposals/pull/143#discussion_r411180111>`_. | ||
|
||
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`` |