Skip to content

Conversation

@wgu-ram-chandra
Copy link
Contributor

@wgu-ram-chandra wgu-ram-chandra commented Jul 24, 2025

See also: [DEPR]: Add FEATURES = FeaturesProxy(...) to your django settings

Description

This PR begins the process of flattening the FEATURES dictionary by moving all boolean feature flags from the nested FEATURES dictionary directly into the settings namespace, while maintaining full backward compatibility through a proxy.

Problem Overview

Both LMS and CMS settings contain a large FEATURES dictionary with boolean values that adds unnecessary complexity:

  • Makes individual feature flags harder to override in tests and settings
  • Creates inconsistent patterns (some booleans in FEATURES, others as direct settings)
  • Adds extra dictionary lookup overhead
  • No technical reason for these booleans to be grouped in a dictionary

Solution

  • Flatten FEATURES keys directly into the settings namespace (like, FEATURES['ENABLE_COURSE_DISCOVERY'] -> ENABLE_COURSE_DISCOVERY)
  • Create FeaturesProxy that maintains the existing FEATURES dictionary API for backward compatibility
  • Preserve all existing functionality while improving settings architecture

Implementation Details

  • All FEATURES boolean flags are now available as direct settings attributes
  • FeaturesProxy provides seamless read/write access to maintain existing settings.FEATURES['KEY'] usage
  • Applied to both LMS and CMS settings
  • Zero breaking changes for external plugins or existing code

Testing

  • All existing tests pass with proxy in place
  • Fixed test isolation issue in xmodule/tests/test_word_cloud.py where patch was conflicting with proxy
Initial State
Screenshot 2025-07-24 at 12 15 55 PM
State after a new value is `set` State after a value is `updated`
Screenshot 2025-07-24 at 12 16 40 PM Screenshot 2025-07-24 at 12 16 51 PM

Backward Compatibility

Backward compatible - existing code using settings.FEATURES['KEY'] continues to work unchanged.

Multi-PR approach

This is Phase 1 of a multi-PR approach:

  1. This PR: Flatten FEATURES + create proxy
  2. Future PRs: Update references by module (LMS, CMS, xmodule, etc.)
  3. Final PR: Remove proxy once all references are updated
image

Below is the Reference Update Script: FEATURES → Direct Settings

Script (click to expand)
import os
import re


# settings.FEATURES.get("KEY", value)
pattern1 = r'settings\.FEATURES\.get\(\s*(?P<q>["\'])(?P<key>\w+)(?P=q)\s*,\s*(?P<value>[^)]+)\s*\)'

# settings.FEATURES.get('ENABLE_SERVICE_STATUS')
pattern1_1 = r'settings\.FEATURES\.get\(\s*(?P<q>["\'])(?P<key>\w+)(?P=q)\s*\)'

# settings.FEATURES['KEY']
pattern2 = r'settings\.FEATURES\[\s*(?P<q>["\'])(?P<key>\w+)(?P=q)\s*\]'

# FEATURES["KEY"] = value
pattern3 = r'FEATURES\[\s*(?P<q>["\'])(?P<key>\w+)(?P=q)\s*\]\s*=\s*(?P<value>.+)'

# FEATURES["KEY"]
pattern3_3 = r'FEATURES\[\s*(?P<q>["\'])(?P<key>\w+)(?P=q)\s*\]'

# with override_settings(FEATURES={'KEY': value})
pattern4 = r'override_settings\(\s*FEATURES\s*=\s*{(?P<features>[^}]+)}\s*\)'

# mock.patch.dict("django.conf.settings.FEATURES", {'KEY': value})
pattern5 = r'mock\.patch\.dict\(\s*(?P<q>["\'])django\.conf\.settings\.FEATURES(?P=q)\s*,\s*{(?P<features>[^}]+)}\s*\)'

# patch.dict(settings.FEATURES, {'KEY': value})
pattern6 = r'patch\.dict\(\s*settings\.FEATURES\s*,\s*{(?P<features>[^}]+)}\s*\)'

# patch.dict("django.conf.settings.FEATURES", {'KEY': value})
pattern7 = r'patch\.dict\(\s*(?P<q>["\'])django\.conf\.settings\.FEATURES(?P=q)\s*,\s*{(?P<features>[^}]+)}\s*\)'

