Fwd: Ticket #5516 / Changeset 6400
Todd O'Bryan
got committed and just wanted to bring them up here.
In my original patch you had to use the following form for app-supplied
commands:
$ django-admin.py app_name.command
I also separated out the help contents by app so that it was clear
exactly which commands were part of Django proper and which were
provided by an app (and which app each command was supplied by).
The patch that actually made it in uses the form
$ django-admin.py command
and mixes all the commands--Django proper and user-supplied--together in
the help documentation.
I'm worried that we're setting ourselves up for two things:
1. Name collisions. If an app supplies the same command with the current
patch, I think that it overrides the default Django command. If two apps
supply the same command, the one that's listed latest in INSTALLED_APPS
wins, overwriting the earlier one. With the idea that apps should aim
toward being pluggable, that seems like a not-so-great idea to me. I've
already been bitten a few times by the global namespace for templates
and would prefer to avoid creating one for commands if we don't have to.
2. Command confusion. Lumping all the commands together makes it very
easy for people to get confused about the scope and origin of a
particular command. If my poll app includes a command called
initialize_types
$ ./manage.py poll.initialize_types
makes it pretty clear that I'm just initializing the types in that app,
not the whole project. If its help text appears as a group with other
commands from the same app, it's clear what it works on and where it
came from. If, on the other hand, the help is mixed in with the rest of
the commands and I call it using
$ ./manage.py initialize_types
it's not at all clear where it came from or what exactly it's doing.
All that said, I'm really excited that apps can supply their own
commands now. :-)
Todd
Russell Keith-Magee
>
> I have a couple of reservations about the version of app commands that
> got committed and just wanted to bring them up here.
> I'm worried that we're setting ourselves up for two things:
> 1. Name collisions.
I can see what you're talking about here, but IMHO it's a much smaller
potential problem than you think. Consider - we install applications
using a single keyword identifier (the app name), and while there are
occasional discussions about how to allow for deploying two
applications with the same name, it isn't a use case that is very
common.
Adding user-space commands is something that only a subset of
applications will need to do. As a result, there is even less
potential for clashes here than there is in application space.
Complicating the basic interface for the invocation of commands to
account for clashes that aren't likely seems like overkill to me.
However, I agree that the help output could be improved to avoid any
potential confusion; ./manage.py help could organize user-defined
commands separately, and ./manage.py help foobar could display the
application that provided the command. Patches welcome :-)
Yours
Russ Magee %-)
Todd O'Bryan
> On 9/22/07, Todd O'Bryan <toddo...@mac.com> wrote:
> >
> > I have a couple of reservations about the version of app commands that
> > got committed and just wanted to bring them up here.
> ...
> > I'm worried that we're setting ourselves up for two things:
> > 1. Name collisions.
> > 2. Command confusion.
>
> I can see what you're talking about here, but IMHO it's a much smaller
> potential problem than you think. Consider - we install applications
> using a single keyword identifier (the app name), and while there are
> occasional discussions about how to allow for deploying two
> applications with the same name, it isn't a use case that is very
> common.
Right. But now we have the problem of apps overriding core commands. Do
we want that as a possibility? It opens a can of worms in terms of being
able to figure out what the heck is going on. "I ran ./manage.py syncdb
and it deleted my whole database. What happened?"
> Adding user-space commands is something that only a subset of
> applications will need to do. As a result, there is even less
> potential for clashes here than there is in application space.
> Complicating the basic interface for the invocation of commands to
> account for clashes that aren't likely seems like overkill to me.
I don't follow this logic. If even one app provides several commands,
the potential for conflict between it and another app's commands exists.
> However, I agree that the help output could be improved to avoid any
> potential confusion; ./manage.py help could organize user-defined
> commands separately, and ./manage.py help foobar could display the
> application that provided the command. Patches welcome :-)
See the original patch for the first part of this. :-)
Todd
Russell Keith-Magee
>
> On Sat, 2007-09-22 at 11:52 +0800, Russell Keith-Magee wrote:
> > Adding user-space commands is something that only a subset of
> > applications will need to do. As a result, there is even less
> > potential for clashes here than there is in application space.
> > Complicating the basic interface for the invocation of commands to
> > account for clashes that aren't likely seems like overkill to me.
>
> I don't follow this logic. If even one app provides several commands,
> the potential for conflict between it and another app's commands exists.
My point is that while the _potential_ for conflict exists, the
likelyhood of _actual_ conflict is quite small.
The potential for conflict currently exists with the naming of
applications. However, for all the potential, it just isn't a problem
that we encounter frequently. We don't have any apparatus to check for
name clashes, yet it doesn't take too long to determine the source of
the problem when it occurs. The solution for this problem isn't to
change the way we reference applications for the 99% case - it's to
find a way to accomodate the 1% case. In the case of applications, one
proposed solution has been to provide an mechanism for installing an
app under an alias.
Yes, there _could_ be a clash of commands, and a user application
_could_ override a default Django command. However, I put it to you
that this is the 1% case, not the 99% case. Most applications won't be
registering commands, so there isn't a huge probability that a clash
will occur - even less of a probability than with applications. Only a
malicious application would try to register a command called 'syncdb',
and I think the Django community is savvy enough to spot those
applications and avoid them.
So my question becomes 'how do we accomodate the 1% case'. I would
suggest the best approach here would be to check for name clashes when
a new command is registered. If a clash is identified, we can either:
1) Raise an error, stopping manage.py
2) Raise a warning, and
a) adopt the new command
b) ignore the new command
3) Register the duplicate command with an auto-aliased name - the
appname.command syntax you proposed is probably a good option here.
Any of these options could be easily accommodated with a slight change
to line 85-86 of management/__init__.py. Preferences?
Yours,
Russ Magee %-)
Yuri Baburov
> 1) Raise an error, stopping manage.py
> 2) Raise a warning, and
> a) adopt the new command
> b) ignore the new command
> 3) Register the duplicate command with an auto-aliased name - the
> appname.command syntax you proposed is probably a good option here.
>
> Any of these options could be easily accommodated with a slight change
> to line 85-86 of management/__init__.py. Preferences?
Maybe some flag in application or in command to allow override?
With warnings+ignoring overridden command by default.
(Or else automatic scripts may stop working correctly - they would not
know about name clash and behave incorrectly, maybe only sometimes,
and it can hurt database, for example.)
--
Best regards, Yuri V. Baburov, ICQ# 99934676, Skype: yuri.baburov,
MSN: bu...@live.com
Ian Kelly
> Yes, there _could_ be a clash of commands, and a user application
> _could_ override a default Django command. However, I put it to you
> that this is the 1% case, not the 99% case. Most applications won't be
> registering commands, so there isn't a huge probability that a clash
> will occur - even less of a probability than with applications. Only a
> malicious application would try to register a command called 'syncdb',
> and I think the Django community is savvy enough to spot those
> applications and avoid them.
I don't think the likelihood is as low as you suggest. True, not all
applications will register commands, but I imagine those that do will
often register multiple commands. In addition, I think there is a
higher likelihood that any two given commands will share the same name
than any two given apps, especially if apps start to copy useful
commands from other apps. Put those together, and I think that
command name collisions could turn out to be more common than app name
collisions after all.
Ian
Todd O'Bryan
> So my question becomes 'how do we accomodate the 1% case'. I would
> suggest the best approach here would be to check for name clashes when
> a new command is registered. If a clash is identified, we can either:
> 1) Raise an error, stopping manage.py
> 2) Raise a warning, and
> a) adopt the new command
> b) ignore the new command
> 3) Register the duplicate command with an auto-aliased name - the
> appname.command syntax you proposed is probably a good option here.
>
OK. But there are some points from the Zen of Python that are, I think,
appropriate here:
* Explicit is better than implicit.
* Special cases aren't special enough to break the rules.
* In the face of ambiguity, refuse the temptation to guess.
* Namespaces are one honking great idea -- let's do more of those!
My point is that my original patch only requires you to put
app_name.command instead of just command. If someone wants to provide a
syncdb command, it would be clear that dbmigrations.syncdb is different
from syncdb in that the former is provided by that app and the latter is
provided by Django itself.
If, on the other hand, dbmigrations also provides migratedb, then, based
on the "if a conflict occurs, append the app_name" you'd have migratedb
and dbmigrations.syncdb, both provided by the same app.
What's more, if two apps provide the same command, which one will get
the app_name in front of it depends on which order the two appear in
INSTALLED_APPS. If we both have app alpha and beta installed, but the
order is different, I might have to use alpha.run_something and
runsomething as commands, while you would use run_something and
beta.run_something. This creates ambiguities, breaks rules with special
cases, and causing problems with implicitness. All of which just is not
Pythonic, at least according to my reading.
In short, we can avoid all these potential problems if we just make
people put the app_module in front of commands. It's one little extra
bit with a period, it's something they should be aware of anyway
(because you shouldn't be using a command unless you know where it's
coming from), and it avoids that 1% of problems without making things
99% more complicated.
Furthermore, sometimes worrying about the 1% of the cases before they've
caused a problem is worth it.
http://www.joelonsoftware.com/articles/Craftsmanship.html
Here's a case where fixing the problems *before* they occur doesn't take
any more effort than not fixing them, so why don't we fix them?
Todd
P.S. Sorry to be such a pain about this. But, as I said, I've been
bitten by the global namespace problem for templates one too many times.
I mentioned it in the high school class I teach and the four students
who've spent a decent amount of time with Django have also been bitten
by it. If we can avoid having the same name refer ambiguously to
multiple things in the command domain, I think that's a good thing. I
will, however, defer to BDFL fiat, but will have an "I told you so"
email ready and waiting for the first time someone gets bitten by this
issue.