Fixing app_label

291 views
Skip to first unread message

Jacob Kaplan-Moss

unread,
Dec 5, 2007, 12:55:11 PM12/5/07
to django-d...@googlegroups.com
Howdy folks --

I just had a nasty realization here at work: we're gonna need to fix
app_label pretty damn quick. The good news is that this means we'll
get to fix this problem on work time; the bad news is that I'm totally
unsure how to start.

For those who haven't been following along, the main problem is that
app_label must be unique across all installed apps, so you can't have,
say, an "ellington.search" and a "marketplace.search" app -- both
would be known by Django as "search", and Django would get all
confused.

http://code.djangoproject.com/ticket/3591 has been following this
problem (and the other, related ones). The patches there look
relatively good and the tests appear to pass, but I'm concerned about
the level of boat-rocking this patch introduces (it basically redoes
the entire app/model loading mechanism in the process).

I know that at least a couple other committers -- Malcolm, Joseph --
have looked at this in the past, as have a number of developer
community members.

So what I'm looking for here is some insight from folks that have
already looked at this ticket. What are the with the current patch?
Are there simpler ways? What types of breakage should I be looking
for? Is there anything out on one of the branches that I'm going to
fubar if I start working on this?

Thoughts, anyone?

Jacob

Marty Alchin

unread,
Dec 5, 2007, 1:27:10 PM12/5/07
to django-d...@googlegroups.com
On Dec 5, 2007 12:55 PM, Jacob Kaplan-Moss <jacob.ka...@gmail.com> wrote:
> http://code.djangoproject.com/ticket/3591 has been following this
> problem (and the other, related ones). The patches there look
> relatively good and the tests appear to pass, but I'm concerned about
> the level of boat-rocking this patch introduces (it basically redoes
> the entire app/model loading mechanism in the process).
>
> I know that at least a couple other committers -- Malcolm, Joseph --
> have looked at this in the past, as have a number of developer
> community members.
>
> So what I'm looking for here is some insight from folks that have
> already looked at this ticket. What are the with the current patch?
> Are there simpler ways? What types of breakage should I be looking
> for? Is there anything out on one of the branches that I'm going to
> fubar if I start working on this?

