Opened 16 years ago

Closed 16 years ago

Last modified 13 years ago

#7922 closed (fixed)

admin.autodiscover() ate my ImportError

Reported by: Jan Rademaker <j.rademaker@…> Owned by: Brian Rosner
Component: contrib.admin Version: dev
Severity: Keywords: ImportError autodiscover
Cc: Brian Rosner Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Just spend some time figuring out what I had done to break the admin interface. As it turned out I had a typo in one of my import statements and this was silenced by autodiscover(). This patch tries to determine if the import failed because admin.py is missing or if the exception was caused by something else.

Attachments (5)

7922.diff (1.6 KB ) - added by Jan Rademaker <j.rademaker@…> 16 years ago.
autodiscover.diff (680 bytes ) - added by Alex Gaynor 16 years ago.
This uses imp to test for the existance of the module.
autodiscover.2.diff (728 bytes ) - added by Alex Gaynor 16 years ago.
Updated, this should work.
autodiscover_getattr_import_option_1.diff (1.5 KB ) - added by oyvind 16 years ago.
Getattr import solution 1
autodiscover_getattr_import_option_2.diff (1.3 KB ) - added by oyvind 16 years ago.
Getattr import solution 2

Download all attachments as: .zip

Change History (18)

by Jan Rademaker <j.rademaker@…>, 16 years ago

Attachment: 7922.diff added

by Alex Gaynor, 16 years ago

Attachment: autodiscover.diff added

This uses imp to test for the existance of the module.

comment:1 by Alex Gaynor, 16 years ago

Triage Stage: UnreviewedAccepted

The technique I use(or testing with imp) can be used to solve similar issues in urlpatterns.

in reply to:  1 comment:2 by Jan Rademaker <j.rademaker@…>, 16 years ago

Replying to Alex:

The technique I use(or testing with imp) can be used to solve similar issues in urlpatterns.

From the imp docs:

    Search for a module.  If path is omitted or None, search for a
    built-in, frozen or special module and continue search in sys.path.
    The module name cannot contain '.'; to search for a submodule of a
    package, pass the submodule name and the package's __path__.

So this won't work for project.app.admin style imports.

comment:3 by Alex Gaynor, 16 years ago

Bah, you are correct, I believe it would be possible to do using imp and passing path, but that may be more trouble than it's worth.

comment:4 by Alex Gaynor, 16 years ago

I lied it should actually be pretty simple

In [39]: imp.find_module('admin', __import__('django.contrib.auth', fromlist=['auth']).__path__)
Out[39]: 
(<open file 'django/contrib/auth/admin.py', mode 'U' at 0xb7896f98>,
 'django/contrib/auth/admin.py',
 ('.py', 'U', 1))

I'm going to update the patch now.

by Alex Gaynor, 16 years ago

Attachment: autodiscover.2.diff added

Updated, this should work.

comment:5 by oyvind, 16 years ago

Fromlist can cause double initialization of modules, see latest patch in #6587 for solution

by oyvind, 16 years ago

Getattr import solution 1

by oyvind, 16 years ago

Getattr import solution 2

comment:6 by Russell Keith-Magee, 16 years ago

milestone: 1.0

The issue of eating imports should be addressed before 1.0; it's not strictly a bug, but it is one of those usability things that deserves attention.

comment:7 by Brian Rosner, 16 years ago

Owner: changed from nobody to Brian Rosner
Status: newassigned

comment:8 by Brian Rosner, 16 years ago

Resolution: fixed
Status: assignedclosed

(In [8174]) Fixed #7922 -- admin.autodiscover() no longer eats ImportErrors for breakfast. Thanks Jan Rademaker and Alex Gaynor for their work on the ticket.

comment:9 by anonymous, 16 years ago

Cc: Brian Rosner added
Needs tests: set
Resolution: fixed
Status: closedreopened

I have a problem because of the changeset 8174

I solve the problem doing any changes:

#before
# django/contrib/admin/init.py

from django.contrib.admin.options import ModelAdmin, HORIZONTAL, VERTICAL
from django.contrib.admin.options import StackedInline, TabularInline
from django.contrib.admin.sites import AdminSite, site

def autodiscover():
    """
    Auto-discover INSTALLED_APPS admin.py modules and fail silently when 
    not present. This forces an import on them to register any admin bits they
    may want.
    """
    import imp
    from django.conf import settings
    for app in settings.INSTALLED_APPS:
        try:
            imp.find_module("admin", app.split("."))
        except ImportError:
            # there is no admin.py in app, skip it.
            continue
        __import__("%s.admin" % app)

#after
# django/contrib/admin/init.py

from django.contrib.admin.options import ModelAdmin, HORIZONTAL, VERTICAL
from django.contrib.admin.options import StackedInline, TabularInline
from django.contrib.admin.sites import AdminSite, site

def autodiscover():
    """
    Auto-discover INSTALLED_APPS admin.py modules and fail silently when 
    not present. This forces an import on them to register any admin bits they
    may want.
    """
#    import imp
    from django.conf import settings
    for app in settings.INSTALLED_APPS:
        try:
#            imp.find_module("admin", app.split("."))
            __import__("%s.admin" % app)
        except ImportError:
            # there is no admin.py in app, skip it.
            continue

please, review the use of the imp library

comment:10 by Brian Rosner, 16 years ago

Resolution: fixed
Status: reopenedclosed

Can you please be specific about what the problem is. You are simply showing how to revert the code. Also please do so in another ticket because the problem was solved, but sounds like a *new* problem has occurred.

comment:11 by Brian Rosner, 16 years ago

Resolution: fixed
Status: closedreopened

Ok, my bad. I made a mistake. anonymous should have been more specific. I have reverted [8174] in [8183]. Looking into it now.

comment:12 by Brian Rosner, 16 years ago

Resolution: fixed
Status: reopenedclosed

(In [8186]) Pass the correct data as the second parameter to find_module to correct admin.autodiscover(). Fixes #7922. Thanks Alex Gaynor for being smarter than me.

comment:13 by Jacob, 13 years ago

milestone: 1.0

Milestone 1.0 deleted

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