Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#32153 closed Bug (fixed)

Support for list arguments inside exclusive required groups

Reported by: Mark Gajdosik Owned by: Hasan Ramezani
Component: Core (Management commands) Version: dev
Severity: Normal Keywords: call_command, exclusive group, nargs, error
Cc: Hasan Ramezani 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 Mark Gajdosik)

Are there any plans to add support for the following?

from django.core.management import BaseCommand

class Command(BaseCommand):
    def add_arguments(self, parser):
        group = parser.add_mutually_exclusive_group(required=True)
        group.add_argument('--foo', nargs='+', type=int)

    def handle(self, *args, **options):
        pass

When calling the above using call_command:

call_command('call_command_test', foo=[1, 2, 3])
# Raises: django.core.management.base.CommandError: Error: argument --foo: invalid int value: '[1, 2, 3]'

call_command('call_command_test', '--foo=1', '--foo=2', '--foo=3')
# Option 'foo' will be of value [3]

I can't think of any workarounds other than setting type=str (somehow, that works fine) and coercing manually. Thank you!

Change History (11)

comment:1 by Mark Gajdosik, 3 years ago

Description: modified (diff)

comment:2 by Mark Gajdosik, 3 years ago

Description: modified (diff)

comment:3 by Hasan Ramezani, 3 years ago

Cc: Hasan Ramezani added

I think we need to add proper handling for required arguments with nargs as well.

in reply to:  3 comment:4 by Mark Gajdosik, 3 years ago

Replying to Hasan Ramezani:

I think we need to add proper handling for required arguments with nargs as well.

That would always be neat. I couldn't get it to work with the current {}={} syntax and the list comprehension:

# Any required arguments which are passed in via **options must be passed
# to parse_args().
for opt in parser_actions:
    if (
            opt.dest in options and
            (opt.required or opt in mutually_exclusive_required_options)
    ):
        parse_args.append(min(opt.option_strings))
        value = arg_options[opt.dest]
        if isinstance(value, (list, tuple)):
            parse_args += map(str, value)
        else:
            parse_args.append(str(value))

comment:5 by Mariusz Felisiak, 3 years ago

Component: UncategorizedCore (Management commands)
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Thanks for this report. Yes we should change parse_args to the list of arguments and their values without using the = syntax, e.g. pass ['--foo', '1', '2'] instead of ['--foo=1 2']. All of these calls should be supported:

management.call_command('command', foo=[1, 2])
management.call_command('command', '--foo', [1, 2])
Last edited 3 years ago by Mariusz Felisiak (previous) (diff)

comment:6 by Hasan Ramezani, 3 years ago

Has patch: set
Owner: changed from nobody to Hasan Ramezani
Status: newassigned

PR
I think we shouldn't support management.call_command('command', '--foo 1 2').
I think python argparse also doesn't support that.
Please correct me if I get it wrong.

comment:7 by Mariusz Felisiak, 3 years ago

Patch needs improvement: set

comment:8 by Mariusz Felisiak, 3 years ago

Patch needs improvement: unset

After reconsideration, I agree that we shouldn't support this, it's not described in the Option value syntax and it can be tricky to support all options.

comment:9 by Mariusz Felisiak, 3 years ago

Triage Stage: AcceptedReady for checkin

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In f06beea9:

Fixed #32153 -- Fixed management commands when using required list options.

Thanks Mark Gajdosik for the report and initial patch.

comment:11 by Mark Gajdosik, 3 years ago

Thank you both for expediting this so quickly!

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