Skip to content

fix: enable the enterprise feature BEFORE plugins are loaded#220

Merged
pwnage101 merged 1 commit into
masterfrom
pwnage101/enable-enterprise-before-plugins
May 14, 2026
Merged

fix: enable the enterprise feature BEFORE plugins are loaded#220
pwnage101 merged 1 commit into
masterfrom
pwnage101/enable-enterprise-before-plugins

Conversation

@pwnage101
Copy link
Copy Markdown
Member

@pwnage101 pwnage101 commented May 13, 2026

This fixes a devstack-specific bug resulting in enterprise and consent plugin settings not being loaded because the entire enterprise feature was disabled (ENABLE_ENTERPRISE_INTEGRATION = False) when the plugins were loaded. Settings in devstack.py are loaded AFTER plugins are loaded, which makes it a bad place to flip on
ENABLE_ENTERPRISE_INTEGRATION because it would be too late.

Here's the order of operations:

  1. lms/envs/common.py is loaded.
    a. ENABLE_ENTERPRISE_INTEGRATION set to False.
    b. add_plugins() is called.
    b. enterprise/settings/common.py is loaded.
    1. plugin_settings() exits early.
  2. lms/envs/production.py is loaded.
    a. Loads the file /edx/etc/lms.yml seeded by configuration_files/lms.yml.
    b. ENABLE_ENTERPRISE_INTEGRATION stays False (‼️This PR updates this file to set it to True).
    b. enterprise/settings/production.py is loaded, which re-loads common.py.
    1. plugin_settings() exits early again because ENABLE_ENTERPRISE_INTEGRATION is still False.
  3. lms/envs/devstack.py (seeded by py_configuration_files/lms.py) is loaded.
    a. FINALLY the enterprise feature is enabled, but it's too late because enterprise plugin has no devstack.py file, nor should it.

Alternatives Considered

  • Just not exiting early inside of plugin_settings(). After all, the plugin framework docs make it pretty clear that installing a plugin pip package implies that it should be enabled---an enable flag would be redundant. The whole point is that a integrating a plugin should be as easy as pip installing it.
    • This approach was rejected at least for now, mainly because we're trying to carefully and incrementally transition the edx-enterprise package from being a required dependency to an optional one. Right now, everybody has this package installed, but not everybody wants it. We need to slowly ween operators off this package but that takes time.

This fixes a devstack-specific bug resulting in enterprise and consent
plugin settings not being loaded because the entire enterprise feature
was disabled (ENABLE_ENTERPRISE_INTEGRATION = False) when the plugins
were loaded.  Settings in devstack.py are loaded AFTER plugins are
loaded, which makes it a bad place to flip on
ENABLE_ENTERPRISE_INTEGRATION because it would be too late.
Copilot AI review requested due to automatic review settings May 13, 2026 23:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Moves the ENABLE_ENTERPRISE_INTEGRATION = true setting from py_configuration_files/lms.py (devstack.py-style overrides loaded after plugins) to configuration_files/lms.yml (YAML configuration loaded before plugins), so the enterprise and consent plugin settings are properly loaded in devstack.

Changes:

  • Remove ENABLE_ENTERPRISE_INTEGRATION = True and its comments from py_configuration_files/lms.py.
  • Add ENABLE_ENTERPRISE_INTEGRATION: true with equivalent comments to configuration_files/lms.yml.
  • Strip a trailing whitespace from the ENABLE_MKTG_SITE: true line.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
py_configuration_files/lms.py Removes the late-loaded enterprise toggle so it can be set earlier via YAML.
configuration_files/lms.yml Adds ENABLE_ENTERPRISE_INTEGRATION: true (with explanatory comments) before plugin loading; cleans trailing whitespace on a nearby line.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@pwnage101 pwnage101 closed this May 14, 2026
@pwnage101 pwnage101 reopened this May 14, 2026
@pwnage101 pwnage101 merged commit 901d080 into master May 14, 2026
47 of 51 checks passed
@pwnage101 pwnage101 deleted the pwnage101/enable-enterprise-before-plugins branch May 14, 2026 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants