Opened 7 years ago

Closed 5 years ago

#9427 closed (fixed)

admin.autodiscover() cannot locate admin inside an egg

Reported by: clint Owned by: nobody
Component: contrib.admin Version: 1.0
Severity: Keywords: admin autodiscover egg
Cc: metzen@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: UI/UX:

Description

In Ticket #8126, a mechanism was added to admin.autodiscover() to be a little more intelligent about how it found and then tried to import admin.py files in applications.

In a project I'm working on now, we have the following structure (I've simplified this a little, obviously). /vendors/ is on my pythonpath.

myproject/
    settings.py
    myapp/
        admin.py
        models.py
    vendor/
       app1/
          models.py
          admin.py
       app2/
          models.py
          admin.py
       app3.egg/   <-- actual, zipped egg
          models.py
          admin.py
    urls.py
  • When I syncdb, app3 is found and tables are made. I can import it in manage.py shell
  • myapp above relates some models in app3 and when I go look at the admin pages for myapp it shows data from those tables.

So it sounds like most parts of Django are finding this application inside the egg just fine.

However:

  • No admin pages for app3 show up in the admin site

From the above, I figured this was probably an issue in admin.autodiscover() not finding the admin.py. I looked in there and dugg into its code. The app path is discovered just fine by the code on line 25 of source:/django/trunk/django/contrib/admin/__init__.py

The apps that loop finds:

['/Users/phaedo/django_src/django/contrib/admin']
['/Users/phaedo/django_src/django/contrib/auth']
['/Users/phaedo/django_src/django/contrib/contenttypes']
['/Users/phaedo/django_src/django/contrib/sessions']
['/Users/phaedo/django_src/django/contrib/sites']
['/Users/phaedo/django_src/django/contrib/redirects']
['/Users/phaedo/src/ars-django-project/myproject/vendor/template_utils']
['/Users/phaedo/src/ars-django-project/myproject/vendor/debug_toolbar']
['/Users/phaedo/src/ars-django-project/myproject/vendor/dbtemplates']
['/Users/phaedo/src/ars-django-project/myproject/vendor/django_evolution']
['/Users/phaedo/src/ars-django-project/myproject/vendor/django_extensions']
['/Users/phaedo/src/ars-django-project/myproject/vendor/timezones']
['/Users/phaedo/src/ars-django-project/myproject/../myproject']
['/Users/phaedo/src/ars-django-project/myproject/vendor/app3-0.1-py2.5.egg/app3']   <--- there it is!
['/Users/phaedo/src/ars-django-project/myproject/myapp']
['/Users/phaedo/src/ars-django-project/myproject/vendor/signedcookies']

Then on line 34 it uses the method: imp.find_module() to see if an admin.py exists in each of those locations. When it looks into /Users/phaedo/src/ars-django-project/myproject/vendor/app3-0.1-py2.5.egg/app3 the method throws this exception:

ImportError: No frozen submodule named /Users/phaedo/src/ars-django-project/myproject/vendor/app3-0.1-py2.5.egg/app3.admin

and continues on to the next item in the list. BUT -- if I were to let that exception pass and move on to the actual import on line 40:

__import__("%s.admin" % app)

Then it imports just fine! So I can only guess at what's happening here.:

# imp.find_module has some sort of bug in it and can't find modules inside eggs
# If that's the case, perhaps Django should work around this?

It looks to me like everywhere else in Django is finding my application inside this egg and admin.autodiscover is doing something different.

My solution:

Cut out all the stuff happening on lines 34-37 and wrap the actual import an a try/except and catch the ImportError. Why do we need to find it before trying to import it? I've attached a patch to this effect. It seems to be like it would be backwards compatible and work around this strange "bug" in find_module.

Attachments (3)

admin_autodiscover_impfix.patch (1.0 KB) - added by clint 7 years ago.
A patch to remove imp.find_module in admin.autodiscover
admin_autodiscover_impfix2.patch (1.5 KB) - added by clint 7 years ago.
This patch allows errors in admin.py to bubble up
admin_autodiscover_impfix3.patch (2.5 KB) - added by metzen 7 years ago.

Download all attachments as: .zip

Change History (8)

Changed 7 years ago by clint

A patch to remove imp.find_module in admin.autodiscover

comment:1 Changed 7 years ago by clint

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

After talking with Brian Rosner and Alex Gaynor in IRC just now, we need to do exception checking to make sure that errors inside admin.py aren't gobbled up. I have attached a new patch that attempts to import admin.py first, and then if that raised an ImportError, we check to see if the admin.py exists. If an ImportError was raised and we find that file to exist, we raise the error, otherwise continue.

Changed 7 years ago by clint

This patch allows errors in admin.py to bubble up

comment:2 Changed 7 years ago by anonymous

  • Cc metzen@… added

comment:3 Changed 7 years ago by bracki

  • Keywords admin autodiscover egg added
  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 7 years ago by metzen

admin_autodiscover_impfix2.patch still does not let an ImportError within your egg'ed app's admin module to bubble up. I've taken a crack at a solution for this, and also supplied some additional comments to clarify things a bit.

See admin_autodiscover_impfix3.patch

Changed 7 years ago by metzen

comment:5 Changed 5 years ago by kmtracey

  • Resolution set to fixed
  • Status changed from new to closed

(In [12989]) Fixed #9427: Allow for autodiscover to load admin modules from apps in eggs. Thanks clint and metzen.

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