Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#24969 closed Uncategorized (worksforme)

Backwards compatibility for optparse in management commands broken for overrides.

Reported by: Paul Butcher Owned by: nobody
Component: Core (Management commands) Version: 1.8
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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 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

Change History (5)

comment:1 by Tim Graham, 9 years ago

Can you suggest a remedy?

comment:2 by Claude Paroz, 9 years ago

Component: UncategorizedCore (Management commands)
Resolution: worksforme
Status: newclosed

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).

in reply to:  1 comment:3 by Paul Butcher, 9 years ago

Replying to 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.

in reply to:  2 comment:4 by Paul Butcher, 9 years ago

Replying to 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.

comment:5 by Claude Paroz, 9 years ago

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.

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