Code

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#13334 closed (fixed)

Importing template tags from an egg fails

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

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 4 years ago.
Patch with a fix for this issue, needs tests
13334.diff (4.2 KB) - added by kmtracey 4 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 4 years ago by ramiro

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

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.

Changed 4 years ago by ramiro

Patch with a fix for this issue, needs tests

comment:2 Changed 4 years ago by ramiro

  • 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 Changed 4 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 4 years ago by kmtracey

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.

Changed 4 years ago by kmtracey

comment:5 Changed 4 years ago by kmtracey

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

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

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

(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 Changed 4 years ago by kmtracey

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

comment:9 Changed 3 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.