Opened 14 years ago

Closed 14 years ago

Last modified 13 years ago

#13334 closed (fixed)

Importing template tags from an egg fails

Reported by: joerg86 Owned by: nobody
Component: Template system Version: dev
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hi,

it seems that from r12944, django cannot deal with template tags and filters imported from egg files. I ran into this as I tried to use sorl-thumbnail 3.2.5 - django gives me a template syntax error saying the "thumbnail" template tag couldn't be found in sorl.thumbnail.templatetags.thumbnail and others.
If I put the same module uncompressed in my pythonpath, everything works.

Thanks for fixing it,

Jörg

Attachments (2)

13334-12970.diff (1.8 KB ) - added by Ramiro Morales 14 years ago.
Patch with a fix for this issue, needs tests
13334.diff (4.2 KB ) - added by Karen Tracey 14 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 by Ramiro Morales, 14 years ago

See #9427 the third attachment there proposes even another idiom to differentiate between ImportError's generated because a module doesn't exist and ImportError's generated by the module itself but that unlike imp.find_module() is compatible with eggs.

by Ramiro Morales, 14 years ago

Attachment: 13334-12970.diff added

Patch with a fix for this issue, needs tests

comment:2 by Ramiro Morales, 14 years ago

Has patch: set
Needs tests: set

We need to work on regression test for loading a module from an egg (this issue) and #13311 / [12944].

comment:3 by Russell Keith-Magee, 14 years ago

Triage Stage: UnreviewedAccepted

comment:4 by Karen Tracey, 14 years ago

Does the attached patch really work for the template tag case? In trying the approach from that 3rd patch on #9427 for #13348, I could not get it to work. sys.path_importer_cache appears to contain stuff under keys slightly different from what is expected there. For example if I load an app from an egg:

>>> import sys
>>> from django.utils.importlib import import_module
>>> mod = import_module('app_with_models')
>>> mod
<module 'app_with_models' from '/home/kmt/software/web/playground/modelapp.egg/app_with_models/__init__.py'>
>>> mod.__file__
'/home/kmt/software/web/playground/modelapp.egg/app_with_models/__init__.py'
>>> mod.__path__
['/home/kmt/software/web/playground/modelapp.egg/app_with_models']

mod.__path__[0] contains both the egg file name and then the package name. This is not found in sys.path_importer_cache:

>>> sys.path_importer_cache[mod.__path__[0]]
Traceback (most recent call last):
  File "<console>", line 1, in <module>
KeyError: '/home/kmt/software/web/playground/modelapp.egg/app_with_models'

Instead the key you need to use is one with just the full path of the egg:

>>> sys.path_importer_cache['/home/kmt/software/web/playground/modelapp.egg']
<zipimporter object "/home/kmt/software/web/playground/modelapp.egg">

Which seems to be more easily found via mod.__loader__:

>>> mod.__loader__
<zipimporter object "/home/kmt/software/web/playground/modelapp.egg">

Then, once you have that thing, its find_module is not quite equivalent to imp.find_module. It (at least the zipimport one) has differences deeper than whether it raises an ImportError or returns None on failure to find the requested submodule.

The imp version does not handle hierarchical (dotted) names in its first argument -- so we pass in 'models', for example, as the 1st arg and the module's path list as the 2nd arg and 'models' is found by imp.find_module.

The zipimport version on the other hand requires a heirarchical path in its first arg, and ignores its 2nd arg. It claims it wants dotted path names in the first arg but the only thing that seems to actually work is os.sep:

>>> mod.__loader__.find_module('models')
>>> mod.__loader__.find_module('app_with_models.models')
>>> mod.__loader__.find_module('app_with_models/models')
<zipimporter object "/home/kmt/software/web/playground/modelapp.egg">

(On Windows that last would not work, what would work would be mod.__loader__.find_module('app_with_models\\models').)

Given that behavior, I'm having a hard time believing that the approach in the patch here really works? But I could be missing something...?

I put a patch on #13348 that attempts to create a general utility function for answering the question of whether a loaded module has a named submodule. At any rate it works in my testing for finding (or not) models modules in apps loaded from eggs.

by Karen Tracey, 14 years ago

Attachment: 13334.diff added

comment:5 by Karen Tracey, 14 years ago

Attached a patch with tests that uses the utility function from the patch on #13348.

While the earlier patch works to load valid tags from eggs, the case where it does not work is where there is an egg with a templatetags module, but attempting to import it raises an ImportError. With the earlier patch this case gets reported indicating that the tag library could not be found ("'broken_egg' is not a valid tag library: Template library broken_egg not found, tried django.templatetags.broken_egg,tagsegg.templatetags.broken_egg") instead of properly indicating what the real problem is ("'broken_egg' is not a valid tag library: ImportError raised loading tagsegg.templatetags.broken_egg: cannot import name Xtemplate").

This patch also changes from raising a bare ImportError to TemplateSyntaxError. That makes the debug page include a the snippet from the template that has caused the problem, and restores much of the text that used to be in the error message for this case (minus the misleading identification of where the templatetags module was supposedly located. The text of the message could maybe be improved but I'm too tired for that just now.)

comment:6 by Karen Tracey, 14 years ago

One last thing -- the earlier patch took the approach of trying the import first, and only if that failed did it attempt to ascertain if the module even had the requested submodule. That may be a better approach than the way that seemed more natural to me: figure out if the module has the requested submodule, and only if it does try the import. The reason I think the try-the-import first approach might be better is that the function to figure out if the module has the submodule may be fragile. I've tested it with eggs and and regular modules, but if there are other loaders out there that do not exactly match what the zipimporter one does, then that routine may not come up with the right answer. If we try the import first and only check for the submodule if that fails, the effect of getting a wrong answer on whether the submodule exists is limited to an error-handling path. With the reverse we stand a chance of refusing to try to import something that may import just fine, simply because we couldn't properly figure out if the submodule exists in the module.

comment:7 by Karen Tracey, 14 years ago

Resolution: fixed
Status: newclosed

(In [12986]) Fixed #13334: Restored ability to load template tags from eggs. Again thanks Ramiro and metzen for pointers on how to find out if a module loaded from an egg has a particular submodule, and Russ for review.

comment:8 by Karen Tracey, 14 years ago

(In [12988]) Add file mistakenly left out of r12986. Refs #13334.

comment:9 by Jacob, 13 years ago

milestone: 1.2

Milestone 1.2 deleted

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