Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify edx-platform settings #36215

Open
10 of 17 tasks
kdmccormick opened this issue Feb 4, 2025 · 4 comments
Open
10 of 17 tasks

Simplify edx-platform settings #36215

kdmccormick opened this issue Feb 4, 2025 · 4 comments
Assignees
@feanil @kdmccormick
Labels
code health Proactive technical investment via refactorings, removals, etc. documentation Relates to documentation improvements epic Large unit of work, consisting of multiple tasks

Comments

@kdmccormick
Copy link
Member

kdmccormick commented Feb 4, 2025 •
edited
Loading

Intended final settings structure

See https://github.com/openedx/edx-platform/blob/master/docs/decisions/0022-settings-simplification.rst#target-settings-structure-for-edx-platform

Tasks
Preview Give feedback
  1. refactor: Clean up lms/envs/production.py cruft #36115
  2. [Attempt 2] refactor: Clean up lms/envs/production.py cruft #36131
    create-sandbox
  3. feat: dump_settings management command #36162
  4. feat: Make dump for fns, classes more stable and helpful #36188
  5. refactor: Use derived_settings to lazy load settings. #36124
  6. [old] refactor: A Better API for Derived Settings #36132
  7. feat!: A Better API for Derived Settings #36192
  8. refactor: Use derive_settings to lazy load settings. #36205
  9. docs: Settings Simplification ADR #36224
  10. build: Start watching the production.py files. #36225
  12. refactor: Clean up CMS production settings cruft #36306
    create-sandbox

Related / nice to have

  • De-crazy the logging settings overrides.
  • De-crazy the MODULESTORE settings conversions.
  • Collapse FEATURES dict
  • Warn/fail when "required" common settings, e.g. secrets, aren't overridden.

Open questions

  • When exactly do we "rebase" Tutor's settings onto (lms,cms)/envs/(common,development)? Most likely, we should do it all at once when those four files are ready, as Tutor's settings are all coupled together into a Jinja hierarchy.
  • Should the settings object should have same type signature between LMS and CMS?

Potential Follow-on Work
@kdmccormick kdmccormick mentioned this issue Feb 4, 2025
Open
@kdmccormick
Copy link
Member Author

@robrap , @dianakhuang -- I have split this off from the greater Let's Talk About OEP-45 issue. We'll be tracking the ongoing edx-platform settings simplification here.

@kdmccormick kdmccormick added epic Large unit of work, consisting of multiple tasks code health Proactive technical investment via refactorings, removals, etc. documentation Relates to documentation improvements labels Feb 4, 2025
@kdmccormick kdmccormick mentioned this issue Feb 4, 2025
Merged
kdmccormick added a commit that referenced this issue Feb 4, 2025
@kdmccormick
feat!: A Better API for Derived Settings (#36192)
3227566 
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
@kdmccormick kdmccormick mentioned this issue Feb 4, 2025
Merged
kdmccormick added a commit that referenced this issue Feb 5, 2025
@kdmccormick
refactor: Use derive_settings to lazy load settings. (#36205)
e1a8b52 
Some of our settings depend on the values of other settings.  Rather
than explicitly looking up each one in the YAML settings file, we can
simply derive them based on the setting in the YAML file after all the
YAML settings have been loaded.

Part of: #36215
Co-Authored-By: Feanil Patel <feanil@axim.org>
@kdmccormick kdmccormick mentioned this issue Feb 6, 2025
Open
@kdmccormick
Copy link
Member Author

Ready for review: #36224

@kdmccormick kdmccormick mentioned this issue Feb 14, 2025
Closed
@kdmccormick kdmccormick marked this as a duplicate of #34446 Feb 14, 2025
This was referenced Feb 14, 2025
Closed
Open
jciasenza pushed a commit to jciasenza/edx-platform that referenced this issue Feb 25, 2025
@kdmccormick @jciasenza
feat!: A Better API for Derived Settings (openedx#36192)
d0b817f 
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: openedx#36215
jciasenza pushed a commit to jciasenza/edx-platform that referenced this issue Feb 25, 2025
@kdmccormick @jciasenza
refactor: Use derive_settings to lazy load settings. (openedx#36205)
4197272 
Some of our settings depend on the values of other settings.  Rather
than explicitly looking up each one in the YAML settings file, we can
simply derive them based on the setting in the YAML file after all the
YAML settings have been loaded.

Part of: openedx#36215
Co-Authored-By: Feanil Patel <feanil@axim.org>
@blarghmatey
Copy link
Contributor

I'm not sure if it's too late in the game, but an approach that I am planning to take for managing the settings in my edX deployments is to develop a plugin that takes advantage of Pydantic models and either includes or vendors the logic from pydjantic. That would allow for easily managing the override of any settings via any of environment variables, YAML fragments, JSON, etc. If that could be the approach taken in the Open edX projects directly then it would save me the effort :)

kdmccormick added a commit that referenced this issue Feb 27, 2025
@kdmccormick
docs: Settings Simplification ADR (#36224)
2dc4dfe 
Part of: #36215

Follow up in: openedx/open-edx-proposals#684
@feanil
Copy link
Contributor

feanil commented Feb 28, 2025

@blarghmatey do you have any examples for this? I'm curious about how this would work.

@kdmccormick kdmccormick mentioned this issue Feb 28, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@feanil feanil

@kdmccormick kdmccormick

Labels
code health Proactive technical investment via refactorings, removals, etc. documentation Relates to documentation improvements epic Large unit of work, consisting of multiple tasks
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@blarghmatey @feanil @kdmccormick