Opened 8 years ago

Closed 8 years ago

Last modified 5 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: UI/UX:

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@…> 8 years ago.
autodiscover.diff (680 bytes) - added by Alex Gaynor 8 years ago.
This uses imp to test for the existance of the module.
autodiscover.2.diff (728 bytes) - added by Alex Gaynor 8 years ago.
Updated, this should work.
autodiscover_getattr_import_option_1.diff (1.5 KB) - added by oyvind 8 years ago.
Getattr import solution 1
autodiscover_getattr_import_option_2.diff (1.3 KB) - added by oyvind 8 years ago.
Getattr import solution 2

Download all attachments as: .zip

Change History (18)

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

Attachment: 7922.diff added

Changed 8 years ago by Alex Gaynor

Attachment: autodiscover.diff added

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

comment:1 Changed 8 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 8 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 8 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 8 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 8 years ago by Alex Gaynor

Attachment: autodiscover.2.diff added

Updated, this should work.

comment:5 Changed 8 years ago by oyvind

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

Changed 8 years ago by oyvind

Getattr import solution 1

Changed 8 years ago by oyvind

Getattr import solution 2

comment:6 Changed 8 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 8 years ago by Brian Rosner

Owner: changed from nobody to Brian Rosner
Status: newassigned

comment:8 Changed 8 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 8 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 8 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 8 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 8 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 5 years ago by Jacob

milestone: 1.0

Milestone 1.0 deleted

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