[Attempt 2] refactor: Clean up lms/envs/production.py cruft #36131
Conversation
This reintroduces commit 1593923, which was reverted due to a typo. The typo is fixed in the commit immediately following this one. Co-Authored-By: Feanil Patel <[email protected]>
kdmccormick
force-pushed
the
kdmccormick/lms-envs-production-cleanup-2
branch
from
b70d54a to
c0cb126
Compare
|
Note: Test plan being discussed in Slack, with request not to merge without a plan. |
There was a problem hiding this comment.
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
added
the
create-sandbox
|
(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. |
|
Sandbox deployment failed 💥 |
0x29a
added
create-sandbox
|
Sandbox deployment successful 🚀 |
Supporting information
Re-introdues #36115, which was reverted by #36129 due to a typo.
The typo fix is included as a separate commit.
Other information
There is an ongoing discussion of how to better safeguard against bugs like the one that led to the revert: #36129 (comment)
Merge considerations
I will wait until Tuesday Jan 21 to merge this.