One behaviour that this patch removes and needs to be re-added is that once a kwarg has been seen, all arguments after the inital must me considered kwargs too. Otherwise this messes with the order of positional arguments.

I think this should work!

as soon as kwargs_switch is True, kwargs_switch is never set to False again. Would have been good to have test that verifies this.

On there other hand, there are a few things that I don't yet understand:

1. You could do something like

    cmd = {
        'prefix': 'orch daemon add',
        'daemon_type': 'placement=smithi044:[v2:172.21.15.44:3301,v1:172.21.15.44:6790]=c'',
        'placement': 'daemon_type=mon`,
    }

which is a bit questionable.

2. the arguments are always a dict already. Parsing the values as k-v args is in general a bit questionable.
3. you're somewhat duplicating ceph_argparse.py here

On there other hand, there are a few things that I don't yet understand:

1. You could do something like

    cmd = {
        'prefix': 'orch daemon add',
        'daemon_type': 'placement=smithi044:[v2:172.21.15.44:3301,v1:172.21.15.44:6790]=c'',
        'placement': 'daemon_type=mon`,
    }

which is a bit questionable.

Somewhat contrived but yes. My solution would have been to require a flag when the argument is defined in the module. Consider

if we'd require a flag like 'name=fs,type=CephString,req=false,kw=true ' for arguments that can be passed a kwargs, we'd skirt a lot of the issues.

1. the arguments are **always** a dict already. Parsing the values as k-v args is in general a bit questionable.

Motivation for this is for modules that have a lot of arguments that have default values. If a user wants to specify only one of those arguments, without the kwargs extension they would have to pass the preceding arguments in the right order. So this should add quite a bit of convenience for users.

2. you're somewhat duplicating ceph_argparse.py here

Motivation for this is for modules that have a lot of arguments that have default values. If a user wants to specify only one of those arguments, without the kwargs extension they would have to pass the preceding arguments in the right order. So this should add quite a bit of convenience for users.

Interesting. I'm facing the same issue with the orch commands. Especially monsters like

with users passing

ceph orch apply rgw myrealm myzone count:3,label:rgws 

really does wrong things.

Anyway.

you want to make a new PR?

you want to make a new PR?

Sure thing.

@neha-ojha might want to add this PR to your batches, as rados/cephadm will be broken otherwise

green: https://pulpito.ceph.com/swagner-2020-08-28_15:31:32-rados:cephadm-wip-swagner-testing-2020-08-28-1411-distro-basic-smithi/

Hm. An idea would be to get this merged now to fix the rados suite and then follow-up with a proper refactorization.

@jan--f would that work?

@neha-ojha might want to add this PR to your batches, as rados/cephadm will be broken otherwise

@sebastian-philipp could you please do me a favor and ping me as well when we have such a failure in future?

Hm. An idea would be to get this merged now to fix the rados suite and then follow-up with a proper refactorization.

@jan--f would that work?

Sure or a revert would work too. I should have a proper fix later today.

Hm. ok then let's merge this and revert (while keeping the pytest) this commit in your PR?