# with patch.dict(settings.FEATURES, key=value):
pattern8 = r'with\s+patch\.dict\(\s*settings\.FEATURES\s*,\s*(?P<key>\w+)\s*=\s*(?P<value>[^)]+)\s*\)'


total_files_changed = 0

def handle_feature_line(line):
    global total_files_changed

    original_line = line

    # Pattern 1: settings.FEATURES.get(...)
    match = re.search(pattern1, line)
    if match:
        data = match.groupdict()
        replacement = f'getattr(settings, "{data["key"]}", {data["value"]})'
        line = re.sub(pattern1, replacement, line)
        print(f"[pattern1] -> {replacement}")

    # Pattern 1_1: settings.FEATURES.get(...)
    match = re.search(pattern1_1, line)
    if match:
        data = match.groupdict()
        replacement = f'getattr(settings, "{data["key"]}")'
        line = re.sub(pattern1_1, replacement, line)
        print(f"[pattern1_1] -> {replacement}")

    # Pattern 2: settings.FEATURES['KEY']
    match = re.search(pattern2, line)
    if match:
        data = match.groupdict()
        replacement = f'settings.{data["key"]}'
        line = re.sub(pattern2, replacement, line)
        print(f"[pattern2] -> {replacement}")

    # Pattern 3: FEATURES['KEY'] = value
    match = re.search(pattern3, line)
    if match:
        data = match.groupdict()
        replacement = f'{data["key"]} = {data["value"]}'
        line = re.sub(pattern3, replacement, line)
        print(f"[pattern3] -> {replacement}")

    # Pattern 3_3: FEATURES['KEY']
    match = re.search(pattern3_3, line)
    if match:
        data = match.groupdict()
        replacement = f'settings.{data["key"]}'
        line = re.sub(pattern3_3, replacement, line)
        print(f"[pattern3_3] -> {replacement}")

    # Pattern 4: with override_settings(FEATURES={...})
    match = re.search(pattern4, line)
    if match:
        features = match.group("features").strip()
        try:
            key, value = map(str.strip, features.split(":", 1))
            key = key.strip('"\'')
            replacement = f"with override_settings({key}={value})"
            line = re.sub(pattern4, replacement, line)
            print(f"[pattern4] -> {replacement}")
        except ValueError:
            print("[pattern4] -> Invalid format, skipping")

    # Pattern 5: mock.patch.dict("django.conf.settings.FEATURES", {...})
    match = re.search(pattern5, line)
    if match:
        features = match.group("features").strip()
        try:
            key, value = map(str.strip, features.split(":", 1))
            key = key.strip('"\'')
            replacement = f"@mock.patch.object(settings, '{key}', {value})"
            line = re.sub(pattern5, replacement, line)
            print(f"[pattern5] -> {replacement}")
        except ValueError:
            print("[pattern5] -> Invalid format, skipping")

    # Pattern 6: patch.dict(settings.FEATURES, {...})
    match = re.search(pattern6, line)
    if match:
        features = match.group("features").strip()
        try:
            key, value = map(str.strip, features.split(":", 1))
            key = key.strip('"\'')
            replacement = f"@patch.object(settings, '{key}', {value})"
            line = re.sub(pattern6, replacement, line)
            print(f"[pattern6] -> {replacement}")
        except ValueError:
            print("[pattern6] -> Invalid format, skipping")

    # Pattern 7: patch.dict("django.conf.settings.FEATURES", {...})
    match = re.search(pattern7, line)
    if match:
        features = match.group("features").strip()
        try:
            key, value = map(str.strip, features.split(":", 1))
            key = key.strip('"\'')
            replacement = f"@mock.patch.object(settings, '{key}', {value})"
            line = re.sub(pattern7, replacement, line)
            print(f"[pattern7] -> {replacement}")
        except ValueError:
            print("[pattern7] -> Invalid format, skipping")
    
    # Pattern 8: with patch.dict(settings.FEATURES, KEY=value)
    match = re.search(pattern8, line)
    if match:
        key = match.group("key")
        value = match.group("value").strip()
        replacement = f"with patch.object(settings, '{key}', {value}):"
        line = re.sub(pattern8, replacement, line)
        print(f"[pattern8] -> {replacement}")


    if line != original_line:
        total_files_changed += 1
        print(f"Rewritten:\n  BEFORE: {original_line.strip()}\n  AFTER:  {line.strip()}")

    return line


