fix: remove custom startup to fix dev reloading

[ormsbee]

The cms/startup.py and lms/startup.py files were created to
allow us to do a lot of custom initialization around things
like the ModuleStore, monkey-patching, adding MIME types to
our process, etc. As far back as 2017, we recognized that
this was a bad thing, marked these modules as "deprecated",
and started removing things or putting them in the standard
Django locations for them (0279181).

In its current state, these startup modules no longer do any
custom work, and just invoke django.startup(). But this is
meant for running Django code in "standalone" usage, e.g. if
you have a script that isn't a management command but needs
some Django functionality.

The "runserver" command used during development normally
launches a child process to serve requests and knows how to
kill and respawn that process when files are modified, so
that changes are reflected. It can also normally handle the
case where there's a SyntaxError in the child process, and
fixing that error will reload the code again.

Something about running django.startup() manually interferes
with this functionality in "runserver". It still reloads the
code in response to changes, but if the code gets into a
broken state for any reason (like a syntax error), the master
process itself dies. That causes the container to restart,
only to die again shortly afterwards in a loop until the
error is fixed. The container restarts will break any shell
you had opened into the container, as well as any IDE
integrations that connected to that container to access the
files and Python instance.

Getting rid of the custom startup code fixes this and moves
us one small step closer to being a more normal Django
project.

[ormsbee]

I think what it comes down to is that we're only supposed to invoke django.setup() once, and Django's various commands do that invocation. But if django.ready() is called by us, it's not re-initialized properly because Apps.ready will set set to True, and whatever extra initialization needs to happen, doesn't.

[ormsbee]

This will break this site-configuration script in Tutor, though I think we can just replace it with a django.setup() call.

[ormsbee]

FYI @blarghmatey and @pdpinch since this will affect the startup of LMS/Studio and I don't have a good way of testing that in non-Tutor environments. Please take a look to see if you have any concerns.

[blarghmatey]

Thanks for the heads up! I don't foresee any problems on our end, but I will run a build in one of our test environments to be sure.

[ormsbee]

@kdmccormick: I pretty much exclusively run tutor dev off of the main branch. What's the right way to test this in the production setup? Should I spin up the Sumac release, run tutor local with this commit cherry-picked? Just doing tutor local launch gave me a lot of errors related to missing packages.

[kdmccormick]

tutor local launch should work fine on main. Have you rebuild the local prod image lately, tutor images build openedx?

[kdmccormick]

(I've also added a label to create a PR sandbox on this.)

[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

[ormsbee]

Have you rebuild the local prod image lately, tutor images build openedx?

Hah, yup, this was exactly it. Started it up and everything works as expected. Studio and LMS both came up normally and celery tasks ran. Thank you!

[blarghmatey]

I was also able to deploy the changes in this branch to one of our QA environments without any issues.

[ormsbee]

@robrap: I've tested in Tutor and @blarghmatey has tested on MIT's deployment. If I can get a reviewer to look through this, would you be comfortable merging this next week?

[robrap]

I’d like to try when it is ready. Anything you can share to help me reduce risk would be appreciated. What did you test in Tutor?

Also, is any of this of concern? https://github.com/search?q=%28org%3Aopenedx+OR+org%3Aedx%29+%22lms%2Fstartup%22++NOT+is%3Aarchived&type=code
I didn’t search for cms.

[ormsbee]

I’d like to try when it is ready. Anything you can share to help me reduce risk would be appreciated. What did you test in Tutor?

I tested starting the LMS and Studio in both dev (so manage.py [lms/cms] runserver) and via uwsgi using lms/wsgi.py and cms/wsgi.py. Did sanity check that courseware rendered in LMS and Studio. Did a publish in Studio and verified that the celery task ran successfully.

Also, is any of this of concern? https://github.com/search?q=%28org%3Aopenedx+OR+org%3Aedx%29+%22lms%2Fstartup%22++NOT+is%3Aarchived&type=code

I'll remove the references to the files, but no, what I saw in that search and in the cms equivalent doesn't concern me. I think it's possible we might be able to remove some of the weirdness around catalog/models.py later on now that we won't have this bespoke startup sequence, but I think leaving it for now is fine.

[ormsbee]

@kdmccormick: I looked through the error log for the sandbox build, but didn't see anything specific enough for me to understand what's going on there and why it's failing.

[ormsbee]

Given that this PR has nothing but deletions, I plan to merge this even if codecov disapproves because I've written no test code for it.

[robrap]

@ormsbee

Given that this PR has nothing but deletions, I plan to merge this even if codecov disapproves because I've written no test code for it.

Does this mean once approved, I plan to have @robrap merge this early next week so he can test it?

[ormsbee]

@robrap: I was planning to wait until you were good with merging, but I didn't think you had the access to bypass the rules and merge even though the codecov check is technically failing. If you do have that ability, I'd be happy to have you merge instead.

[robrap]

@ormsbee: The only thing I see that is clearly blocking is that there still isn’t an approved review from someone with write access. Once you have that, we can see if I can merge.

[kdmccormick]

Code LGTM. I haven't tested, but I see that Dave and Tobias both have.

[ormsbee]

@robrap: Ah, great! I didn't realize it was non-blocking. I'll do minor comment cleanup tonight, and leave the merge timing next week to your discretion then. Thank you!

[robrap]

Ok. Let me know when it is ready to merge. Thanks.

[ormsbee]

@robrap: It should be merge-ready now. I did add another commit to just remove the weird local import rather than updating the comment about why it was there (d5aa834).

[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