admin.autodiscover() uses method that cannot find files inside of eggs: possible fix included

[Clint Ecker]

Hi all,
I've opened a ticket on this matter (http://code.djangoproject.com/
ticket/9427) but the gist of it is this:

I have a django application that is packaged as an egg. Inside this
package are many files which include an admin.py. It would appear
that the rest of django finds the files inside (models.py for one)
because syncdb makes the tables and so forth. However, the admin
application does not show anything for this application. When the
application exists as a normal folder-style package, all is well.

As you can see in the ticket, I dug around contrib/admin/__init__.py
and debugged what it was doing there. What it looks like is happening
is that imp.find_module() cannot find any file inside eggs. Calling a
regular __import__ on the admin file inside that app works fine
though.

So what to do here? As I can see it, autodiscover does two things:
finds out the paths to where applications live and then imports their
admin.py if it exists. Since we (apparently) cannot reliably
determine if an admin.py exists inside an application under all
circumstances using imp.find_module, we could just try to import the
admin.py and raise an error if it doesn't exist. I think the larger
issue here is in imp and Alex agrees that its probably an upstream
issue but:

#1. This egg packaged app worked everywhere else in Django as far as
I can tell
#2. If this is indeed a problem in a major version of Python (2.5),
shouldn't we work around the issue as long as it keeps backwards
compatibility?

But what if there are errors inside the admin.py? We need to bubble
those up.

So after talking with Alex Gaynor and Brian Roser in #django-dev I
have attached a patch that essentially attempts to import the admin.py
in the application. If that fails, it checks to see if that file
exists. If it exists, we throw an exception because something is
obviously wrong. If the import failed and the file doesn't exist,
then we just move on.

I'm throwing this out there just to get a wider eye on the patch just
in case there are any edge cases that these changes might affect.

Again the ticket is http://code.djangoproject.com/ticket/9427

Thanks!

--
Clint Ecker -- http://blog.clintecker.com
c: 312.863.9323
---
twitter: clint
skype: clintology
AIM: clintecker
Gtalk: clint...@gmail.com

[Waylan Limberg]

On Wed, Oct 22, 2008 at 7:16 PM, Clint Ecker <clint...@gmail.com> wrote:
>
> Hi all,
> I've opened a ticket on this matter (http://code.djangoproject.com/
> ticket/9427) but the gist of it is this:
>

[snip]

>
> I'm throwing this out there just to get a wider eye on the patch just
> in case there are any edge cases that these changes might affect.
>

What about an admin.py in an egg that legitimately raises an error on
import? If imp.find_module can't find it as you claim, won't that
error be eaten? Now, I realize that should probably be caught while
testing not in an egg first, but it is a real possibility. Perhaps we
shouldn't care about that one though.

--
----
Waylan Limberg
way...@gmail.com

[alex....@gmail.com]

Waylan, while discussing this with Clint, I came to the conclusion
that we will still face this problem with this solution. However,
IMO, that is indicative of an upstream problem and should be reported
up stream, once it is fixed there this solution will continue to work
correctly for both regular modules and eggs.

Alex

On Oct 23, 12:20 am, "Waylan Limberg" <way...@gmail.com> wrote:

[Kurt]

Could always use the super hacky "check the ImportError message for
the name of the admin module" to decide whether to reraise the error
or not.

On Oct 22, 11:37 pm, "alex.gay...@gmail.com" <alex.gay...@gmail.com>
wrote:

[Ludvig Ericson]

On Oct 23, 2008, at 16:53, Kurt wrote:

>
> Could always use the super hacky "check the ImportError message for
> the name of the admin module" to decide whether to reraise the error
> or not.

Please don't.

The solution lies within the fourth argument to __import__.

The statement
from x.y import z
equates to
z = __import__("x.y", {}, {}, ["z"]).z
(I think you can guess what " as q" changes.)

So, what we want the autodiscover to do is to run
from <appname> import admin
which would be
mod = __import__(appname, {}, {}, "admin")

And that should be enough, since the admin registering machinery
should caretake the rest-- the module has been imported. Proof:
>>> __import__("django.core", {}, {}, ["files"]).files
<module 'django.core.files' from '.../django/core/files/__init__.pyc'>
-- Restart CPython --
>>> __import__("django.core", {}, {}, [""]).files
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: 'module' object has no attribute 'files'

Hope that made sense,
Ludvig

[Ludvig Ericson]

> So, what we want the autodiscover to do is to run
> from <appname> import admin
> which would be
> mod = __import__(appname, {}, {}, "admin")

I should proof-read more.

from <appname> import admin
is the same as
admin = __import__(appname, {}, {} ["admin"]).admin
_but_ our interest lies within executing the code, so this is enough:
__import__(appname, {}, {} ["admin"])
Without any try/except clause, because Python gives no error if you
specify a from_name (fourth argument) which doesn't exist. So that's
really all you need.