refactor: bring common settings into common module #37746

wgu-taylor-payne · 2025-12-10T19:30:42Z

Description

Bring up more common settings from lms/envs/common.py and cms/envs/common.py. This is done by calculating common paths in openedx/envs/common.py, using Derived to delay resolution of platform specific dependencies, or bringing up the common parts of mutable settings while adding the different parts in the platform specific common files.

Here is a summary of the differences in the rendered settings between master and this branch:

EVENT_BUS_PRODUCER_CONFIG: removed redundant values (based on comments: cms, lms).
OPTIONAL_APPS: swap the order of 'integrated_channels.cornerstone' and 'integrated_channels.xapi' in CMS to match LMS. From my analysis of the the apps, this reordering shouldn't introduce any side-effects.
INSTALLED_APPS: Changed based on the reorder in OPTIONAL_APPS.
USE_L10N: this variable was removed from Django 5.0 and it isn't referenced in the code, so I thought we'd be safe to remove it at this time.
LOGIN_ISSUE_SUPPORT_LINK: moved to Derived from the value of SUPPORT_SITE_LINK. The default for SUPPORT_SITE_LINK is '', but is overridden via YAML in cms/envs/mock.yml to https://support.localhost. Similarly ID_VERIFICATION_SUPPORT_LINK, PASSWORD_RESET_SUPPORT_LINK, and ACTIVATION_EMAIL_SUPPORT_LINK were changed to be derived from SUPPORT_SITE_LINK, so have similar implications.
WEBPACK_LOADER['STATS_FILE']: Different for test settings only. Changed to be derived from the STATIC_ROOT setting, so now this value is updated based on the STATIC_ROOT for testing.
QUEUE_VARIANT and CONFIG_PREFIX: SERVICE_VARIANT has been changed to always be cms or lms. This means it will never be falsy and will always be lowercase. This means we can use SERVICE_VARIANT directly in the different queue names, and no longer need to use CONFIG_PREFIX and QUEUE_VARIANT.

Supporting information

This task is part of the effort to simplify settings as outlined in ADR 0022 - Django settings simplification.

Testing instructions

Run the dump_settings management command on the different terminal settings modules, given different yml config files and compare the diff on the rendered settings between master and tpayne/consolidate-with-derived. To simplify this, I used the script diff_settings.py and also table_diff.py to simplify the diff output. I've added the output of these scripts in this gist.

Deadline

None

openedx-webhooks · 2025-12-10T19:30:47Z

Thanks for the pull request, @wgu-taylor-payne!

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.

wgu-taylor-payne · 2025-12-11T19:20:56Z

cms/envs/common.py

-    XModuleMixin,
-    EditInfoMixin,
+# DO NOT EXPAND THIS LIST!! See declaration in openedx/envs/common.py for more information
+XBLOCK_MIXINS.insert(2, ResourceTemplates)


I tried to append ResourceTemplates with the others added here in the CMS, but this caused an issue with method resolution in the tests (TypeError: Cannot create a consistent method resolution).

wgu-taylor-payne · 2025-12-11T19:23:19Z

cms/envs/common.py

 # prefix.
 CONFIG_PREFIX = SERVICE_VARIANT + "." if SERVICE_VARIANT else ""

+SERVICE_NAME = 'cms'


I thought at first to use SERVICE_VARIANT to stand in place of 'lms' and 'cms', but SERVICE_VARIANT as it has been defined historically is not guaranteed to have these values. So, I introduce this new value.

Can you say more about this? What do you mean it's not gauranteed to have these values?

Here is how it is declared in the common.py modules (along with the variables that first depend on it and my commentary):

# we get the value from the environment, or fall back on "lms" (or "cms") # we don't validate the value from the environment SERVICE_VARIANT = os.environ.get('SERVICE_VARIANT', "lms") # this line anticipates that SERVICE_VARIANT could be falsy CONFIG_PREFIX = SERVICE_VARIANT + "." if SERVICE_VARIANT else "" # this line anticipates that SERVICE_VARIANT might not be lowercase QUEUE_VARIANT = CONFIG_PREFIX.lower()