I was just thinking about this ticket a few minutes ago, actually,
while revisiting the though of dynamic models. I had looked at it some
time ago, for a vaguely related issue (#4144), but I didn't really
need the whole thing, so I didn't do much digging.

Without looking too much at the patch so far, I'd say to consider
whether or not backwards-compatibility is an essential goal. An
INSTALLED_APPS upgrade is on Adrian's 1.0 hitlist, which means (to me)
that some amount of backwards-incompatibility is allowed, as long as
it's done soon. Of course, the backwards-compatibility portion of the
patch looks to be minimal, so it might be just as easy to leave it in.

Also, speaking of Adrian, the way he worded it ("Make INSTALLED_APPS
an instance, and each app an instance") sounds slightly different than
how I understand the patch thus far. I don't think there's anything
making INSTALLED_APPS an instance of anything, it's still a list as
far as I know. So Adrian might have some of his own ideas about how it
should work that would be worth investigating.

That wording also tends to support the idea that you're allowed to
break backwards-compatibility for people tracking trunk. They're
supposed to be paying attention to all this and/or the Wiki anyway, so
this would be just another one-off post-0.96 code change to comply
with a recent checkout. And since it's only a small section of one
file, I think it's even more reasonable than more far-reaching changes
like unicode or auto-escaping.

Anyway, I'll need to look into this patch for my own sanity anyway, so
if I come up with any other thoughts on it, I'll definitely voice
them. For the most part though, let's hope Vinay Sajip is paying
attention, as his thoughts would be of great use, I'm sure.

-Gul

Adrian Holovaty

unread,
Dec 5, 2007, 2:48:48 PM12/5/07
to django-d...@googlegroups.com
On Dec 5, 2007 11:55 AM, Jacob Kaplan-Moss <jacob.ka...@gmail.com> wrote:
> http://code.djangoproject.com/ticket/3591 has been following this
> problem (and the other, related ones). The patches there look
> relatively good and the tests appear to pass, but I'm concerned about
> the level of boat-rocking this patch introduces (it basically redoes
> the entire app/model loading mechanism in the process).

Great to hear this is gaining some momentum. I'm not a fan of the
patch on 3591, as it goes only halfway (by adding the app() function),
stopping short of a full, elegant solution. The approach I've been
wanting to take is to make INSTALLED_APPS an instance of an
AppCollection class (insert better name here), which is a collection
of App instances, each of which has a configurable label (and other
configuration). With that in place, any code that needs to iterate
over the INSTALLED_APPS will simply do something like this:

from django.conf import settings
for app in settings.INSTALLED_APPS:
print app.label
print app.verbose_name # Finally apps can have a human-readable name!
# print whatever else...

The settings file syntax could look like this:

INSTALLED_APPS = AppCollection(
App('django.contrib.admin'),
App('ellington.news'),
)

Maybe *args would be interpreted as App classes and **kwargs could be
for AppCollection-level options, like this:

INSTALLED_APPS = AppCollection(
some_option=True,
App('django.contrib.admin'),
)

I'm not sure what options would be available to an AppCollection, but
at least this gives us the flexibility for options in the future.

This could be fully backwards-compatible. If INSTALLED_APPS is a
tuple/list of strings, then we convert it to an AppCollection
automatically.

I'm very willing to help implement this, as it's been on my to-do list
(and the ticket is assigned to me).

Adrian

--
Adrian Holovaty
holovaty.com | djangoproject.com

Adrian Holovaty

unread,
Dec 5, 2007, 2:52:46 PM12/5/07
to django-d...@googlegroups.com
On Dec 5, 2007 1:48 PM, Adrian Holovaty <holo...@gmail.com> wrote:
> The settings file syntax could look like this:
>
> INSTALLED_APPS = AppCollection(
> App('django.contrib.admin'),
> App('ellington.news'),
> )

Whoop, I stopped short of saying this explicitly, but this is how
you'd specify a custom app_label:

INSTALLED_APPS = AppCollection(
App('django.contrib.admin'),

App('ellington.news', label='foo'),
)

Ivan Sagalaev

unread,
Dec 5, 2007, 3:10:59 PM12/5/07
to django-d...@googlegroups.com
Adrian Holovaty wrote:
> The settings file syntax could look like this:
>
> INSTALLED_APPS = AppCollection(
> App('django.contrib.admin'),
> App('ellington.news'),
> )

Why not also use actual module objects here instead of strings like with
view functions in urlconfs?

Joseph Kocherhans

unread,
Dec 5, 2007, 3:16:44 PM12/5/07
to django-d...@googlegroups.com

I don't see the benefit. It makes sense for views so you can apply
decorators to them in your urls.py file. I'm not sure what you'd want
to do with a module. The less you actually import into your settings
file, the better IMO.

Joseph

flo...@gmail.com

unread,
Dec 5, 2007, 3:35:12 PM12/5/07
to Django developers
Joseph makes a good point about not importing much in the settings
file, so maybe one solution is not to do it in the settings file. We
could treat it like urls. That is, INSTALLED_APPS = 'myproject.apps',
and myproject/apps.py can contain the imports, etc. The nice thing
there is that now we could introduce an include() element which would
include all of the apps from some other path. It would be a little
clunkier to implement backwards compatibility this way though.

Just a thought,
Eric Florenzano

Jacob Kaplan-Moss

unread,
Dec 5, 2007, 3:59:17 PM12/5/07
to django-d...@googlegroups.com
Hey Adrian --

I haven't had time to fully think through your idea -- there's
something about it that rubs me the wrong way, but I can't put my
finger on it -- but I do have one question/idea:

On 12/5/07, Adrian Holovaty <holo...@gmail.com> wrote:
> This could be fully backwards-compatible. If INSTALLED_APPS is a
> tuple/list of strings, then we convert it to an AppCollection
> automatically.

Would you be OK letting this be some sort of a mixed-content thing? I
think it would go a long way towards making the change less disruptive
if INSTALLED_APPS was still a list but allowed objects in it::

INSTALLED_APPS = [
'some.app',
app('django.contrib.admin', verbose_name="Admin")
]

I'll try to figure out what about AppCollection makes me uncomfortable
and get back to you...

Jacob

Jacob Kaplan-Moss

unread,
Dec 5, 2007, 4:00:55 PM12/5/07
to django-d...@googlegroups.com
On 12/5/07, Ivan Sagalaev <Man...@softwaremaniacs.org> wrote:
> Why not also use actual module objects here instead of strings like with
> view functions in urlconfs?

It's *extremely* useful to be able to inspect settings files *without*
triggering imports of all INSTALLED_APPS -- I've got at least a dozen
utilities that operate on multiple sites by introspecting various
settings modules.

IOW, settings modules should always be importable, even if some
particular app referenced is broken.

Jacob

Marty Alchin

unread,
Dec 5, 2007, 4:09:58 PM12/5/07
to django-d...@googlegroups.com
On Dec 5, 2007 4:00 PM, Jacob Kaplan-Moss <jacob.ka...@gmail.com> wrote:
> It's *extremely* useful to be able to inspect settings files *without*
> triggering imports of all INSTALLED_APPS -- I've got at least a dozen
> utilities that operate on multiple sites by introspecting various
> settings modules.
>
> IOW, settings modules should always be importable, even if some
> particular app referenced is broken.

Not only that, but there are logistical problems with importing an app directly.

For starters, importing the app-level module itself offers little
benefit. Its namespace wouldn't have access to any modules inside the
app (such as models, urls and views), so we'd end up just having to
use its __name__ attribute and import those by hand the way we do now,
so it would just add an extra step to the process.

On top of that, what if an app's module makes a decision based on a
particular setting? It would have to import settings, and if it's
being imported from settings already, that's a potentially nasty
circular import just waiting to happen.

Worse yet, if we solve the first problem by importing the 'models'
module directly, and use that as the app's module (as Django already
does internally), we're *guaranteed* to have circular imports, as the
"from django.db import models" line at the top of all 'models' modules
triggers a check for settings.DATABASE_ENGINE (at least). And, of
course, there's the ugliness of having all the 'models' modules having
to be renamed at the top of your settings file:

from django.contrib.auth import models as auth
from django.contrib.sessions import models as sessions
from django.contrib.admin import models as admin
from django.contrib.comments import models as comments

I just see a great many problems, and no real benefits.

-Gul

Marty Alchin

unread,
Dec 5, 2007, 4:02:01 PM12/5/07
to django-d...@googlegroups.com
On Dec 5, 2007 2:48 PM, Adrian Holovaty <holo...@gmail.com> wrote:
> Great to hear this is gaining some momentum. I'm not a fan of the
> patch on 3591, as it goes only halfway (by adding the app() function),
> stopping short of a full, elegant solution. The approach I've been
> wanting to take is to make INSTALLED_APPS an instance of an
> AppCollection class (insert better name here), which is a collection
> of App instances, each of which has a configurable label (and other
> configuration). With that in place, any code that needs to iterate
> over the INSTALLED_APPS will simply do something like this:
>
> from django.conf import settings
> for app in settings.INSTALLED_APPS:
> print app.label
> print app.verbose_name # Finally apps can have a human-readable name!
> # print whatever else...

Not that it would matter for most people, but would that mean you
could install the same app twice, under different names? For instance,
when building an online game, you might have a Places app to keep
track of places in an in-game world, but you'd also like to use it to
store actual player locations (in the real world). You clearly
wouldn't want these two to mix, so it might be useful to have
something like this:

INSTALLED_APPS = AppCollection(
App('basic.places', label='game_places'),
App('basic.places', label='real_places'),
)

Then, the models in the Places app would be installed twice, once for
each label, using the label as a prefix. That's probably too much of
an edge case to go out of our way for, but if it just happened to be
supported, it could make for something interesting things. Yes, I know
it's a pony.

> Maybe *args would be interpreted as App classes and **kwargs could be
> for AppCollection-level options, like this:
>
> INSTALLED_APPS = AppCollection(
> some_option=True,
> App('django.contrib.admin'),
> )

Surely you meant to have the positional arguments come before the
keyword arguments. ;) I wonder if it would also be useful to allow
additional AppCollection instances in addition to App instances. I
keep thinking about Satchmo, which has several apps, most of which are
necessary for a fully-functional store. If they could provide an
importable AppCollection instance, it'd be nice if we could include
that alongside the rest of our apps.

