Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#24769 closed Cleanup/optimization (fixed)

Cast optparse verbosity argument to an integer for better backwards compatibility

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

Description

With Django 1.8 and Python 3.4.3, I see the following error:

% manage.py test
...
  File "…/django18/django/core/management/commands/test.py", line 65, in execute
    if options['verbosity'] > 0:
TypeError: unorderable types: str() > int()

It appears to stem from the optparse parser being used (because
BaseCommand.option_list is non-empty (for --pdb and --ipdb)):

parser.add_option('-v', '--verbosity', action='store', dest='verbosity', default='1',
    type='choice', choices=['0', '1', '2', '3'],
    help='Verbosity level; 0=minimal output, 1=normal output, 2=verbose output, 3=very verbose output')

The type "choice" is a subtype of "string" options (https://docs.python.org/2/library/optparse.html).

It seems like this option whould need to be casted to an int after parsing.

This was added in 85686386 (#19973).

Traceback (most recent call last):
  File "…/bin/manage.py", line 37, in <module>
    execute_from_command_line(sys.argv)
  File "…/django18/django/core/management/__init__.py", line 338, in execute_from_command_line
    utility.execute()
  File "…/django18/django/core/management/__init__.py", line 330, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "…/django18/django/core/management/commands/test.py", line 30, in run_from_argv
    super(Command, self).run_from_argv(argv)
  File "…/django18/django/core/management/base.py", line 390, in run_from_argv
    self.execute(*args, **cmd_options)
  File "…/django18/django/core/management/commands/test.py", line 65, in execute
    if options['verbosity'] > 0:
TypeError: unorderable types: str() > int()

I have not tried it with Django master, because there are several other problems.

In case it has been fixed there already, I'd say that this is something that should get backported.

Change History (10)

comment:1 Changed 4 years ago by Tim Graham

May be a duplicate of #24518 -- are you using an old version of django-configurations? If not, please tell us how we can reproduce the issue.

comment:2 Changed 4 years ago by Daniel Hahler

Has patch: set
Version: 1.8master

I am using django-configurations, but from master.

In #24518 django-configurations is the one that adds to BaseCommand.option_list (which triggers the optparse code path).

I'd say it should be reproducible by just using the deprecated parser, usually by providing option_list.
(e.g. for django-pdb: https://github.com/tomchristie/django-pdb/blob/master/django_pdb/management/commands/test.py#L16-27)

See https://github.com/django/django/pull/4628 for a PR.

I think this issue can be closed as duplicate of #24518 then, which should be re-opened until it's fixed.

Last edited 4 years ago by Daniel Hahler (previous) (diff)

comment:3 Changed 4 years ago by Claude Paroz

Django core commands are now argparse-based, so setting option_list for those commands should be considered as a bug. Reading the code from django-configurations, it appears it does now the right thing. But I may have missed some valid code path which triggers the issue. Maybe with a sample project?

comment:4 Changed 4 years ago by Daniel Hahler

The problem in my case is not with django-configurations, but django-pdb.
It just got fixed there for Django 1.8, too: https://github.com/tomchristie/django-pdb/commit/90e6bec2d463ba215d75f196d398dc0d4c36dc00.

I can see that casting verbosity to int would be a behaviour change (for the old parser). But when a command isn't using option_list that would happen anyway (because it uses the new parser).

Therefore I still think that it's better for consistency to use int in both cases.

comment:5 Changed 4 years ago by Tim Graham

Is there much benefit in fixing this in Django versus having projects switch to argparse as necessary when they add 1.8 compatibility? I'd just like to spend our time most efficiently.

comment:6 Changed 4 years ago by Tim Graham

Needs tests: set
Triage Stage: UnreviewedAccepted

comment:7 Changed 4 years ago by Tim Graham

Summary: "TypeError: unorderable types: str() > int()" with options['verbosity'] and optparseCast optparse verbosity argument to an integer for better backwards compatibility
Type: BugCleanup/optimization
Version: master1.8

comment:8 Changed 4 years ago by Claude Paroz

As for me, this enters the category "We should have made it better, but the train has left the station...".

comment:9 Changed 4 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In a0047c62:

Fixed #24769 -- Cast optparse verbosity argument to an integer for better backwards compatibility.

Using BaseCommand.options_list makes Django use the legacy optparse
parser, which does not set the verbosity attribute correctly. Now the
verbosity argument is always cast to int. Regression in 8568638 (#19973).

Initial report and patch from blueyed.

comment:10 Changed 4 years ago by Tim Graham <timograham@…>

In 76c526f8:

[1.8.x] Fixed #24769 -- Cast optparse verbosity argument to an integer for better backwards compatibility.

Using BaseCommand.options_list makes Django use the legacy optparse
parser, which does not set the verbosity attribute correctly. Now the
verbosity argument is always cast to int. Regression in 8568638 (#19973).

Initial report and patch from blueyed.

Backport of a0047c6242fd48068eb444e0a58f7a5d2bc1bcd3 from master

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