Skip to content

Commit

Permalink
fix: remove custom startup to fix dev reloading
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ormsbee committed Feb 26, 2025
1 parent 9451cfc commit 6dae997
Show file tree
Hide file tree
Showing 7 changed files with 0 additions and 66 deletions.
10 changes: 0 additions & 10 deletions cms/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@
only running cms tests.
"""


import importlib
import logging
import os

import pytest

Expand All @@ -29,13 +26,6 @@ def pytest_configure(config):
else:
logging.info("pytest did not register json_report correctly")

if config.getoption('help'):
return
settings_module = os.environ.get('DJANGO_SETTINGS_MODULE')
startup_module = 'cms.startup' if settings_module.startswith('cms') else 'lms.startup'
startup = importlib.import_module(startup_module)
startup.run()


@pytest.fixture(autouse=True, scope='function')
def _django_clear_site_cache():
Expand Down
20 changes: 0 additions & 20 deletions cms/startup.py

This file was deleted.

3 changes: 0 additions & 3 deletions cms/wsgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@
import os # lint-amnesty, pylint: disable=wrong-import-order, wrong-import-position
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "cms.envs.aws")

import cms.startup as startup # lint-amnesty, pylint: disable=wrong-import-position
startup.run()

# This application object is used by the development server
# as well as any WSGI server configured to use this file.
from django.core.wsgi import get_wsgi_application # lint-amnesty, pylint: disable=wrong-import-order, wrong-import-position
Expand Down
20 changes: 0 additions & 20 deletions lms/startup.py

This file was deleted.

3 changes: 0 additions & 3 deletions lms/wsgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
import os # lint-amnesty, pylint: disable=wrong-import-order, wrong-import-position
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "lms.envs.aws")

import lms.startup as startup # lint-amnesty, pylint: disable=wrong-import-position
startup.run()

# This application object is used by the development server
# as well as any WSGI server configured to use this file.
from django.core.wsgi import get_wsgi_application # lint-amnesty, pylint: disable=wrong-import-order, wrong-import-position
Expand Down
3 changes: 0 additions & 3 deletions lms/wsgi_apache_lms.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "lms.envs.aws")
os.environ.setdefault("SERVICE_VARIANT", "lms")

import lms.startup as startup # lint-amnesty, pylint: disable=wrong-import-position
startup.run()

# This application object is used by the development server
# as well as any WSGI server configured to use this file.
from django.core.wsgi import get_wsgi_application # lint-amnesty, pylint: disable=wrong-import-order, wrong-import-position
Expand Down
7 changes: 0 additions & 7 deletions manage.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,13 @@
"""
# pylint: disable=wrong-import-order, wrong-import-position


from openedx.core.lib.logsettings import log_python_warnings
log_python_warnings()

# Patch the xml libs before anything else.
from openedx.core.lib.safe_lxml import defuse_xml_libs # isort:skip
defuse_xml_libs()

import importlib
import os
import sys
from argparse import ArgumentParser
Expand Down Expand Up @@ -51,7 +49,6 @@ def parse_args():
help_string=lms.format_help(),
settings_base='lms/envs',
default_settings='lms.envs.devstack_docker',
startup='lms.startup',
)

cms = subparsers.add_parser(
Expand All @@ -70,7 +67,6 @@ def parse_args():
settings_base='cms/envs',
default_settings='cms.envs.devstack_docker',
service_variant='cms',
startup='cms.startup',
)

edx_args, django_args = parser.parse_known_args()
Expand Down Expand Up @@ -99,8 +95,5 @@ def parse_args():
# This will trigger django-admin.py to print out its help
django_args.append('--help')

startup = importlib.import_module(edx_args.startup)
startup.run()

from django.core.management import execute_from_command_line
execute_from_command_line([sys.argv[0]] + django_args)

0 comments on commit 6dae997

Please sign in to comment.