from satchmo import apps

INSTALLED_APPS = AppCollection(
App('django.contrib.admin'),

apps.full_store,
)

But that's probably too much of an off-shoot request, especially since
it would introduce another import into settings.py

> I'm not sure what options would be available to an AppCollection, but
> at least this gives us the flexibility for options in the future.

Even not supporting **kwargs at all right now still gives us the
flexibility for options in the future. We can just add ", **kwargs" to
AppCollection.__init__ and away we go. It wouldn't cause any breakage
to add that later on. But I like the idea, even though I can't think
of any real use for it yet, either.

-Gul

Ivan Sagalaev

unread,
Dec 5, 2007, 4:49:13 PM12/5/07
to django-d...@googlegroups.com
Jacob Kaplan-Moss wrote:
> It's *extremely* useful to be able to inspect settings files *without*
> triggering imports of all INSTALLED_APPS

Got it.

Amit Upadhyay

unread,
Dec 6, 2007, 6:24:04 AM12/6/07
to django-d...@googlegroups.com
On Dec 6, 2007 1:46 AM, Joseph Kocherhans <jkoch...@gmail.com> wrote:
> Why not also use actual module objects here instead of strings like with
> view functions in urlconfs?

I don't see the benefit. It makes sense for views so you can apply
decorators to them  in your urls.py file. I'm not sure what you'd want
to do with a module. The less you actually import into your settings
file, the better IMO.


