Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#25510 closed Bug (fixed)

Putting invalid app name in INSTALLED_APPS makes runserver raise django.core.exceptions.AppRegistryNotReady

Reported by: Sylvain Fankhauser Owned by: nobody
Component: Core (Other) Version: 1.8
Severity: Normal Keywords:
Cc: me@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If you put an app that can't be imported in INSTALLED_APPS and then run the runserver command, you get the following exception:

Traceback (most recent call last):
  File "./manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/home/sylvain/.virtualenvs/tmp-89f7042ac871db52/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 351, in execute_from_command_line
    utility.execute()
  File "/home/sylvain/.virtualenvs/tmp-89f7042ac871db52/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 343, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/sylvain/.virtualenvs/tmp-89f7042ac871db52/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 177, in fetch_command
    commands = get_commands()
  File "/home/sylvain/.virtualenvs/tmp-89f7042ac871db52/local/lib/python2.7/site-packages/django/utils/lru_cache.py", line 101, in wrapper
    result = user_function(*args, **kwds)
  File "/home/sylvain/.virtualenvs/tmp-89f7042ac871db52/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 72, in get_commands
    for app_config in reversed(list(apps.get_app_configs())):
  File "/home/sylvain/.virtualenvs/tmp-89f7042ac871db52/local/lib/python2.7/site-packages/django/apps/registry.py", line 137, in get_app_configs
    self.check_apps_ready()
  File "/home/sylvain/.virtualenvs/tmp-89f7042ac871db52/local/lib/python2.7/site-packages/django/apps/registry.py", line 124, in check_apps_ready
    raise AppRegistryNotReady("Apps aren't loaded yet.")
django.core.exceptions.AppRegistryNotReady: Apps aren't loaded yet.

Other commands such as migrate or check correctly raise an ImportError.

Change History (13)

comment:1 Changed 4 years ago by Tim Graham

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

It's a regression in 1.8.5 bisected to cc14d51ee8325c82665cb98af4dfe49aab565d52.

comment:2 Changed 4 years ago by Aymeric Augustin

I tried writing a test for this but it's hard. The following test fails when the bug is present and hangs when it's fixed:

diff --git a/tests/admin_scripts/tests.py b/tests/admin_scripts/tests.py
index 4e976d8..fdaaf21 100644
--- a/tests/admin_scripts/tests.py
+++ b/tests/admin_scripts/tests.py
@@ -1385,6 +1385,21 @@ class ManageRunserverEmptyAllowedHosts(AdminScriptTestCase):
         self.assertOutput(err, 'CommandError: You must set settings.ALLOWED_HOSTS if DEBUG is False.')
 
 
+class ManageRunserverNonExistentApp(AdminScriptTestCase):
+    def setUp(self):
+        self.write_settings('settings.py', apps=['no_such_app'])
+
+    def tearDown(self):
+        self.remove_settings('settings.py')
+
+    def test_non_existent_app_raises_import_error(self):
+        """
+        Ensure that a non-existent app raises an ImportError (#25510).
+        """
+        out, err = self.run_manage(['runserver'])
+        self.assertNoOutput(out)
+        self.assertOutput(err, 'ImportError')
+
 class ManageTestserver(AdminScriptTestCase):
     from django.core.management.commands.testserver import Command as TestserverCommand
 

I don't want to make a test that depends on a time.sleep(2) because it will be slow and flaky. I don't have a suitable way to detect that runserver has forked a child and is running. I can't run runserver without autoreload because that makes the bug go away.

comment:3 Changed 4 years ago by Aymeric Augustin

Has patch: set
Severity: Release blockerNormal

comment:4 Changed 4 years ago by Andriy Sokolovskiy

Since another problem was found here
https://code.djangoproject.com/ticket/25523#comment:2 may be it should be considered as release blocker

comment:5 Changed 4 years ago by Andriy Sokolovskiy

Cc: me@… added

comment:6 Changed 4 years ago by Aymeric Augustin

It can be considered a release blocker if it meets one of these conditions — from https://docs.djangoproject.com/en/1.8/internals/release-process/:

Security issues.
Data loss bugs.
Crashing bugs.
Major functionality bugs in newly-introduced features.

If you think it does, feel free to toggle the flag again. (I would find that advantageous because I could stop caring about this messy problem, given that other people will pay attention in the run up to the 1.9 release.)

comment:7 Changed 4 years ago by Tim Graham

I thought we should backport to 1.8 since it's a regression in a minor release.

comment:8 Changed 4 years ago by Aymeric Augustin

I updated the patch to address the comment on the PR and added release notes for backporting to 1.8.6.

comment:9 Changed 4 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

Pending minor comment on the PR.

comment:10 Changed 4 years ago by Aymeric Augustin <aymeric.augustin@…>

Resolution: fixed
Status: newclosed

In df0a446f:

Fixed #25510 -- Allowed runserver to start with incorrect INSTALLED_APPS.

In that case, the content of INSTALLED_APPS will be ignored until it's
fixed and the autoreloader kicks in. I confirmed this behavior manually.
As explained on the ticket it's hard to write a test for this case

comment:11 Changed 4 years ago by Aymeric Augustin <aymeric.augustin@…>

In 9ccb92a:

[1.8.x] Fixed #25510 -- Allowed runserver to start with incorrect INSTALLED_APPS.

In that case, the content of INSTALLED_APPS will be ignored until it's
fixed and the autoreloader kicks in. I confirmed this behavior manually.
As explained on the ticket it's hard to write a test for this case

Backport of df0a446f from master.

comment:12 Changed 4 years ago by Tim Graham <timograham@…>

In 24ebf17f:

[1.9.x] Fixed #25510 -- Allowed runserver to start with incorrect INSTALLED_APPS.

In that case, the content of INSTALLED_APPS will be ignored until it's
fixed and the autoreloader kicks in. I confirmed this behavior manually.
As explained on the ticket it's hard to write a test for this case.

Backport of df0a446fd4c864c003e4f941b5b7abd6f10c9427 from master

comment:13 Changed 4 years ago by Tim Graham <timograham@…>

In 7a59910:

Refs #25510 -- Forwardport of 1.9.1 release note.

Note: See TracTickets for help on using tickets.
Back to Top