Exceptions in admin.py can get hidden by an AlreadyRegistered exception

Consider the following admin.py file:

from django.contrib import admin
from testpro.testapp.models import *

admin.site.register(Screenshot)

raise Exception("Exception that can be much less obvious than this staged one (like a missed import).")

It's loaded by admin.autodiscover() in urls.py.

Now start the development server in debug mode and access the admin. What happens is this:

• On the first request after starting the dev server our test exception is shown as the response.

ImproperlyConfigured at /admin/
Error while importing URLconf 'testpro.urls': Exception that can be much less obvious than this staged one (like a missed import).

• For every subsequent request this exception is the response:

ImproperlyConfigured at /admin/
Error while importing URLconf 'testpro.urls': The model Screenshot is already registered

Needless to say this can be really frustrating to debug if one does not see the first and correct exception by chance.

[Julien Phalip]

This is probably the same issue as in #8118. It would be good to test if it's exclusively an issue with the dev server, or with any server like Apache too.

[Brian Rosner]

This is indeed a duplicate of #8118.

[Brian Rosner]

Wait. I am wrong. My bad.

[Brian Rosner]

Julien, my guess on this problem is that Python re-imports urls.py again on the second request because the exception preventing it from making it to sys.modules. I would suspect this is a problem anywhere Python is. I just don't know how much can be done without changing the behavior of registration, which I disagree should happen. There may either be something I am missing however.

I'll try and write a patch for it and test that on Apache also.

Fixes the bug by unregistering models that were registered in the admin.py file before the exception occured so they can be safely registered again

[Julien Phalip]

Just closed #8118. I believe both tickets are different symptoms of the same problem, and jarrow's patch takes the right approach here.

You want to unregister models that have been registered before the exception was raised, as they will be registered again when that admin.py module is re-loaded in subsequent times.

[Julien Phalip]

Again, it would be worth testing the patch both on the dev server and a regular server. There might be some side effects, but hopefully not.

I tested the problem on Apache 2, it's also present. The fix works on Apache and the dev server.

The test suite is running fine.

This should work as we are only unregistering models that will be re-registered next time anyway and do not touch the rest. This way even "valid" AlreadyRegistered exceptions (with the previous call to register() in the file or outside) should work correctly.

[oyvind]

another solution, would this work?

No, as the patch doesn't compile ;) Unless I'm missing something this line is pseudo-code:

for app in settings.INSTALLED_APPS and app not in valid_apps_admin:

But seriously, it can't work as the patch only makes sure that admin.py files that had no errors are not loaded again. But that that's the case anyway. It does not change anything for the ones that produce exceptions.

[oyvind]

oops, my bad, should be a if in there :)

[oyvind]

this was what i meant :)

Yeah. But that still doesn't fix the problem :) I've tested your patch already corrected like in the second version.

If I understand it correctly python only evaluates the statements in the admin.py files the first time they are imported. So what you do -- making sure that they are not imported twice -- does not change anything. The problem only exists when an exception occurs in an admin.py file. Then python has to re-evaluate that file.

If you see any particular problem with my patch plz post it. The tests run fine but that doesn't mean I can't have missed something ...

[oyvind]

Ah, now I get it, you patch is correct :)

Simply copy the registry before import and restore on exception. This prevents AlreadyRegistered and NotRegistered exceptions.

#8282 also mentions the possibility of NotRegistered exceptions on the subsequent requests. This can occur when unregistering models that were in the registry before the import.

The new patch fixes this by simply copying the registry instead of trying to unregister and register. Re-registering wouldn't be possibly anyway as we don't have the necessary information.

[Gary Wilson]

A different approach using a loading flag as suggested by Malcolm in IRC.

[Malcolm Tredinnick]

What's wrong with just setting the variable to True or False? All you're trying to do is stop the function being run recursively.

[Karen Tracey]

Another person stumbled over this & reported it as #9417.

[Gary Wilson]

(In [9680]) Fixed #8245 -- Added a LOADING flag to autodiscover to prevent an admin.py module with errors from raising a spurious AlreadyRegistered exception in a subsequent call to autodiscover.

[Gary Wilson]

Backported to 1.0.X in r9681.

Cool, thanks for the fix. I think this solution has some drawbacks though:

• You only see the real exception on the first request
• All the stuff getting executed behind the exception in the admin.py file is missing (which could cause other exceptions in turn)
• To get the real exception back and to execute the stuff behind the source the exception one has to restart the server (even the development one)

The previous patch (though admittedly not that pretty) didn't have these issues. Was this the intended behaviour?

[(none)]

Milestone post-1.0 deleted

[Carl Meyer]

#11957 is caused by the fix applied here. Discussion at ​http://groups.google.com/group/django-developers/browse_thread/thread/465a4ed96de40c48