-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat!: A Better API for Derived Settings #36192
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.
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
feat!: A Better API for Derived Settings #36192
Conversation
7b56894 to
11dbd37
Compare
|
@feanil , this is ready for review. @dianakhuang , I have confirmed this PR using the redacted LMS yaml file. If you're able to provide the redacted CMS yaml, I can confirm with that as well. |
|
@kdmccormick working on a redacted studio file right now, so I'll try to get you one by the end of today. |
|
Tyty @dianakhuang -- I will keep this open until I hear from you. |
feanil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, much easier to manage.
|
Confirmed with CMS settings. Will rebase and merge. |
11dbd37 to
2426dc7
Compare
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
1 similar comment
|
2U Release Notice: This PR has been deployed to the edX production environment. |
A tiny update to the directions in devstack.py to enable theming based on recent changes after #36192 was merged.
Updates theming config example in our docs after recent changes to edx-platform (see openedx/openedx-platform#36192).
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
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
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
Description
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_settingsto properly derive settings declared using the new API.BREAKING CHANGE: The
derivedandderived_collection_entryfunction arereplaced with the
Derivedclass. We do not expect those functions to havebeen 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_settingsfunction, which we DO know to be used by some external edx-platform plugins.
Before
After
Supporting info
Part of:
Builds on a previous settings refactoring:
Next up:
derive_settingsto lazy load settings. #36205Testing instructions
First, download prod-like YML files to ensure that this works in a non-Tutor environment...
Then, download and save this to
diff_settings.shin the root of edx-platform:Finally, from within a Tutor LMS or CMS shell, run
./diff_settings.sh master kdmccormick/use-derived-toolingResult should be
"No difference!"