Hi Aymeric, It took me awhile, but I haz an update:
https://github.com/django/django/pull/3475 Loader cleanup This addresses the code duplication between get_internal and load_template_sources. I also made the app directories loader inherit from the filesystem loader, since the only real difference between the two are the directories they look in. Cached loader The cached loader didn't clean up so nicely. I thought I could remove the custom load_template implementation, but a problem arises if the cached loader is configured with both recursive and non-recursive loaders. Therefore, I kept the old implementation in place for now. Updating the cached loader was a bit tricky. The old loader caches templates and TemplateDoesNotExist exceptions with a key defined by template_name and template_dirs. This isn't as straightforward when the skip argument is added to the equation. The options were then to create a custom cache key per skip argument, or defer caching to the lower-level methods. During extending, skip always includes the originating template. That would lead to many more cache misses, so I chose to do the latter instead. When skip isn't provided, I use the simple algorithm. There may be a simpler approach, here. I've done some performance testing without noticing much difference, but this is something I'll test some more. I also experimented with using lru_cache instead of local dictionaries. This simplifies the implementation but is measurably slower. Origin hash The cache algorithm I added uses the origin object as a cache key. The "name" attribute is used as part of the key value. Therefore, the underlying loader must be sure to set name to a unique value per template. You mention above that "name" must be optional for loaders that don't load templates from the filesystem, but I'm not sure this is true. Either name, or another attribute on origin, needs to uniquely identify the template within a loader. Otherwise, there's not a way to implement __eq__ and __hash__. The locmem loader is a non-filesystem loader that is able to do this. I also implemented a sample db loader that does as well: https://github.com/prestontimmons/dbloader/blob/master/dbloader/loader.py Can you think of a loader where we wouldn't want to require a unique name? Egg and db loader origin Some loaders use different identifiers for templates. The filesystem loaders are just file paths and the locmem loader is just a string key, but the egg loader is a tuple, (app_config.name, pkg_name) and the sample db loader is the model instance. The egg and db loaders don't have an obvious attribute on origin to save the extra information to. You'll see in those cases that I subclassed origin as EggOrigin and DbOrigin. Does that makes sense, or should I look for a way to incorporate that into LoaderOrigin? LoaderOrigin vs StringOrigin I looked at combining these, but didn't yet since it doesn't impact the feature this patch is implementing. If they're combined to a DTLOrigin, should that live in django.template.base? It can't live in django.template.loader since that module isn't safe for django.template.base to import. context.origin In order for the origin to be available to the ExtendsNode, I also used the context.origin = self.origin hack in the Context.render method. I saw the logic you used to only set engine on toplevel_render. When I moved this assignment into the if statement or not, it didn't seem to make a difference. Is that a hack we can live with for now? Debug view This branch includes an update to the debug views. The best way to view the output is to run the debug page in a browser. I added a sample project here with various scenarios: https://github.com/prestontimmons/project-15053 There's one case where the debug postmortem isn't optimal. If multiple engines are configured and a template is found by the second engine that fails extending, the postmortem won't include the templates that were tried by the initial get_template call to the first engine. I'm not seeing an obvious way to persist that information at the moment. Things to do The next things I plan to do is measure the cached loader performance more closely and add the docs. -- Preston -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/5aad6c45-70eb-45cb-92d9-4e975e3dc50a%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
