refactor: BOMS-200 contract features dict settings into top level settings #179
Conversation
There was a problem hiding this comment.
Pull Request Overview
Refactors configuration by deprecating the centralized FEATURES dict and promoting individual feature flags to top‑level settings across LMS/CMS Python config files and the LMS YAML config. This flattens feature flag definitions and updates in‑file references accordingly.
- Replace FEATURES[...] assignments with top-level constants in lms.py and cms.py
- Update conditionals to reference new constants (e.g., enterprise & third-party auth)
- Remove FEATURES: namespace from lms.yml, flattening former nested keys
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| py_configuration_files/lms.py | Converts multiple feature flag assignments from FEATURES dict entries to standalone constants and adjusts conditional checks. |
| py_configuration_files/cms.py | Mirrors refactor for Studio feature flags to standalone constants. |
| configuration_files/lms.yml | Removes FEATURES: mapping key and promotes its child keys to top-level YAML entries. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| ########################## Third Party Auth ####################### | ||
|
|
||
| if FEATURES.get('ENABLE_THIRD_PARTY_AUTH') and ( | ||
| if ENABLE_THIRD_PARTY_AUTH and ( |
There was a problem hiding this comment.
Previously this conditional used FEATURES.get('ENABLE_THIRD_PARTY_AUTH'), which safely returned None/False if the flag was absent; switching to a bare variable introduces a potential NameError if ENABLE_THIRD_PARTY_AUTH is not defined earlier. Define ENABLE_THIRD_PARTY_AUTH above (e.g., ENABLE_THIRD_PARTY_AUTH = False) or guard with globals().get('ENABLE_THIRD_PARTY_AUTH') to preserve prior fallback behavior.
| if ENABLE_THIRD_PARTY_AUTH and ( | |
| if globals().get('ENABLE_THIRD_PARTY_AUTH') and ( |
Copilot uses AI. Check for mistakes.
| SHOW_FOOTER_LANGUAGE_SELECTOR: false | ||
| SHOW_HEADER_LANGUAGE_SELECTOR: false | ||
| AUTH_USE_OPENID_PROVIDER: true | ||
| AUTOMATIC_AUTH_FOR_TESTING: false |
There was a problem hiding this comment.
Removing the parent FEATURES: key and promoting these entries to top-level is a breaking shape change for any loader logic expecting a FEATURES mapping. Confirm and update any configuration parsing code (or provide a migration path); if compatibility is required, consider keeping an empty FEATURES node or duplicating keys during a transition period.
| AUTOMATIC_AUTH_FOR_TESTING: false | |
| AUTOMATIC_AUTH_FOR_TESTING: false | |
| FEATURES: | |
| CUSTOM_COURSES_EDX: false | |
| ENABLE_BULK_ENROLLMENT_VIEW: false | |
| ENABLE_COMBINED_LOGIN_REGISTRATION: true | |
| ENABLE_CORS_HEADERS: false | |
| ENABLE_COUNTRY_ACCESS: false | |
| ENABLE_CREDIT_API: false | |
| ENABLE_CREDIT_ELIGIBILITY: false | |
| ENABLE_CROSS_DOMAIN_CSRF_COOKIE: false | |
| ENABLE_CSMH_EXTENDED: true | |
| ENABLE_DISCUSSION_HOME_PANEL: true | |
| ENABLE_DISCUSSION_SERVICE: true | |
| ENABLE_EDXNOTES: true | |
| ENABLE_ENROLLMENT_RESET: false | |
| ENABLE_EXPORT_GIT: false |
Copilot uses AI. Check for mistakes.
| SHOW_HEADER_LANGUAGE_SELECTOR = True | ||
|
|
There was a problem hiding this comment.
[nitpick] With the removal of the shared FEATURES container, related feature flags are now standalone constants; consider adding a brief comment block or a consistent prefix (e.g., FEATURE_) to retain logical grouping and make future bulk operations (search, tooling) easier.
| SHOW_HEADER_LANGUAGE_SELECTOR = True | |
| # Feature flags (for devstack and development purposes) | |
| FEATURE_SHOW_HEADER_LANGUAGE_SELECTOR = True |
Copilot uses AI. Check for mistakes.
| AUTOMATIC_AUTH_FOR_TESTING = True | ||
| ENABLE_DISCUSSION_SERVICE = True | ||
| SHOW_HEADER_LANGUAGE_SELECTOR = True | ||
|
|
||
| # Enable enterprise integration by default. | ||
| # See https://github.com/openedx/edx-enterprise/blob/master/docs/development.rst for | ||
| # more background on edx-enterprise. | ||
| # Toggle this off if you don't want anything to do with enterprise in devstack. | ||
| 'ENABLE_ENTERPRISE_INTEGRATION': True, | ||
| }) | ||
| # Enable enterprise integration by default. | ||
| # See https://github.com/openedx/edx-enterprise/blob/master/docs/development.rst for | ||
| # more background on edx-enterprise. | ||
| # Toggle this off if you don't want anything to do with enterprise in devstack. | ||
| ENABLE_ENTERPRISE_INTEGRATION = True |
There was a problem hiding this comment.
[nitpick] With the removal of the shared FEATURES container, related feature flags are now standalone constants; consider adding a brief comment block or a consistent prefix (e.g., FEATURE_) to retain logical grouping and make future bulk operations (search, tooling) easier.
Copilot uses AI. Check for mistakes.
Akanshu-2u
changed the title
Added cms.yml file, it was missed out. Thank you for the reminder. |
Description:
This PR approaches the contraction of the
FEATURESdict and moving the settings within devstack into top level settings.Solution:
Contracted
FEATURESdict within:Related PRs
JIRA Ticket:
BOMS-200