Refactoring command options

7 views
Skip to first unread message

Todd O'Bryan

unread,
Sep 8, 2007, 10:58:19 PM9/8/07
to django-d...@googlegroups.com
I think my refactoring of django.core.management to let each command
register its own options is complete. (Can anybody think of a good way
to test everything, other than running stuff by hand, which I've been
doing?)

I did hit a couple of issues:

1. The args for loaddata had 'fixture, fixture, ...' I thought it should
be 'fixture [fixture ...]' (without the commas) and changed it in the
patch.

2. runfcgi had its own help option for printing help. I fiddled with it
a little so that 'django-admin.py help runfcgi' prints something
consistent with what all the other commands do.

The patch (which is surprisingly puny for the mental gymnastics I feel
like I had to do) is at http://code.djangoproject.com/ticket/5369

Comments appreciated!
Todd

Adrian Holovaty

unread,
Sep 9, 2007, 6:16:21 PM9/9/07
to django-d...@googlegroups.com
On 9/8/07, Todd O'Bryan <toddo...@mac.com> wrote:
> I think my refactoring of django.core.management to let each command
> register its own options is complete.

Great stuff, Todd. I've committed it in [6075]. I left a few comments
at http://code.djangoproject.com/ticket/5369 about the main changes I
made to the patch, but otherwise all was used pretty much as-is.
Thanks for your work!

Adrian

--
Adrian Holovaty
holovaty.com | djangoproject.com

Todd O'Bryan

unread,
Sep 9, 2007, 7:05:56 PM9/9/07
to django-d...@googlegroups.com
On Sun, 2007-09-09 at 17:16 -0500, Adrian Holovaty wrote:
> On 9/8/07, Todd O'Bryan <toddo...@mac.com> wrote:
> > I think my refactoring of django.core.management to let each command
> > register its own options is complete.
>
> Great stuff, Todd. I've committed it in [6075]. I left a few comments
> at http://code.djangoproject.com/ticket/5369 about the main changes I
> made to the patch, but otherwise all was used pretty much as-is.
> Thanks for your work!
>
> Adrian
>

Woo-hoo! To quote Sally Field, "You like me, you really like me."

And thanks for the clean-up of the option_list mess. I was thinking like
a Java programmer and missed the clean, Pythonic way of dealing with
that.

Now, I think it should be possible to let apps add their own commands
pretty cleanly...

Todd

SmileyChris

unread,
Sep 9, 2007, 7:41:51 PM9/9/07
to Django developers
On Sep 10, 10:16 am, "Adrian Holovaty" <holov...@gmail.com> wrote:

> On 9/8/07, Todd O'Bryan <toddobr...@mac.com> wrote:
>
> > I think my refactoring of django.core.management to let each command
> > register its own options is complete.
>
> Great stuff, Todd. I've committed it in [6075].

It would seem this broke reloading of the built-in web server.

Todd O'Bryan

unread,
Sep 9, 2007, 9:04:56 PM9/9/07
to django-d...@googlegroups.com

Are you sure it's broken? I just checked and it's still working for me.

Todd

Russell Keith-Magee

unread,
Sep 9, 2007, 10:41:33 PM9/9/07
to django-d...@googlegroups.com

I'm seeing breakage too: I haven't tested it that extensively, but I'm
getting the following pair of tracebacks consistently on Windows on
the first reload of the server:

------

Traceback (most recent call last):
File "C:\eclipse\infrastructure\django\django\core\management\base.py", line 6
4, in run
self.execute(*args, **options.__dict__)
File "C:\eclipse\infrastructure\django\django\core\management\base.py", line 8
1, in execute
output = self.handle(*args, **options)
File "C:\eclipse\infrastructure\django\django\core\management\commands\runserv
er.py", line 75, in handle
autoreload.main(inner_run)
File "C:\eclipse\infrastructure\django\django\utils\autoreload.py", line 90, i
n main
reloader_thread()
File "C:\eclipse\infrastructure\django\django\utils\autoreload.py", line 65, i
n reloader_thread
sys.exit(3) # force reload
SystemExit: 3
usage: runserver [options] [optional port number, or ipaddr:port]

Starts a lightweight Web server for development.

Traceback (most recent call last):
File "C:\eclipse\infrastructure\django\django\core\management\base.py", line 6
4, in run
self.execute(*args, **options.__dict__)
File "C:\eclipse\infrastructure\django\django\core\management\base.py", line 8
1, in execute
output = self.handle(*args, **options)
File "C:\eclipse\infrastructure\django\django\core\management\commands\runserv
er.py", line 75, in handle
autoreload.main(inner_run)
File "C:\eclipse\infrastructure\django\django\utils\autoreload.py", line 95, i
n main
sys.exit(restart_with_reloader())
SystemExit: 0
usage: runserver [options] [optional port number, or ipaddr:port]

