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
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
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
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'),
)
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.
Joseph
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
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
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
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
Got it.
> Why not also use actual module objects here instead of strings like withI don't see the benefit. It makes sense for views so you can apply
> view functions in urlconfs?
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.
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/
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
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
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.
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
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
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
> 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/