And then I noticed in manage.py, that the lms service-variant argument assumes more choices that just lms:

lms.add_argument( '--service-variant', choices=['lms', 'lms-xml', 'lms-preview'], default='lms', help='Which service variant to run, when using the production environment')

In practice today, it might just be 'lms' and 'cms' for this value, but I wasn't fully sure if that was the case or not., so I decided to err on the safe side.

Thanks for the clarification. Yea, I can see how you would think it could be other values. But in practice, you can assume it's going to be either lms or cms, the value must be set for manage.py to run and the lms-xml and lms-preview variants are historic and not expected to work anymore.

FYI, I'm cleaning up manage.py in #37790 to reduce confusion.

Thanks for clarifying that. I'm trying to think through the implications, and am left wondering if SERVICE_VARIANT should just be declared 'lms' in lms/envs/common.py and 'cms' in cms/envs/common.py, not allowing the value to be set in the environment. The service variant would just be dependent on the DJANGO_SETTINGS_MODULE (the settings module inherits from a cms or lms settings module, and the appropriate value will be set in the respective common.py file).

I'm happy to make that change, but want to make sure I'm not missing anything.

am left wondering if SERVICE_VARIANT should just be declared 'lms' in lms/envs/common.py and 'cms' in cms/envs/common.py, not allowing the value to be set in the environment.

Yes, I think this is exactly right. Back when lms-preview and lms-xml existed, this would have been a bad assumption, but as far as I can tell, we're clear now to make this change.

wgu-taylor-payne · 2025-12-11T19:28:23Z

cms/envs/common.py

-    # LMS events. These have to be copied over here because lms.common adds some derived entries as well,
-    # and the derivation fails if the keys are missing. If we ever remove the import of lms.common, we can remove these.
-    'org.openedx.learning.certificate.created.v1': {
-        'learning-certificate-lifecycle':
-            {'event_key_field': 'certificate.course.course_key', 'enabled': False},
-    },
-    'org.openedx.learning.certificate.revoked.v1': {
-        'learning-certificate-lifecycle':
-            {'event_key_field': 'certificate.course.course_key', 'enabled': False},
-    },


Removed these now that we have removed the dependency on lms.envs.common. Introduced in #33458.

wgu-taylor-payne · 2025-12-11T19:36:14Z

cms/envs/test.py



 import os
+import tempfile


Implicitly available from openedx/envs/test.py, but explicitly declaring now.

wgu-taylor-payne · 2025-12-11T19:38:11Z

lms/envs/common.py