Apps can have their own middlewares. Though it can still be handled without imports.  

INSTALLED_APPS = AppCollection(
   App('foo.blog', middlewares=['example.mymiddleware']),
)

--
Amit Upadhyay
Vakow! www.vakow.com
+91-9820-295-512

Malcolm Tredinnick

unread,
Dec 7, 2007, 3:25:03 AM12/7/07
to django-d...@googlegroups.com

On Wed, 2007-12-05 at 11:55 -0600, Jacob Kaplan-Moss wrote:
[...]

> So what I'm looking for here is some insight from folks that have
> already looked at this ticket. What are the with the current patch?
> Are there simpler ways? What types of breakage should I be looking
> for? Is there anything out on one of the branches that I'm going to
> fubar if I start working on this?

Jumping in a little late here, but the things I remember thinking about
(and discussing briefly with Adrian when he described his plan to me) is
that app_label collisions are often only the tip of the iceberg in some
respect. They also tend to be an indicator of upcoming template
directory name collisions -- for example if you use the
templates/<app_name>/* convention -- and custom templatetag name
collisions.

None of those problems are in any way insurmountable, but we need to
think through what will work practically for those situations. For
example, the template app_loader, if it remains, should not use the
"label", in Adrian's scheme, to load the template because that would
mean an application cannot load its own templates (it won't know what
its own label will be because that is set by the user at installation
time and unknown to the app writer). I think we're probably fine here:
app_loader continues to just merge all template directories at the top
level and application writers still strive for unique names for their
template directories (or stop using app_loader, which is something I've
been tending towards more and more lately).

I haven't thought about this problem a lot lately, since I knew Adrian,
Vinay and Joseph were all thinking about it and was happy to leave it in
their capable hands.

Regards,
Malcolm

--
Works better when plugged in.
http://www.pointy-stick.com/blog/

Forest Bond

unread,
Dec 7, 2007, 7:09:15 AM12/7/07
to django-d...@googlegroups.com
Hi,

On Fri, Dec 07, 2007 at 07:25:03PM +1100, Malcolm Tredinnick wrote:
> (or stop using app_loader, which is something I've been tending towards more
> and more lately).

And how does one do that?

-Forest
--
Forest Bond
http://www.alittletooquiet.net

signature.asc

Empty

unread,
Dec 7, 2007, 10:56:09 AM12/7/07
to django-d...@googlegroups.com
On Dec 7, 2007 7:09 AM, Forest Bond <for...@alittletooquiet.net> wrote:
> Hi,
>
> On Fri, Dec 07, 2007 at 07:25:03PM +1100, Malcolm Tredinnick wrote:
> > (or stop using app_loader, which is something I've been tending towards more
> > and more lately).
>
> And how does one do that?
>

Just remove 'django.template.loaders.app_directories.load_template_source',
from the TEMPLATE_LOADERS list in your settings.py module.

Then all of your templates would be found via the file system template loader.

By the way, I also do not use app loader.

Michael Trier
blog.michaeltrier.com

Forest Bond

unread,
Dec 7, 2007, 11:09:48 AM12/7/07
to django-d...@googlegroups.com
Hi,

On Fri, Dec 07, 2007 at 10:56:09AM -0500, Empty wrote:
> On Dec 7, 2007 7:09 AM, Forest Bond <for...@alittletooquiet.net> wrote:
> > On Fri, Dec 07, 2007 at 07:25:03PM +1100, Malcolm Tredinnick wrote:
> > > (or stop using app_loader, which is something I've been tending towards
> > > more and more lately).
> >
> > And how does one do that?
> >
>
> Just remove 'django.template.loaders.app_directories.load_template_source',
> from the TEMPLATE_LOADERS list in your settings.py module.
>
> Then all of your templates would be found via the file system template loader.

Oh, right. Misunderstood the original statement. Sorry about that.

signature.asc

Vinay Sajip

unread,
Dec 7, 2007, 12:39:52 PM12/7/07
to Django developers

On Dec 5, 7:48 pm, "Adrian Holovaty" <holov...@gmail.com> wrote:
> On Dec 5, 2007 11:55 AM, Jacob Kaplan-Moss <jacob.kaplanm...@gmail.com> wrote:
>
> Great to hear this is gaining some momentum. I'm not a fan of the
> patch on 3591, as it goes only halfway (by adding the app() function),
> stopping short of a full, elegant solution. The approach I've been
> wanting to take is to makeINSTALLED_APPSaninstanceof an
> AppCollection class (insert better name here), which is a collection
> of App instances, each of which has a configurable label (and other
> configuration). With that in place, any code that needs to iterate
> over theINSTALLED_APPSwill simply do something like this:

The current patch only goes halfway in order to preserve backward
compatibility. If backward compatibility *can* be sacrificed, then it
might be better to force all entries in INSTALLED_APPS to be App
instances. I can't see a clear need for an AppCollection class other
than future -proofing - but introducing it now will definitely break
backward compatibility, won't it? Perhaps we should think of what
functionality would go into an AppCollection. I also think that
AppCollection will conceptually overlap with AppCache - both are
collections of installed apps in a Django instance. Why can't we just
expose AppCache which is already there anyway?

My $0.02,

Vinay Sajip

Vinay Sajip

unread,
Dec 7, 2007, 12:56:06 PM12/7/07
to Django developers


On Dec 7, 8:25 am, Malcolm Tredinnick <malc...@pointy-stick.com>
wrote:
> On Wed, 2007-12-05 at 11:55 -0600, Jacob Kaplan-Moss wrote:
>
> [...]
>
> Jumping in a little late here, but the things I remember thinking about
> (and discussing briefly with Adrian when he described his plan to me) is
> that app_label collisions are often only the tip of the iceberg in some
> respect. They also tend to be an indicator of upcoming template
> directory name collisions -- for example if you use the
> templates/<app_name>/* convention -- and custom templatetag name
> collisions.
>
> None of those problems are in any way insurmountable, but we need to
> think through what will work practically for those situations. For
> example, the template app_loader, if it remains, should not use the
> "label", in Adrian's scheme, to load the template because that would
> mean an application cannot load its own templates (it won't know what
> its own label will be because that is set by the user at installation
> time and unknown to the app writer). I think we're probably fine here:
> app_loader continues to just merge all template directories at the top
> level and application writers still strive for unique names for their
> template directories (or stop using app_loader, which is something I've
> been tending towards more and more lately).
>

I agree that these issues need to be thought through. A related issue
(mentioned elsewhere on this thread) is whether you could have
multiple app instances with the same code but different labels: should
this be allowed/disallowed/checked for? As for standard template
loading, there's no reason that the standard template loading
(app_loader) code could not be modified to work with App instances,
using whatever logic to use the application path and/or label to
locate the templates. Or have I misunderstood Malcolm's comment?

Regards,

Vinay Sajip

Marty Alchin

unread,
Dec 7, 2007, 1:35:42 PM12/7/07
to django-d...@googlegroups.com
On Dec 7, 2007 12:56 PM, Vinay Sajip <vinay...@yahoo.co.uk> wrote:
> this be allowed/disallowed/checked for? As for standard template
> loading, there's no reason that the standard template loading
> (app_loader) code could not be modified to work with App instances,
> using whatever logic to use the application path and/or label to
> locate the templates. Or have I misunderstood Malcolm's comment?

The problem here is that a distributed app has to expect specific
paths to its templates, both for its views (for render_to_response,
for instance) and its templates (for extends and include). If my
template says "{% extend 'modular/module.html %}', what would happen
if someone had "App('modular', name='mod_app')"? How would the extends
tag be able to locate the correct template if someone essentially
renamed the app?

-Gul

Vinay Sajip

unread,
Dec 7, 2007, 5:47:05 PM12/7/07
to Django developers


On Dec 7, 6:35 pm, "Marty Alchin" <gulop...@gamemusic.org> wrote:
The current code in django/template/loaders/app_directories.py appears
to search in the directory 'templates' relative to the directory
containing the app module. I can't quite see why this needs to change:
it's independent of the app label, shouldn't that be OK?

Regards,

Vinay Sajip

Gary Wilson

unread,
Dec 9, 2007, 8:41:12 PM12/9/07
to django-d...@googlegroups.com

It does look in the "templates" directory of the application directory;
however, it gets treated as a standard, global template directory. Current
practice is to repeat the name of your application inside the "templates"
directory to avoid name collisions. For example:

myblogapp/templates/myblogapp/base.html

So even if you were to specify an app_label of "blog" for this app, you would
still have to reference the templates as "myblogapp/base.html".

The current app_directories template loader has always bugged me because it is:

1) inefficient - all application template directories are added as global
template directories and are searched each time by the template loader.

2) unpredictable - any application could, unbeknownst to the developer,
override another application's templates. For example, an application could
include an "admin" directory in its "templates" directory that overrides admin
templates.

3) not DRY - just silly I have to make my "templates" directory one level
deeper with a duplicated name.

For those of you that have stopped using the app_directories loader in favor
of the standard filesystem loader and TEMPLATE_DIRS, what was your reasoning
behind doing so?

And doesn't doing things this was cause the same problems? Every directory
added is going to get searched as a global template directory. You're still
going to have the unpredictability of apps possibly overriding templates of
other apps. You are still going to be repeating yourself by having to put the
app in INSTALLED_APPS and its template directory in TEMPLATE_DIRS.

I have always thought that you should be able to put an app's templates in its
"templates" directory and that those templates should only be accessible when
using the app name in front of it, like "myblogapp/base.html". The
"myblogapp/templates" directory should not be looked at in any other case, and
only global template directories are looked at every time by the filesystem
template loader.

This should also work with custom app_naming, since now the templates would be
accessible with the custom name, i.e. "blog/base.html". This would also work
if you had two instances of an app running under two different names, i.e.
"blog1/base.html" and "blog2/base.html". And I could still override these in
a global template directory with directories named "blog1" and "blog2".

Gary

SmileyChris

unread,
Dec 9, 2007, 9:42:32 PM12/9/07
to Django developers


On Dec 10, 2:41 pm, Gary Wilson <gary.wil...@gmail.com> wrote:
> The current app_directories template loader has always bugged me because it is:
>
> 1) inefficient - all application template directories are added as global
> template directories and are searched each time by the template loader.
>
> 2) unpredictable - any application could, unbeknownst to the developer,
> override another application's templates. For example, an application could
> include an "admin" directory in its "templates" directory that overrides admin
> templates.
>
> 3) not DRY - just silly I have to make my "templates" directory one level
> deeper with a duplicated name.
>
> For those of you that have stopped using the app_directories loader in favor
> of the standard filesystem loader and TEMPLATE_DIRS, what was your reasoning
> behind doing so?
>
> And doesn't doing things this was cause the same problems? Every directory
> added is going to get searched as a global template directory. You're still
> going to have the unpredictability of apps possibly overriding templates of
> other apps. You are still going to be repeating yourself by having to put the
> app in INSTALLED_APPS and its template directory in TEMPLATE_DIRS.
>
> I have always thought that you should be able to put an app's templates in its
> "templates" directory and that those templates should only be accessible when
> using the app name in front of it, like "myblogapp/base.html". The
> "myblogapp/templates" directory should not be looked at in any other case, and
> only global template directories are looked at every time by the filesystem
> template loader.

You're not forced to use the app_directories loader. I'm sure some
people are using it in their project specifically for overriding admin
templates.

What you're proposing is a separate loader. It sounds easily doable
(and like a reasonable idea).
I'd suggest it just uses a "local_templates" directory or similar
rather than stepping on the toes of app_directories loader.

Joseph Kocherhans

unread,
Dec 10, 2007, 1:58:23 PM12/10/07
to django-d...@googlegroups.com

FWIW I pretty much agree with Gary. And surprisingly, I actually agree
with myself of two years ago when we discussed this before [1] [2].
There's also another related thread [3] and a ticket [4] that Adrian
and Jacob both agreed to mark as wontfix. Their arguments against such
a template loader are in the ticket. They have a valid point, but
changing the app_directories template loader still seems like the
cleanest way to deal with custom app_labels and templates to me. Maybe
this will prompt ideas for another solution though.

Joseph

[1] http://groups.google.com/group/django-developers/browse_thread/thread/7197d27127494ee2/df18ee9b91ba383c
[2] http://groups.google.com/group/django-developers/msg/f10d34ebd266fbe2
[3] http://groups.google.com/group/django-developers/browse_thread/thread/49bf270495624a4/8e99219f87db975f
[4] http://code.djangoproject.com/ticket/1586

Vinay Sajip

unread,
Dec 10, 2007, 6:45:05 PM12/10/07
to Django developers


On Dec 10, 6:58 pm, "Joseph Kocherhans" <jkocherh...@gmail.com> wrote:
> [1]http://groups.google.com/group/django-developers/browse_thread/thread...
> [2]http://groups.google.com/group/django-developers/msg/f10d34ebd266fbe2
> [3]http://groups.google.com/group/django-developers/browse_thread/thread...
> [4]http://code.djangoproject.com/ticket/1586

Joseph, thanks for the summary - very useful. I pretty much agree with
what I think you/Gary are saying. If my understanding is correct,
there are two main approaches to template loading (provided as
batteries-included template loaders):

1. Search through the directories in settings.TEMPLATE_DIRS. This
approach suffers from the need to list app template directories in
TEMPLATE_DIRS, which is not DRY because it (sort of) repeats the list
of installed apps in INSTALLED_APPS. You also have to use absolute
paths rather than relative paths, which is an added pain. Efficiency
is not optimal because of the linear search through the directory
list.
2. Search through 'templates' subdirectories in each of the installed
apps. This is good because you don't have to add a repeated directory
list in TEMPLATE_DIRS, but again not optimal because of the linear
search: if every app in INSTALLED_APPS defines some templates, then
templates for apps positioned later in INSTALLED_APPS pay a run-time
penalty because the 'templates' directories of all earlier apps are
searched first.

Both approaches suffer from the fact that multiple apps can define the
same template names - e.g. "base.html" as an ancestor template. The
official way to disambiguate is to place the templates one level down,
using a prefix folder reflecting the app. So in app1 you need to
inherit "app1/base.html" from "app1/child.html" and in app2 you need
to inherit "app2/base.html" from "app2/child.html". This is somewhat
kludgy, to say the least.

It seems like what is needed is an app-centric strategy for template
loading. I'll propose an alternative solution in the interests of
stimulating further discussion, but please be gentle with me ;-)

Assuming for the purposes of discussion that get_template is renamed
to old_get_template, which uses the current strategy, then pseudo code
for the proposed approach might look something like:

def get_template(template_name):
calling_app = get_calling_app() # figure out later how this will work
if calling_app is None:
result = old_get_template(template_name)
else:
try:
result = app_specific_loader(calling_app, template_name)
except TemplateDoesNotExist:
result = old_get_template(template_name)
return result

We assume that calling_app is any app-related object from which we can
get the location of the 'templates' subdirectory, and that
app_specific_loader will get this directory and look for the template
in it, avoiding the kludgy prefix: template names would be mapped
directly below the 'templates' directory in the application. So, for
some app with path 'my.package.app', 'base.html' would map to '/path/
to/my/package/app/templates/base.html', 'news/child.html' would map to
'/path/to/my/package/app/templates/news/child.html', etc. For the
purposes of loading its own templates, an app never needs to refer to
its app_label and doesn't care about it at all. And since the app is
searched first, it can easily override templates defined in other
apps, e.g. admin templates, by returning its own versions.

How would an inter-app template reference work? One possible way would
be to see if a template name looked like '~some_label/some/path/
base.html', in which case the '~some_label' part would be stripped off
and used to locate the app with label 'some_label', and the template
would be expected to be in '/path/to/app/whose/label/is/some_label/
templates/some/path/base.html'. But there's still a problem with
needing to fixup template references if an app gets renamed in
settings.INSTALLED_APPS, and there's no easy way to avoid this
(AFAICT) unless the whole app path is used to specify the app, rather
than a label. For example, '~django.contrib.admin/auth/
base_site.html'.

How might get_calling_app() work? To avoid large scale changes in
function/method signatures, and backward compatibility headaches, a
threadlocal seems the easiest way of achieving this. Every request
which maps to an app will trigger calling a view, which in turn will
load templates as part of its operation. So, if each view is
associated with an app, then the resolver machinery can set the
threadlocal appropriately during request dispatch, and get_calling_app
can read it. The app for each view could be specified either in the
tuples passed to django.conf.urls.defaults.patterns, or as an optional
keyword argument to it, which would be treated as applying to all
tuples passed in that call. I haven't of course thought all of this
through fully, but I want to make sure I'm not going off down a road
which the core developers are uncomfortable with. Plus, I've probably
missed some obvious point which kills the idea stone dead :-(

Does any of this make sense?

Regards,

Vinay Sajip

Malcolm Tredinnick

unread,
Dec 10, 2007, 7:10:36 PM12/10/07
to django-d...@googlegroups.com

On Mon, 2007-12-10 at 15:45 -0800, Vinay Sajip wrote:
[...]

> Plus, I've probably
> missed some obvious point which kills the idea stone dead :-(

Since views are just Python functions, they don't necessarily have a
concept of "current app". It also looks very over-engineered without any
motivating purpose. Why is all this machinery necessary?

The app_loader template loader is a short cut for one particular case.
However, if we were to completely remove the app_loader today, nothing
would be lost in terms of functionality. People could add
<app_name>/templates/ to their TEMPLATE_DIRS setting and it would all
Just Work As Before(tm).

I feel there's a lot of deck chair rearranging going on in this thread
for no particular purpose. Solving the app_lable issue is important. I
sincerely regret bringing up the broader design thinking, since Jacob
had probably realised that already and it's sent the thread off into a
wild tangent that is entirely orthogonal to the issue at hand.

Malcolm

--
The sooner you fall behind, the more time you'll have to catch up.
http://www.pointy-stick.com/blog/

Vinay Sajip

unread,
Dec 11, 2007, 2:19:06 AM12/11/07
to Django developers


On Dec 11, 12:10 am, Malcolm Tredinnick <malc...@pointy-stick.com>
wrote:
> On Mon, 2007-12-10 at 15:45 -0800, Vinay Sajip wrote:
>
> [...]
>
> > Plus, I've probably
> > missed some obvious point which kills the idea stone dead :-(
>
> Since views are just Python functions, they don't necessarily have a
> concept of "current app". It also looks very over-engineered without any
> motivating purpose. Why is all this machinery necessary?

Perhaps it isn't. I thought Gary made some valid points, and clearly
one or two others feel the same way about the points he made - so it
was just to stimulate discussion on how those problems might be solved
(or not). Yes, views are Python functions - but they get the concept
of "current user" through the dispatch machinery, and I don't see
"current app" as being very different.

> The app_loader template loader is a short cut for one particular case.
> However, if we were to completely remove the app_loader today, nothing
> would be lost in terms of functionality. People could add
> <app_name>/templates/ to their TEMPLATE_DIRS setting and it would all
> Just Work As Before(tm).

Agreed. There are many ways to skin a cat.

> I feel there's a lot of deck chair rearranging going on in this thread
> for no particular purpose. Solving the app_lable issue is important. I
> sincerely regret bringing up the broader design thinking, since Jacob
> had probably realised that already and it's sent the thread off into a
> wild tangent that is entirely orthogonal to the issue at hand.

It's not for no particular purpose. I agree that the app_label issue
and the template loading issues are orthogonal. Though of course I'm
biased ;-) I can't see a strong reason why the #3591 patch can't go in
as it is, right now, with perhaps minor mods.

On the template loading issues, the present situation is not ideal, so
there's no harm in trying to see if other approaches can yield an
improvement.

Best regards,

Vinay Sajip

Vinay Sajip

unread,
Dec 11, 2007, 3:27:16 AM12/11/07
to Django developers


On Dec 5, 5:55 pm, "Jacob Kaplan-Moss" <jacob.kaplanm...@gmail.com>
wrote:
> Howdy folks --
>
> I just had a nasty realization here at work: we're gonna need to fix
> app_label pretty damn quick. The good news is that this means we'll
> get to fix this problem on work time; the bad news is that I'm totally
> unsure how to start.
>
> For those who haven't been following along, the main problem is that
> app_label must be unique across all installed apps, so you can't have,
> say, an "ellington.search" and a "marketplace.search" app -- both
> would be known by Django as "search", and Django would get all
> confused.
>
> http://code.djangoproject.com/ticket/3591has been following this
> problem (and the other, related ones). The patches there look
> relatively good and the tests appear to pass, but I'm concerned about
> the level of boat-rocking this patch introduces (it basically redoes
> the entire app/model loading mechanism in the process).

The change in django/db/models/loading.py is of similar magnitude to
the change that was made when AppCache was introduced - it also
revamped the app/model loading machinery to a similar extent. And the
tests do all pass, so unless we feel that the tests need beefing up,
there shouldn't be a lack of confidence in the patch from a TDD
viewpoint.

> I know that at least a couple other committers -- Malcolm, Joseph --
> have looked at this in the past, as have a number of developer
> community members.
>
> So what I'm looking for here is some insight from folks that have
> already looked at this ticket. What are the with the current patch?
> Are there simpler ways? What types of breakage should I be looking
> for? Is there anything out on one of the branches that I'm going to
> fubar if I start working on this?
>

If there is a simpler solution, I would surely welcome it. In terms of
breakage, the big change is that INSTALLED_APPS now can contain both
strings and app instances, so existing code which iterates over
settings.INSTALLED_APPS should now iterate over the list returned by
get_installed_app_paths(). Given that the number of places this
iteration happens in Django itself was the cause of most of the edits
in the patch, I would guess that in other code, the same sort of
changes might need to be made.

Best regards,

Vinay Sajip
Reply all
Reply to author
Forward
0 new messages