Opened 17 years ago

Last modified 16 years ago

#6017 closed

order of django-admin.py options matters, but shouldn't — at Version 2

Reported by: Todd O'Bryan Owned by: nobody
Component: Core (Other) Version: dev
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Russell Keith-Magee)

In ticket #5369, django-admin.py and manage.py are supposed to read all options, looking for --settings and --pythonpath, and then use those two if they're present to access app-supplied commands. The code (which was mine) has a bug, in that options stop getting read as soon as something other than --settings or --pythonpath gets hit.

Without the patch, you currently get:

>>> from django.core.management import LaxOptionParser, get_version
>>> from django.core.management.base import BaseCommand
>>> parser = LaxOptionParser(version=get_version(), option_list=BaseCommand.option_list)
>>> options, args = parser.parse_args('django-admin.py command --option --settings=settingsmodule --pythonpath=/home/user/django-dir'.split())
>>> print options
{'pythonpath': None, 'settings': None}
>>> args
['django-admin.py', 'command', '--settings=settingsmodule', '--pythonpath=/home/user/django-dir']

where both --pythonpath and --settings get ignored.

With the included patch, you get:

>>> from django.core.management import LaxOptionParser, get_version
>>> from django.core.management.base import BaseCommand
>>> parser = LaxOptionParser(version=get_version(), option_list=BaseCommand.option_list)
>>> options, args = parser.parse_args('django-admin.py command --option --settings=settingsmodule --pythonpath=/home/user/django-dir'.split())
>>> print options
{'pythonpath': '/home/user/django-dir', 'settings': 'settingsmodule'}
>>> args
['django-admin.py', 'command', '--option']

as desired.

If someone wants me to stick the doctest code somewhere, please tell me where. I think I may need to make a whole new folder, but I don't want to make a mess.

Change History (4)

by Todd O'Bryan, 17 years ago

Attachment: lax-option-parser.patch added

comment:1 by Todd O'Bryan, 17 years ago

Component: UncategorizedCore framework
Has patch: set

I so wish we could edit tickets after we make a mess of them. (Or maybe I should learn to click the preview button.)

Anyway, I included tests in django/tests/regressiontests/core_management/tests.py. I also included empty init.py and models.py files, but I don't think those show up.

comment:2 by Russell Keith-Magee, 17 years ago

Description: modified (diff)
Needs tests: set
Triage Stage: UnreviewedAccepted

The patch itself looks good; my quick poking didn't reveal any problems, but that's just quick poking. Since you worked out a way to regression test this bit, it should be a bit more comprehensive than one test. There are several branch paths in the patch - these should all be tested (at the very least, -X , --X and --X=Y option types should all be checked, in various orders and combinations with --settings etc).

Also, a minor point of style - calling the test directory 'management' is sufficient.

I've attached a slightly revised patch, with a couple more tests and cleaned up comments; if you can bulk out the patch with some more tests, I'll commit.

by Russell Keith-Magee, 17 years ago

Attachment: 6017-r6718.diff added

Revised patch for lax option parser; comment and test case changes.

Note: See TracTickets for help on using tickets.
Back to Top