+CONTEXT_PROCESSORS.remove('django.contrib.messages.context_processors.messages')
+CONTEXT_PROCESSORS[5:5] = [


This is one where to match the original order, we have to be a little creative. I was on the fence with this one.

wgu-taylor-payne · 2025-12-11T19:57:10Z

lms/envs/common.py

-    # CMS events. These have to be copied over here because cms.common adds some derived entries as well,
-    # and the derivation fails if the keys are missing. If we ever fully decouple the lms and cms settings,
-    # we can remove these.
-    'org.openedx.content_authoring.xblock.published.v1': {
-        'course-authoring-xblock-lifecycle':
-            {'event_key_field': 'xblock_info.usage_key', 'enabled': False},
-    },
-    'org.openedx.content_authoring.xblock.deleted.v1': {
-        'course-authoring-xblock-lifecycle':
-            {'event_key_field': 'xblock_info.usage_key', 'enabled': False},
-    },
-    'org.openedx.content_authoring.xblock.duplicated.v1': {
-        'course-authoring-xblock-lifecycle':
-            {'event_key_field': 'xblock_info.usage_key', 'enabled': False},
-    },


Removed now that common modules are decoupled. (Introduced in #33458).

wgu-taylor-payne · 2025-12-11T19:59:55Z

openedx/envs/common.py

 DEBUG = False

 USE_TZ = True
+TIME_ZONE = 'UTC'


I thought it would be valuable to gather all the Django specific settings into this section, so I brought all the Django settings that weren't in this section previously and added them here, like this one.

These are the settings, I can undo this if it is problematic.

TIME_ZONE

CSRF_COOKIE_AGE

CSRF_TRUSTED_ORIGINS

CSRF_COOKIE_SECURE

ALLOWED_HOSTS

X_FRAME_OPTIONS

DATA_UPLOAD_MAX_MEMORY_SIZE

DATA_UPLOAD_MAX_NUMBER_FIELDS

STATICFILES_FINDERS

USE_I18N

LANGUAGE_CODE

CACHES

DATABASES

DEFAULT_AUTO_FIELD

No, I think that's a helpful reorganization.

openedx/envs/common.py

wgu-taylor-payne · 2025-12-11T20:10:11Z

openedx/envs/common.py

 }

-USE_I18N = True
-USE_L10N = True


this variable was removed from Django 5.0 and it isn't referenced in the code, so I thought we'd be safe to remove it at this time.

wgu-taylor-payne · 2025-12-11T20:13:31Z

openedx/envs/common.py

+    ('integrated_channels.cornerstone', None),
+    ('integrated_channels.xapi', None),


The LMS has cornerstone and xapi in this order. For the CMS, this is a swap in the order of these two. Looking at the apps (cornerstone, xapi), I didn't see anything that would make this problematic.

wgu-taylor-payne · 2025-12-11T21:15:13Z

@kdmccormick @feanil Could either of you look this over when you get a chance?

feanil · 2025-12-16T15:48:51Z

Took a quick first pass, I'll take a deeper look later this week.

feanil

I didn't see any issues in my review. I think it's worth moving from SERVICE_NAME to SERVICE_VARIANT but I don't feel strongly enough about that to block merging this as is. It's a huge improvement and we can deal with the small complexity of having both of those if we need to.

feanil · 2025-12-18T19:14:07Z

lms/envs/common.py

-    ('integrated_channels.integrated_channel', None),
-    ('integrated_channels.degreed', None),
-    ('integrated_channels.degreed2', None),
-    ('integrated_channels.sap_success_factors', None),
-    ('integrated_channels.cornerstone', None),
-    ('integrated_channels.xapi', None),
-    ('integrated_channels.blackboard', None),
-    ('integrated_channels.canvas', None),
-    ('integrated_channels.moodle', None),
-
-    # Channel Integrations Apps
-    ('channel_integrations.integrated_channel', None),
-    ('channel_integrations.degreed2', None),
-    ('channel_integrations.sap_success_factors', None),
-    ('channel_integrations.cornerstone', None),
-    ('channel_integrations.xapi', None),
-    ('channel_integrations.blackboard', None),
-    ('channel_integrations.canvas', None),
-    ('channel_integrations.moodle', None),


As far as I know this is not documented anywhere. I was unable to find out why we have both of these. The integrated-channels repo came later as a way to separate the concerns of the enterprise repo from the integrated reporting capabilities but I'm not sure what state it's in at the moment.

wgu-taylor-payne · 2025-12-19T19:16:46Z

@feanil @kdmccormick I updated SERVICE_VARIANT to not be set from the environment. Do those changes look good? Should I also remove these references?

edx-platform/manage.py

Line 87 in 7b91d1d

os.environ.setdefault("SERVICE_VARIANT", edx_args.service_variant)

edx-platform/openedx/core/djangoapps/content/course_overviews/management/commands/simulate_publish.py

Line 192 in 7b91d1d

if os.environ.get('SERVICE_VARIANT', 'cms').startswith('lms'):

edx-platform/lms/wsgi_apache_lms.py

Line 15 in 7b91d1d

os.environ.setdefault("SERVICE_VARIANT", "lms")

feanil · 2025-12-22T19:25:23Z

@wgu-taylor-payne the changes look good. I think as long as we fix all the gets we can drop the sets. I think it's fine to do that in this PR but also fine to do it as a separate follow-up PR.

wgu-taylor-payne · 2026-01-05T16:35:10Z

manage.py

    )