def _run(file):
    ignored_files = []
    new_lines = []

    with open(file, 'r') as fr:
        lines = fr.readlines()
        for line in lines:
            if "FEATURES" in line: # and not (line.startswith("#") or line.startswith("##")):
                print(f"FEATURE: {line}")
                new_line = handle_feature_line(line)
            else:
                new_line = line

            new_lines.append(new_line)
    
    with open(file, 'w') as fw:
        for line in new_lines:
            fw.write(line)
        


if __name__ == "__main__":
    for root, dirs, files in os.walk("./lms/envs"):
        for file in files:
            full_path = f'{root}/{file}'
            if full_path.endswith(".py") and full_path.find('__init__') == -1 and full_path.find('/tutor/') == -1:
                print(f'Current File: {root}/{file}')
                _run(full_path)

    print(f"Total Files changed: {total_files_changed}")


""" Formats
settings.FEATURES['MODE_CREATION_FOR_TESTING']
settings.FEATURES.get("ENABLE_CREDIT_ELIGIBILITY", False)
settings.FEATURES.get(
        'ENABLE_CERTIFICATES_IDV_REQUIREMENT')
FEATURES['PREVENT_CONCURRENT_LOGINS'] = False

with override_settings(FEATURES={'DISABLE_ADVANCED_SETTINGS': disable_advanced_settings}):
with override_settings(FEATURES=PROCTORED_EXAMS_ENABLED_FEATURES):

@patch.dict("django.conf.settings.FEATURES", {"DISABLE_ADVANCED_SETTINGS": True})
@patch.dict(settings.FEATURES, {'ENABLE_EDXNOTES': True})
@override_settings(FEATURES={'ENABLE_CREATOR_GROUP': True})
@mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True})
@unittest.skipUnless(settings.FEATURES.get('ENTRANCE_EXAMS', False), True)
@mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_PUBLISHER': False})
"""

Fixes: #36890
CC: @kdmccormick

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jul 24, 2025
@openedx-webhooks
Copy link

openedx-webhooks commented Jul 24, 2025

Thanks for the pull request, @wgu-ram-chandra!

This repository is currently maintained by @openedx/wg-maintenance-edx-platform.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details
Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

@github-project-automation github-project-automation bot moved this to Needs Triage in Contributions Jul 24, 2025
@wgu-ram-chandra wgu-ram-chandra force-pushed the collapse-features-dictionary branch 5 times, most recently from aeb1504 to f2ac8a1 Compare July 25, 2025 15:35
@kdmccormick kdmccormick self-requested a review July 25, 2025 15:53
@wgu-ram-chandra wgu-ram-chandra added create-sandbox open-craft-grove should create a sandbox environment from this PR mao-onboarding Reviewing this will help onboard devs from an Axim mission-aligned organization (MAO). labels Jul 25, 2025
@open-craft-grove
Copy link

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@open-craft-grove
Copy link

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@mphilbrick211 mphilbrick211 moved this from Needs Triage to Ready for Review in Contributions Jul 28, 2025
@mphilbrick211 mphilbrick211 moved this from Ready for Review to Waiting on Author in Contributions Jul 28, 2025
Copy link
Member

@kdmccormick kdmccormick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great overall approach and great work so far 👍🏻 Just some comments on the implementation of the FEATURES proxy.

@open-craft-grove
Copy link

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@wgu-ram-chandra wgu-ram-chandra force-pushed the collapse-features-dictionary branch from d034f51 to d689dec Compare July 28, 2025 22:05
@open-craft-grove
Copy link

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@wgu-ram-chandra wgu-ram-chandra force-pushed the collapse-features-dictionary branch 2 times, most recently from e915471 to 03204fd Compare July 29, 2025 00:32
@open-craft-grove
Copy link

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@open-craft-grove
Copy link

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@wgu-ram-chandra wgu-ram-chandra force-pushed the collapse-features-dictionary branch from 9d9a03e to 20b41a0 Compare July 29, 2025 03:55
@open-craft-grove
Copy link

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@open-craft-grove
Copy link

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@wgu-ram-chandra wgu-ram-chandra force-pushed the collapse-features-dictionary branch 2 times, most recently from dd6c5f8 to 067150b Compare September 9, 2025 18:03
@wgu-taylor-payne wgu-taylor-payne self-requested a review September 9, 2025 18:25
Copy link
Contributor

