Skip to content
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

refactor: Clean up CMS production settings cruft #36306

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kdmccormick
Copy link
Member

@kdmccormick kdmccormick commented Feb 26, 2025

Description

What it says on the tin. See individual commits for more details

Supporting Information

Part of:

This is essentially the CMS version of:

Testing instructions

With an up-to-date version of both master and this PR's branch, run ./diff_settings.sh master kdmccormick/settings-cms-envs-production, where diff_settings.sh is:

#!/usr/bin/env bash
# Usage: ./diff_settings.sh GIT_REF_A GIT_REF_B
# Example: ./diff_settings.sh upstream/master myusername/myrefactorbranch
# Will fail if git state is not clean.

set -xeuo pipefail  # Be verbose and strict

REF_A=$1
REF_B=$2
DIR_A=dump_settings_a
DIR_B=dump_settings_b

SETTINGS_AND_CFG=( \
"envs.production,lms/envs/minimal.yml," \
"envs.production,lms/envs/mock.yml,cms/envs/mock.yml" \
"envs.tutor.production,$LMS_CFG,$CMS_CFG" \
"envs.tutor.development,$LMS_CFG,$CMS_CFG" \
)

rm -rf "$DIR_A" "$DIR_B"
mkdir "$DIR_A" "$DIR_B"

for settings_and_cfg in "${SETTINGS_AND_CFG[@]}" ; do
    settings="${settings_and_cfg%%,*}"
    cfg="${settings_and_cfg#*,}"

    lms_cfg="${cfg%%,*}"
    lms_module="lms.${settings}"
    lms_outfile="${lms_module}__${lms_cfg//\//_}.json"
    git checkout "$REF_A"
    DJANGO_SETTINGS_MODULE="$lms_module" LMS_CFG="$lms_cfg" ./manage.py lms dump_settings > "$DIR_A/$lms_outfile"
    git checkout "$REF_B"
    DJANGO_SETTINGS_MODULE="$lms_module" LMS_CFG="$lms_cfg" ./manage.py lms dump_settings > "$DIR_B/$lms_outfile"

    cms_cfg="${cfg#*,}"
    if [[ -n "$cms_cfg" ]]; then
        cms_module="cms.${settings}"
        cms_outfile="${cms_module}__${cms_cfg//\//_}.json"
        git checkout "$REF_A"
        DJANGO_SETTINGS_MODULE="$cms_module" CMS_CFG="$cms_cfg" ./manage.py lms dump_settings > "$DIR_A/$cms_outfile"
        git checkout "$REF_B"
        DJANGO_SETTINGS_MODULE="$cms_module" CMS_CFG="$cms_cfg" ./manage.py lms dump_settings > "$DIR_B/$cms_outfile"
    fi
done

diff "$DIR_A" "$DIR_B" && echo "No difference!"

Should yield:

...
+ diff dump_settings_a dump_settings_b
diff dump_settings_a/cms.envs.production__cms_envs_mock.yml.json dump_settings_b/cms.envs.production__cms_envs_mock.yml.json
4405,4415d4404
<     "KEYS_WITH_MERGED_VALUES": [
<         "FEATURES",
<         "TRACKING_BACKENDS",
<         "EVENT_TRACKING_BACKENDS",
<         "JWT_AUTH",
<         "CELERY_QUEUES",
<         "MKTG_URL_LINK_MAP",
<         "MKTG_URL_OVERRIDES",
<         "REST_FRAMEWORK",
<         "EVENT_BUS_PRODUCER_CONFIG"
<     ],
diff dump_settings_a/cms.envs.tutor.development___openedx_config_cms.env.yml.json dump_settings_b/cms.envs.tutor.development___openedx_config_cms.env.yml.json
2115,2125d2114
<     "KEYS_WITH_MERGED_VALUES": [
<         "FEATURES",
<         "TRACKING_BACKENDS",
<         "EVENT_TRACKING_BACKENDS",
<         "JWT_AUTH",
<         "CELERY_QUEUES",
<         "MKTG_URL_LINK_MAP",
<         "MKTG_URL_OVERRIDES",
<         "REST_FRAMEWORK",
<         "EVENT_BUS_PRODUCER_CONFIG"
<     ],
diff dump_settings_a/cms.envs.tutor.production___openedx_config_cms.env.yml.json dump_settings_b/cms.envs.tutor.production___openedx_config_cms.env.yml.json
2079,2089d2078
<     "KEYS_WITH_MERGED_VALUES": [
<         "FEATURES",
<         "TRACKING_BACKENDS",
<         "EVENT_TRACKING_BACKENDS",
<         "JWT_AUTH",
<         "CELERY_QUEUES",
<         "MKTG_URL_LINK_MAP",
<         "MKTG_URL_OVERRIDES",
<         "REST_FRAMEWORK",
<         "EVENT_BUS_PRODUCER_CONFIG"
<     ],

@kdmccormick kdmccormick force-pushed the kdmccormick/settings-cms-envs-production branch 2 times, most recently from 8fcc3de to e0e2246 Compare February 28, 2025 20:59
@kdmccormick kdmccormick force-pushed the kdmccormick/settings-cms-envs-production branch from e0e2246 to accbbec Compare February 28, 2025 22:42
@kdmccormick kdmccormick added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Feb 28, 2025
We separate out the handful of settings which have useful comments.
The rest of the settings' comments were not helpful--they were either
just stating the obvious, or they were duplicative of what's documented
in common.py.
@kdmccormick kdmccormick marked this pull request as ready for review February 28, 2025 23:00
@kdmccormick kdmccormick requested a review from feanil as a code owner February 28, 2025 23:00
@kdmccormick
Copy link
Member Author

@feanil This is ready for review.

I've tested using dump_settings, and I'll smoke test with the PR sandbox on Monday.

@open-craft-grove
Copy link

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

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants