Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#33205 closed Bug (fixed)

call_command() fails when required mutually exclusive arguments use the same `dest`.

Reported by: Peter Law Owned by: Hasan Ramezani
Component: Core (Management commands) Version: 3.2
Severity: Normal Keywords:
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

I have a command which accepts two different ways to specify a time -- either as a timestamp or as a duration in the future:

pause (--for duration | --until time)
class Command(BaseCommand):
    def add_arguments(self, parser) -> None:
        group = parser.add_mutually_exclusive_group(required=True)
        group.add_argument('--for', dest='until', action='store', type=parse_duration_to_time)
        group.add_argument('--until', action='store', type=parse_time)

    def handle(self, until: datetime, **_):
        pass

This works fine on the command line, however there doesn't seem to be a way to make this work through call_command. Specifically there are two sides to the failure:

  • while I can provide an until value (as a string, which is processed by parse_time) there is no mechanism to pass a for value if that's how I want to spell the input
  • the for value is always required and attempts to parse the (string) until value passed, which then errors since the input formats are very different

Change History (11)

comment:1 by Mariusz Felisiak, 3 years ago

Cc: Hasan Ramezani added
Component: UncategorizedCore (Management commands)
Summary: call_command fails when required mutually exclusive group arguments use the same `dest`call_command() fails when required mutually exclusive arguments use the same `dest`.
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Thanks for the report. The following calls work as expected for me :

management.call_command('pause', '--until=1')
management.call_command('pause', '--until', '1')
management.call_command('pause', '--for=1')
management.call_command('pause', '--for', '1')

however I confirmed an issue when passing arguments in keyword arguments:

management.call_command('pause', until='1')
management.call_command('pause', **{'for': '1'}) # Using "call_command('pause', for='1')" raises SyntaxError

This is caused by using dest for mapping **options to arguments, see call_command().

comment:2 by Hasan Ramezani, 3 years ago

I can create a patch to fix the two above-mentioned issues but using the command with both options together:

management.call_command('pause', **{'for': '1', 'util': '1'})

won't raise an error because both options use the same dest. I will investigate it more.

I don't know do we have to support passing both dest and arg name as keyword arguments? in the example of this ticket, if we call management.call_command('pause', until='1'), it should be considered as util arg or for(because dest of for is util as well)

Version 2, edited 3 years ago by Hasan Ramezani (previous) (next) (diff)

comment:3 by Peter Law, 3 years ago

Ah, interesting, I wasn't aware that those other spellings worked! I'd be happy to switch to using those spellings instead (I've confirmed they work for my original case).

Perhaps just documenting this limitation (and the fact that those spellings work as alternatives) and/or improving the related error messages could be a way to go here?

in reply to:  2 ; comment:4 by Mariusz Felisiak, 3 years ago

Replying to Hasan Ramezani:

I don't know do we have to support passing both dest and arg name as keyword arguments? in the example of this ticket, if we call management.call_command('pause', until='1'), it should be considered as until arg or for (because dest of for is until as well)

We should support option names as documented:

** options
named options accepted on the command-line.

so

  • management.call_command('pause', until='1') should work the same as calling pause --until 1
  • management.call_command('pause', **{'for': '1'}) should work the same as calling pause --for 1
  • management.call_command('pause', **{'for': '1', 'until': '1'}) should work the same as calling pause --for 1 --until 1 and raise an exception

in reply to:  4 comment:5 by Hasan Ramezani, 3 years ago

Replying to Mariusz Felisiak:

I am not sure about your second example:

  • management.call_command('pause', **{'for': '1'}) should work the same as calling pause --for 1

Based on the documentation, it seems we have to pass dest as keyword argument name when we define dest for arguments.

Some command options have different names when using call_command() instead of django-admin or manage.py. For example, django-admin createsuperuser --no-input translates to call_command('createsuperuser', interactive=False). To find what keyword argument name to use for call_command(), check the command’s source code for the dest argument passed to parser.add_argument().

Also, when Django adds required arguments in call command, it search for dest in options.

comment:6 by Mariusz Felisiak, 3 years ago

You're right, sorry, I missed "... check the command’s source code for the dest argument passed to parser.add_argument().". In that case I would raise an error that passing dest with multiple arguments via **options is not supported.

comment:7 by Hasan Ramezani, 3 years ago

Owner: changed from nobody to Hasan Ramezani
Status: newassigned

comment:8 by Hasan Ramezani, 3 years ago

Has patch: set

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

Fixed #33205 -- Made call_command() raise TypeError when dest with multiple arguments is passed.

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

In c9ebe4ca:

[4.0.x] Fixed #33205 -- Made call_command() raise TypeError when dest with multiple arguments is passed.

Backport of c1e4111c74ee9d9f48cbee5a5b7c40289203c93d from main

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