@wgu-taylor-payne wgu-taylor-payne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for making those changes. I think this looks good to go, but we need to coordinate an updated DEPR and also changes to Tutor.

Here are the FEATURE settings that Tutor and tutor-mfe reference:

|----------------------------------+--------------------|
| FEATURE setting                  | Tutor source       |
|----------------------------------+--------------------|
| ENABLE_CORS_HEADERS              | common_all.py      |
| ENABLE_COURSEWARE_INDEX          | common_cms.py      |
| ENABLE_COURSEWARE_MICROFRONTEND  | lms/development.py |
| ENABLE_COURSEWARE_SEARCH         | common_lms.py      |
| ENABLE_COURSE_DISCOVERY          | common_lms.py      |
| ENABLE_DASHBOARD_SEARCH          | common_lms.py      |
| ENABLE_DISCUSSION_SERVICE        | common_all.py      |
| ENABLE_ENTERPRISE_INTEGRATION    | lms/development.py |
| PREVENT_CONCURRENT_LOGINS        | common_all.py      |
| ENABLE_AUTHN_MICROFRONTEND       | tutor-mfe          |
| ENABLE_NEW_BULK_EMAIL_EXPERIENCE | tutor-mfe          |
|----------------------------------+--------------------|

I think one good approach to resolve this is to add something like this to the top of common_all.py:

try:
    from openedx.core.lib.features_setting_proxy import FeaturesProxy
    FEATURES = FeaturesProxy(globals())
except ImportError:
    pass

This would provide backwards compatibility with the FEATURES dictionary, as we transition off of it. @wgu-ram-chandra, do you want to create a PR for this on the Tutor main branch?

The instructions in the DEPR, which was created when we were trying the read-only approach, will not work with the new updates. We need to request that operators add something like the snippet above to any custom settings modules that write to FEATURES. How do you recommend we go about this @kdmccormick?

@open-craft-grove
Copy link

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@wgu-ram-chandra wgu-ram-chandra force-pushed the collapse-features-dictionary branch from 067150b to 906e676 Compare September 9, 2025 19:10
@open-craft-grove
Copy link

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@wgu-ram-chandra wgu-ram-chandra force-pushed the collapse-features-dictionary branch from 906e676 to 790a67a Compare September 9, 2025 20:11
@open-craft-grove
Copy link

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@open-craft-grove
Copy link

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@open-craft-grove
Copy link

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@kdmccormick
Copy link
Member

kdmccormick commented Sep 10, 2025

The instructions in the #37226, which was created when we were trying the read-only approach, will not work with the new updates. We need to request that operators add something like the snippet above to any custom settings modules that write to FEATURES. How do you recommend we go about this @kdmccormick?

I recommend closing the current DEPR, and opening a new one. Use the fast-track template, as we do not need a comment period for this.

You do not need to make the Tutor updates yourself. Instead, I recommend sharing the new DEPR in the #tutor-maintenance channel. The maintainers there are quite responsive and should be able to make the necessary Tutor updates shortly.

(we're trying to maintain a degree of separation between Tutor and Open edX projects; we want to establish that Open edX devs are responsible for communicating breaking changes clearly, but not responsible for updating downstream projects like Tutor)

@kdmccormick kdmccormick self-requested a review September 10, 2025 17:15
Copy link
Member

@kdmccormick kdmccormick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks great!

I'll merge once that DEPR is posted and the "breaking changes" date has passed. I think 1 week is appropriate, since the change is so small.

@open-craft-grove
Copy link

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@open-craft-grove
Copy link

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@kdmccormick
Copy link
Member

Related DEPR: #37345

@feanil
Copy link
Contributor

feanil commented Sep 18, 2025

Breaking changes are now unblocked so I'll be merging this now.

@feanil feanil merged commit f120a7c into openedx:master Sep 18, 2025
49 checks passed
@github-project-automation github-project-automation bot moved this from In Eng Review to Done in Contributions Sep 18, 2025
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

create-sandbox open-craft-grove should create a sandbox environment from this PR mao-onboarding Reviewing this will help onboard devs from an Axim mission-aligned organization (MAO). open-source-contribution PR author is not from Axim or 2U

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Collapse the FEATURES dictionary into boolean settings

8 participants