-    edx_args, django_args = parser.parse_known_args()
+    known_args, remaining_args = parser.parse_known_args()


pylint didn't like these variables being defined with the same name as the top level scope.

manage.py

wgu-taylor-payne · 2026-01-05T19:00:20Z

@feanil I decided to make the SERVICE_VARIANT environ get/set changes here in this PR if you want to look those over when you get a chance.

wgu-taylor-payne requested review from feanil and kdmccormick as code owners December 10, 2025 19:30

openedx-webhooks added open-source-contribution PR author is not from Axim or 2U core contributor PR author is a Core Contributor (who may or may not have write access to this repo). labels Dec 10, 2025

openedx-webhooks added this to Contributions Dec 10, 2025

github-project-automation bot moved this to Needs Triage in Contributions Dec 10, 2025

wgu-taylor-payne added the mao-onboarding Reviewing this will help onboard devs from an Axim mission-aligned organization (MAO). label Dec 10, 2025

wgu-taylor-payne marked this pull request as draft December 10, 2025 20:24

mphilbrick211 moved this from Needs Triage to Waiting on Author in Contributions Dec 11, 2025

wgu-taylor-payne force-pushed the tpayne/consolidate-with-derived branch from dce6a63 to 34baf2d Compare December 11, 2025 18:43

wgu-taylor-payne commented Dec 11, 2025

View reviewed changes

wgu-taylor-payne marked this pull request as ready for review December 11, 2025 21:09

feanil approved these changes Dec 18, 2025

View reviewed changes

wgu-taylor-payne force-pushed the tpayne/consolidate-with-derived branch from bb342c7 to c06bbd2 Compare December 19, 2025 17:36

wgu-taylor-payne requested a review from feanil December 19, 2025 18:57

wgu-taylor-payne mentioned this pull request Dec 20, 2025

refactor: hoist annotations for common settings with differing values #37800

Merged

wgu-taylor-payne force-pushed the tpayne/consolidate-with-derived branch 2 times, most recently from b5c2099 to c46a399 Compare December 24, 2025 01:16

wgu-taylor-payne commented Jan 5, 2026

View reviewed changes

feanil approved these changes Jan 6, 2026

View reviewed changes

mphilbrick211 moved this from Waiting on Author to In Eng Review in Contributions Jan 6, 2026

wgu-taylor-payne mentioned this pull request Jan 7, 2026

[DEPR]: Setting SERVICE_VARIANT via environment variable #37847

Open

1 task

refactor: bring common settings into common module

fb2477f

wgu-taylor-payne force-pushed the tpayne/consolidate-with-derived branch from e3b62fb to fb2477f Compare January 8, 2026 18:53

wgu-taylor-payne merged commit 7111e8b into openedx:master Jan 9, 2026
84 of 104 checks passed

github-project-automation bot moved this from In Eng Review to Done in Contributions Jan 9, 2026

		CONTEXT_PROCESSORS.remove('django.contrib.messages.context_processors.messages')
		CONTEXT_PROCESSORS[5:5] = [

		('integrated_channels.cornerstone', None),
		('integrated_channels.xapi', None),

refactor: bring common settings into common module #37746

refactor: bring common settings into common module #37746

Uh oh!

Conversation

wgu-taylor-payne commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Supporting information

Testing instructions

Deadline

Uh oh!

openedx-webhooks commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wgu-taylor-payne Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wgu-taylor-payne Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wgu-taylor-payne commented Dec 11, 2025

Uh oh!

feanil commented Dec 16, 2025

Uh oh!

feanil left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wgu-taylor-payne commented Dec 19, 2025

Uh oh!

feanil commented Dec 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wgu-taylor-payne commented Jan 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wgu-taylor-payne commented Dec 10, 2025 •

edited

Loading

openedx-webhooks commented Dec 10, 2025 •

edited

Loading

wgu-taylor-payne Dec 11, 2025 •

edited

Loading

wgu-taylor-payne Dec 18, 2025 •

edited

Loading