Opened 10 years ago

Closed 10 years ago

Last modified 7 years ago

#7922 closed (fixed)

admin.autodiscover() ate my ImportError

Reported by: Jan Rademaker <j.rademaker@…> Owned by: Brian Rosner
Component: contrib.admin Version: master
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@…> 10 years ago.
autodiscover.diff (680 bytes) - added by Alex Gaynor 10 years ago.
This uses imp to test for the existance of the module.
autodiscover.2.diff (728 bytes) - added by Alex Gaynor 10 years ago.
Updated, this should work.
autodiscover_getattr_import_option_1.diff (1.5 KB) - added by oyvind 10 years ago.
Getattr import solution 1
autodiscover_getattr_import_option_2.diff (1.3 KB) - added by oyvind 10 years ago.
Getattr import solution 2

Download all attachments as: .zip

Change History (18)

Changed 10 years ago by Jan Rademaker <j.rademaker@…>

Attachment: 7922.diff added

Changed 10 years ago by Alex Gaynor

Attachment: autodiscover.diff added

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

comment:1 Changed 10 years ago by Alex Gaynor

Triage Stage: UnreviewedAccepted

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

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

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 Changed 10 years ago by Alex Gaynor

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 Changed 10 years ago by Alex Gaynor

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.

Changed 10 years ago by Alex Gaynor

Attachment: autodiscover.2.diff added

Updated, this should work.

comment:5 Changed 10 years ago by oyvind

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

Changed 10 years ago by oyvind

Getattr import solution 1

Changed 10 years ago by oyvind

Getattr import solution 2

comment:6 Changed 10 years ago by Russell Keith-Magee

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 Changed 10 years ago by Brian Rosner

Owner: changed from nobody to Brian Rosner
Status: newassigned

comment:8 Changed 10 years ago by Brian Rosner

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 Changed 10 years ago by anonymous

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 Changed 10 years ago by Brian Rosner

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 Changed 10 years ago by Brian Rosner

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 Changed 10 years ago by Brian Rosner

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 Changed 7 years ago by Jacob

milestone: 1.0

Milestone 1.0 deleted

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