Revisiting #8245: admin.py errors under runserver cause app to disappear

[Carl Meyer]

Hi all,

Today I filed #11957 [1], and because it is questioning an earlier
bugfix, I was asked to bring up the issue for discussion here.

Under the dev server, if an error occurs while importing an app's
admin.py, the error will show up on the first request, and then
disappear. On all subsequent requests (until the dev server is
restarted), the admin will load normally, but without that app or its
models. Even if the admin.py is fixed, the app will be missing until
the server is manually restarted. I've observed the confusion this
causes to beginners working through part 2 of the tutorial: they learn
that under most conditions the development server does not require a
restart, and when they hit this situation there is no clue that
restarting the dev server is the necessary fix.

This confusing behavior is a consequence of the fix chosen for #8245
[2] and applied in r9680 [3]. In order to avoid spurious
AlreadyRegistered errors masking the real error in an admin.py, a
global LOADING flag was introduced that causes admin.autodiscover() to
bail out if an earlier run did not finish successfully. The unintended
side effect is that once autodiscover() has encountered an error in an
admin.py under the dev server, it will perpetually bail until the
server is manually restarted (runserver auto-reloading does not help
here, because an admin.py that has failed to import is not in
sys.modules, so fixing the error in it doesn't trigger a restart).

There was an alternative patch attached to #8245 by jarrow that
instead rolls back all changes to the admin's model registry if an
error is encountered during autodiscover (thus avoiding spurious
AlreadyRegistered exceptions, but without the need for a global flag).
If I roll back r9680 and apply jarrow's patch instead, both #8245 and
the issue I describe above are solved.

Can anyone shed light on why jarrow's patch was rejected for #8245 (no
reasons are provided in the ticket)? Is there any reason not to switch
to that approach now in order to get more predictable behavior from
admin.py errors under runserver?

Thanks,

Carl

[1] http://code.djangoproject.com/ticket/11957
[2] http://code.djangoproject.com/ticket/8245
[3] http://code.djangoproject.com/changeset/9680

[Carl Meyer]

Anyone have thoughts on this? It would be really nice to get this
fixed so admin.py errors don't break the runserver, and I think the
updated patch on #11957 accomplishes this without any breakage.

Carl

[Chris Beaven]

I haven't looked hard at the fix, but I hit the bug a lot and agree
that it would be great if this was fixed :)