[Attempt 2] refactor: Clean up lms/envs/production.py cruft

[kdmccormick]

Description

This PR has 3 commits:

1. We re-introduce refactor: Clean up lms/envs/production.py cruft #36115, which was reverted by Revert "refactor: Clean up lms/envs/production.py cruft" #36129 due to a typo.
2. We fix said typo
3. We tighten up our backwards-compatibility of ENV_TOKENS and a few other "auxillary" settings dictionaries, out of an abundance of caution and a desire not to break any existing Django plugins unintentionally. See commit description for more info.

Altogether, this PR makes exactly ONE change to lms/envs/production.py, and that is the removal of KEYS_WITH_MERGED_VALUES from the settings namespace. This "setting" was an internal implementation detail of production.py, which should have been prefixed with an underscore. I am not considering this a breaking change.

Otherwise, this PR should exactly preserve the global namespace of lms/envs/production.py given any LMS_CFG yaml file.

Testing Instructions

Here is a little script you can paste into edx-platform/diff_settings.sh (note the Tutor vs non-Tutor difference):

#!/usr/bin/env bash
# Usage: ./diff_settings.sh GIT_REF_A GIT_REF_B
# 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=( envs.devstack envs.production )  #  If not using Tutor
SETTINGS=( envs.tutor.production envs.devstack envs.tutor.development )  # If using Tutor

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

for system in cms lms ; do
    for settings in "${SETTINGS[@]}" ; do
        module="${system}.${settings}"
        git checkout "$REF_A"
        DJANGO_SETTINGS_MODULE="$module" ./manage.py "$system" dump_settings > "$DIR_A/$module.json"
        git checkout "$REF_B"
        DJANGO_SETTINGS_MODULE="$module" ./manage.py "$system" dump_settings > "$DIR_B/$module.json"
    done
done

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

Ensure you have this PR's branch checked out and a updated master branch, and then run:

./diff_settings.sh master kdmccormick/lms-envs-production-cleanup-2

You should see that the only diff is the removal of KEYS_WITH_MERGED_VALUES from the LMS files:

+ diff dump_settings_a dump_settings_b
diff dump_settings_a/lms.envs.devstack.json dump_settings_b/lms.envs.devstack.json
2770,2780d2769
<     "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/lms.envs.tutor.development.json dump_settings_b/lms.envs.tutor.development.json
2709,2719d2708
<     "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/lms.envs.tutor.production.json dump_settings_b/lms.envs.tutor.production.json
2446,2456d2445
<     "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"
<     ],

[robrap]

Note: Test plan being discussed in Slack, with request not to merge without a plan.

[feanil]

I took a look through again with a finer view of the additions to make sure there weren't any typos and I was able to bring up the LMS and CMS from this version of the settings file and the minimal.yaml locally so I think it's good to go but I'll wait till we add the unit test that we were discussing in slack before providing the final approval.

[kdmccormick]

(rephrasing from Slack) I don't see any way to unit test this PR, since unit tests use lms/envs/test.py, which is based directly on lms/envs/common.py, not lms/envs/production.py. I think we will have to rely on code review, manual testing, and the PR sandbox for verification.

[open-craft-grove]

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]

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

[kdmccormick]

Converting this to draft until I can manually reverify it using dump_settings from #36162

[open-craft-grove]

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]

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

[kdmccormick]

@robrap @feanil This is ready for re-review.

[open-craft-grove]

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

[open-craft-grove]

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

[robrap]

Thanks @kdmccormick. I'm going to have @dianakhuang take over anything required to get this over-the-line from 2U's end. Thanks!

[kdmccormick]

Sounds good @robrap . Since non-SRE engineers cannot run management commands on stage (unless that changed since I left), I would recommend using a prod-like yaml file to compare the output of dump_settings for this PR vs for master before I merge, and ensure that KEYS_WITH_MERGED_VALUES is the only differing setting.

[robrap]

Yes. @dianakhuang is working on getting a prod-like yaml file on a sandbox.