pepper vs. salt has different `test.arg_repr` output types about pepper HOT 11 CLOSED

@whiteinge nice and thanks for looking into it.

Actually the main problem for me was it exposed a bug in salt-virt which I now have a fix for and will get in later today.
So all good on my end 👍

from pepper.

Oh and just to be extra thorough, if you watch the salt-api trace logs you'll see the same types as well:

00:57:13 [28149] [TRACE   ] get_event() received = {'tag': 'salt/job/20170301005713642409/ret/sip-cannon', 'data': {'fun_args': ['trunkte
st2', '2', '2048', 'image=/var/lib/libvirt/images/centos66.qcow2', 'enable_vnc=True', 'config={"master": "goodboy.qa.sangoma.local"}', 's
tart=False', 'enable_qcow=True'], 'jid': '20170301005713642409', 'return': {'args': "('trunktest2', '2', '2048')", 'kwargs': '{\'__pub_us
er\': \'saltvirt\', \'__pub_arg\': [\'trunktest2\', \'2\', \'2048\', \'image=/var/lib/libvirt/images/centos66.qcow2\', \'enable_vnc=True\
', \'config={"master": "goodboy.qa.sangoma.local"}\', \'start=False\', \'enable_qcow=True\'], \'start\': False, \'config\': {\'master\': 
\'goodboy.qa.sangoma.local\'}, \'image\': \'/var/lib/libvirt/images/centos66.qcow2\', \'__pub_fun\': \'test.arg_repr\', \'enable_qcow\': 
True, \'__pub_jid\': \'20170301005713642409\', \'__pub_tgt\': \'sip-cannon\', \'__pub_tgt_type\': \'glob\', \'enable_vnc\': True, \'__pub
_ret\': \'\'}'}, 'retcode': 0, 'success': True, 'cmd': '_return', '_stamp': '2017-03-01T05:57:13.698160', 'fun': 'test.arg_repr', 'id': '
sip-cannon'}}                                                                                                                            

from pepper.

Coincidentally, there's an open PR in Salt that has a fix to make the foo=Foo processing identical to the salt CLI processing. Until that fix is merged and released the best way to preserve types is to use the new --json flag in Pepper (#98) to send JSON instead of name=value arguments.

from pepper.

Ninja edit: I failed at reading -- that PR is a fix for calling Runners. I'm not aware of a mismatch when calling execution functions on minions.

I'll take a closer look at this.

from pepper.

@whiteinge yeah I think I found the reason after using the python api.

args are shipped verbatim as parsed strings as opposed to converting key=val pairs into **kwargs as is done with the runner client.

The tricky part I'm not sure how to handle is how to deliver parsed **kwargs for each underlying client type. For example local takes them in a kwarg dict whilst runner takes them as **kwargs (which I don't think works by the way in salt core).

I can rummage up a PR for at least the local client if you're cool with it.

from pepper.

I haven't had a chance to take a look at this yet, but shipping args verbatim as name=value pairs is what we want Pepper to do. The reason for that is that the salt CLI command gets that same format from the shell and Salt actually splits them (and also runs the right-hand side through the YAML parser) which is what that parse_input function does. Pepper needs to be updated to also do that for runner calls (once the above PR is merged).

from pepper.

@whiteinge

shipping args verbatim as name=value pairs is what we want Pepper to do

kk that's fine then but I guess it's besides the point of the missing type casting?

Salt actually splits them (and also runs the right-hand side through the YAML parser) which is what that parse_input function does.

^ would make me think that once that's fixed then maybe this type difference problem goes away?

from pepper.

Yeah, I think so. Running the right-hand side through the YAML parser is what performs the type casting. E.g. foo=True becomes {"foo": true}. And that's what the CLI relies on for that too.

from pepper.

I can reproduce this. The problem is somewhere in salt-api or in Salt. Trying to find the culprit.

If you need a workaround in the meantime, sending JSON via the --json flag to Pepper will allow you to specify the right data types up-front rather than relying on the name=value parsing in Salt.

from pepper.

This will be fixed with the neon release of salt.

from pepper.

@gtmanfred woot glad to see it finally landed :)

from pepper.