Code

Opened 9 years ago

Closed 9 years ago

Last modified 8 years ago

#583 closed enhancement (fixed)

[patch] Add app-template dirs to TEMPLATE_DIRS

Reported by: sune.kirkeby@… Owned by: adrian
Component: Template system Version:
Severity: normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

This patch automatically adds templates/ directories under app-modules to TEMPLATE_DIRS. This is nice :)

Attachments (4)

settings.patch (1.0 KB) - added by sune.kirkeby@… 9 years ago.
settings-v2.patch (1.6 KB) - added by sune.kirkeby@… 9 years ago.
settings-v3.patch (1.6 KB) - added by sune.kirkeby@… 9 years ago.
Python 2.3 safe patch
apploader.py (889 bytes) - added by hugo 9 years ago.
template loader that looks into app directories

Download all attachments as: .zip

Change History (18)

Changed 9 years ago by sune.kirkeby@…

comment:1 Changed 9 years ago by adrian

In your patch, what does this comment mean?

"There is a good reason i jump through these hoops, if the original TEMPLATE_DIRS setting was a list-instance, we should keep it that way."

comment:2 Changed 9 years ago by adrian

This solution could have some performance issues for installations with a lot of values in INSTALLED_APPS. For that reason, I'm a bit weary of including the patch.

That said, maybe it could be optional, according to a GET_TEMPLATES_IN_APPS setting, which would be set to False by default.

comment:3 Changed 9 years ago by sune.kirkeby@…

"There is a good reason i jump through these hoops, if the original TEMPLATE_DIRS setting was a list-instance, we should keep it that way."

In Django Developer Gadgets I introduced the template_dirs_hacker middleware, it depends on TEMPLATE_DIRS being a list, so it can change it with slice-assignment. That was what I was referring to, sorry if that was a bit obscure for anyone else :)

comment:4 Changed 9 years ago by sune.kirkeby@…

This solution could have some performance issues for installations with a lot of values in INSTALLED_APPS. For that reason, I'm a bit weary of including the patch.

I don't see how; the patch only includes app-templates directories if they actually exist, and is only executed when the settings module is imported initially. But, if you still want it, I can add a GET_TEMPLATES_IN_APPS setting.

comment:5 Changed 9 years ago by adrian

Yeah, I'm super-concerned about keeping startup costs low, so a GET_TEMPLATES_IN_APPS setting would be best.

comment:6 Changed 9 years ago by sune.kirkeby@…

I'm super-concerned about keeping startup costs low,

Hmm... Right. I can't say that I can see why; but it's your call. Here is a new patch, which adds a ADD_APP_TEMPLATE_DIRS setting; I changed the name, to better describe what it does. Also, I would argue that you should change the default value to True, and let the convenience of the common user come before the efficience of initilization for the one-in-thousand user...

Changed 9 years ago by sune.kirkeby@…

comment:7 Changed 9 years ago by eugene@…

I can't say that I can see why

I can say why: FastCGI. Speed of the process creation becomes crucial, especially under heavy load.

Changed 9 years ago by sune.kirkeby@…

Python 2.3 safe patch

comment:8 Changed 9 years ago by adrian

I just chatted about this on IRC with Hugo, and he suggested we use the new TEMPLATE_LOADERS setting (from [870]) to accomplish this -- say, by adding something like django.core.template.loaders.autoapp to that setting. This solution would mean we wouldn't be creating another setting. The problem with this, though, is that the autoapp loader would have to know how to load from the filesystem *and* from eggs, which is redundant. Thoughts?

comment:9 Changed 9 years ago by adrian

  • Status changed from new to assigned

comment:10 Changed 9 years ago by hugo

Hmm. When I played with the template loaders, I just added two template loaders to the list (one the dbtemplate loader, the other the standard template loader). Templates where first searched in the first one and next in the second one if they didn't exist in the first - so just add the egg loader _and_ the autoapp loader would give you exactly what extending the template directories would. I thought that's why the TEMPLATE_LOADERS thingy is a list - to have multiple of them installed. Or am I missing something?

The autoapp thingy would just be a "decentralized file storage" - for example it could have some kind of caching of directory content so that it only searches in the directories when the content is actually changed, that would remove penalties from having many apps (and therefore many template directories to check).

Changed 9 years ago by hugo

template loader that looks into app directories

comment:11 Changed 9 years ago by hugo

I attached a sample implementation of such a app-path-loader. It doesn't do any caching of directory contents, that's left as an exercise for the reader ;-)

comment:12 Changed 9 years ago by adrian

Ah, but your apploader.py example doesn't handle loading templates from eggs, which was my point. I'm considering moving the admin into a standalone app in contrib (see #627), and that would mean the default admin templates would live within the admin app, rather than in django/conf/admin_templates.

This arrangement would need to work with eggs, too (see #596), so that means the auto-app-path loader would need to work with both filesystem- and egg-based templates. So that's why I'm leaning toward a GET_TEMPLATES_IN_APPS setting.

comment:13 Changed 9 years ago by adrian

On second thought, it'd probably be cleaner if it were a template loader rather than a separate setting, because the auto-app thing would have to look for installed apps in both filesystem and eggs either way.

comment:14 Changed 9 years ago by adrian

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

(In [892]) Fixed #583 -- Added app_directories template loader, which searches for templates in 'templates' directory in each INSTALLED_APPS package. It's turned off by default.

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.