Starts a lightweight Web server for development.
------

I haven't dug into this too far, but if I had to guess I'd say the
order of arguments passed to the respawn doesn't match the arguments
originally passed to the server.

Yours,
Russ Magee %-)

Adrian Holovaty

unread,
Sep 9, 2007, 10:55:11 PM9/9/07
to django-d...@googlegroups.com
On 9/9/07, Russell Keith-Magee <freakb...@gmail.com> wrote:
> I'm seeing breakage too: I haven't tested it that extensively, but I'm
> getting the following pair of tracebacks consistently on Windows on
> the first reload of the server:

Hmm, I confirm I get the same error on OS X -- it happens after the
server is reloaded. How strange...

Todd O'Bryan

unread,
Sep 9, 2007, 11:23:02 PM9/9/07
to django-d...@googlegroups.com
On Sun, 2007-09-09 at 21:55 -0500, Adrian Holovaty wrote:
> On 9/9/07, Russell Keith-Magee <freakb...@gmail.com> wrote:
> > I'm seeing breakage too: I haven't tested it that extensively, but I'm
> > getting the following pair of tracebacks consistently on Windows on
> > the first reload of the server:
>
> Hmm, I confirm I get the same error on OS X -- it happens after the
> server is reloaded. How strange...
>
> Adrian
>
I see the problem. The reloader uses sys.argv and grabs the last command
line directly. By the time it gets to runserver.Command.handle, the
command line has been munged a little.

Strangely, though, I got a clean reload on Linux.

Todd

SmileyChris

unread,
Sep 10, 2007, 12:23:40 AM9/10/07
to Django developers
On Sep 10, 2:55 pm, "Adrian Holovaty" <holov...@gmail.com> wrote:
> Hmm, I confirm I get the same error on OS X -- it happens after the
> server is reloaded. How strange...

So if we have several confirmations of breakage, shouldn't it be
rolled back?

Adrian Holovaty

unread,
Sep 10, 2007, 12:25:07 AM9/10/07
to django-d...@googlegroups.com
On 9/9/07, Todd O'Bryan <toddo...@mac.com> wrote:
> I see the problem. The reloader uses sys.argv and grabs the last command
> line directly. By the time it gets to runserver.Command.handle, the
> command line has been munged a little.

That doesn't make any sense, though. Which part of the code changes
sys.argv? I can't seem to find it. Does the optparse module do that?
And we were already using optparse before...

Adrian Holovaty

unread,
Sep 10, 2007, 12:40:06 AM9/10/07
to django-d...@googlegroups.com
On 9/9/07, SmileyChris <smile...@gmail.com> wrote:
> So if we have several confirmations of breakage, shouldn't it be
> rolled back?

Nah, no need to roll this one back, unless it takes forever to figure
out. The changeset has too many files, and the breakage isn't
horrible.

Todd O'Bryan

unread,
Sep 10, 2007, 1:15:36 AM9/10/07
to django-d...@googlegroups.com
On Sun, 2007-09-09 at 23:25 -0500, Adrian Holovaty wrote:
> On 9/9/07, Todd O'Bryan <toddo...@mac.com> wrote:
> > I see the problem. The reloader uses sys.argv and grabs the last command
> > line directly. By the time it gets to runserver.Command.handle, the
> > command line has been munged a little.
>
> That doesn't make any sense, though. Which part of the code changes
> sys.argv? I can't seem to find it. Does the optparse module do that?
> And we were already using optparse before...

I didn't change sys.argv at all. It's possible optparse does, but you're
right that this is odd. I'll take another look in the morning. I still
haven't seen the error on Linux, though.

Todd

Adrian Holovaty

unread,
Sep 11, 2007, 12:38:04 AM9/11/07
to django-d...@googlegroups.com
On 9/9/07, SmileyChris <smile...@gmail.com> wrote:
> It would seem this broke reloading of the built-in web server.

I've fixed that in [6094].

Todd O'Bryan

unread,
Sep 11, 2007, 10:06:39 AM9/11/07
to django-d...@googlegroups.com
The code you removed was trying to catch an error created by using
illegal options or arguments in one of the commands. Apparently optparse
still prints the usage message, but you also see the Python error.

It looks like sys.exit(3) is raising an exception that no one has ever
noticed before. Not sure if it's worth it, but maybe specifying a more
specific kind of exception would solve the problem.

Todd

Reply all
Reply to author
Forward
0 new messages