[Django] #24969: Backwards compatibility for optparse in management commands broken for overrides.
Django
overrides.
-------------------------------+--------------------
Reporter: paul-butcher | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
Since this change:
https://github.com/django/django/commit/856863860352aa1f0288e6c9168a0e423c4f5184
The built-in options are always added, regardless of what is in
option_list for the Command.
Management commands that, in previous versions, overrode or replaced items
in the default option_list now raise an OptionConflictError on invocation.
This was noticed using Lettuce, (http://lettuce.it/), where invocation,
thus:
{{{$ python manage.py harvest}}}
gives the following response
{{{optparse.OptionConflictError: option -v/--verbosity: conflicting option
string(s): -v, --verbosity}}}
The `harvest` management command replaces the verbosity option, by
removing it from the option_list, thus:
{{{
class Command(BaseCommand):
help = u'Run lettuce tests all along installed apps'
args = '[PATH to feature file or folder]'
requires_model_validation = False
option_list = BaseCommand.option_list[1:] + (
make_option('-v', '--verbosity', action='store', dest='verbosity',
default='4',
type='choice', choices=map(str, range(5)),
help='Verbosity level; 0=no output, 1=only dots, 2=only
scenario names, 3=colorless output, 4=normal output (colorful)'),
...
}}}
Lettuce has a its [https://github.com/gabrielfalcao/lettuce/issues/480 own
corresponding version of this issue].
In earlier versions of Django, verbosity was the first item in
BaseCommand.option_list, so was discarded and replaced by the result of
make_option above.
In the current version, BaseCommand.option_list is no longer a collection
of defaults, and its former content is now mandatory and added
independently.
This problem does not necessarily arise using argparse, as the derived
class has greater control by overriding `add_arguments`
--
Ticket URL: <https://code.djangoproject.com/ticket/24969>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
overrides.
Reporter: paul-butcher | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Can you suggest a remedy?
--
Ticket URL: <https://code.djangoproject.com/ticket/24969#comment:1>
Django
overrides.
Reporter: paul-butcher | Owner: nobody
Type: Uncategorized | Status: closed
Component: Core (Management | Version: 1.8
commands) | Resolution:
Severity: Normal | worksforme
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* status: new => closed
* resolution: => worksforme
* component: Uncategorized => Core (Management commands)
Comment:
I think the workaround is to customize the `create_parser` method instead
of fiddling with `option_list`.
I commented on the original report
(https://github.com/gabrielfalcao/lettuce/issues/480#issuecomment-111247247).
--
Ticket URL: <https://code.djangoproject.com/ticket/24969#comment:2>
Django
overrides.
Reporter: paul-butcher | Owner: nobody
Type: Uncategorized | Status: closed
Component: Core (Management | Version: 1.8
commands) | Resolution:
Severity: Normal | worksforme
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by paul-butcher):
Replying to [comment:1 timgraham]:
* Reinstate BaseCommand.option_list as it was before
* In `use_argparse`, change the condition (currently `not
bool(self.option_list)`) to `self.option_list == BaseCommand.option_list`.
* In `create_parser`, remove all the individual `add_option` calls,
leaving just the one in the loop.
That way, the old defaulting/overriding behaviour could be maintained.
Another candidate could be to set the conflict_handler to "resolve",
however, that would be a major change to the behaviour (authors of derived
classes may be expecting an OptionConflictError that doesn't come), and
would still only allow derived classes to override, rather than remove the
default options. So, that would be inappropriate.
--
Ticket URL: <https://code.djangoproject.com/ticket/24969#comment:3>
Django
overrides.
Reporter: paul-butcher | Owner: nobody
Type: Uncategorized | Status: closed
Component: Core (Management | Version: 1.8
commands) | Resolution:
Severity: Normal | worksforme
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by paul-butcher):
Replying to [comment:2 claudep]:
> I think the workaround is to customize the `create_parser` method
instead of fiddling with `option_list`.
> I commented on the original report
(https://github.com/gabrielfalcao/lettuce/issues/480#issuecomment-111247247).
Changes to option parsing in derived classes should ideally update them to
use the modern argparse mechanism, rather than to keep alive the
deprecated optparse mechanism. I'm sure that eventually, all the various
authors of management commands will get around to updating them. In the
case of lettuce, where the problem was spotted, I may well do so myself if
no-one else beats me to it.
However, as it stands, upgrading Django causes some optparse-based
management commands to stop working, which is what I meant by "Backwards
compatibility ... broken".
If this is a desired behaviour, aimed at providing a stronger nudge than a
Deprecation Warning, then that is fine (though one might argue that the
optparse stuff should just be removed, if that's the case), but then this
probably becomes a documentation issue, where the "Default options" are
now "Built-in" or "Compulsory" options.
--
Ticket URL: <https://code.djangoproject.com/ticket/24969#comment:4>
Django
overrides.
Reporter: paul-butcher | Owner: nobody
Type: Uncategorized | Status: closed
Component: Core (Management | Version: 1.8
commands) | Resolution:
Severity: Normal | worksforme
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
For the optparse to argparse migration, we tried to keep the management
command system as much as backwards-compatible as possible, but it's
totally possible we could have performed better.
If you have an idea about a better solution, feel free to propose a proof-
of-concept patch and we'll consider it.
--
Ticket URL: <https://code.djangoproject.com/ticket/24969#comment:5>