{"action":"created","issue":{"url":"https://api.github.com/repos/saltstack/salt/issues/50124","repository_url":"https://api.github.com/repos/saltstack/salt","labels_url":"https://api.github.com/repos/saltstack/salt/issues/50124/labels{/name}","comments_url":"https://api.github.com/repos/saltstack/salt/issues/50124/comments","events_url":"https://api.github.com/repos/saltstack/salt/issues/50124/events","html_url":"https://github.com/saltstack/salt/pull/50124","id":371757870,"node_id":"MDExOlB1bGxSZXF1ZXN0MjI0MTM0NDQy","number":50124,"title":"make salt-api arg/kwarg handling the same as cli for all clients","user":{"login":"mattp-","id":44374,"node_id":"MDQ6VXNlcjQ0Mzc0","avatar_url":"https://avatars1.githubusercontent.com/u/44374?v=4","gravatar_id":"","url":"https://api.github.com/users/mattp-","html_url":"https://github.com/mattp-","followers_url":"https://api.github.com/users/mattp-/followers","following_url":"https://api.github.com/users/mattp-/following{/other_user}","gists_url":"https://api.github.com/users/mattp-/gists{/gist_id}","starred_url":"https://api.github.com/users/mattp-/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mattp-/subscriptions","organizations_url":"https://api.github.com/users/mattp-/orgs","repos_url":"https://api.github.com/users/mattp-/repos","events_url":"https://api.github.com/users/mattp-/events{/privacy}","received_events_url":"https://api.github.com/users/mattp-/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"gtmanfred","id":732321,"node_id":"MDQ6VXNlcjczMjMyMQ==","avatar_url":"https://avatars0.githubusercontent.com/u/732321?v=4","gravatar_id":"","url":"https://api.github.com/users/gtmanfred","html_url":"https://github.com/gtmanfred","followers_url":"https://api.github.com/users/gtmanfred/followers","following_url":"https://api.github.com/users/gtmanfred/following{/other_user}","gists_url":"https://api.github.com/users/gtmanfred/gists{/gist_id}","starred_url":"https://api.github.com/users/gtmanfred/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gtmanfred/subscriptions","organizations_url":"https://api.github.com/users/gtmanfred/orgs","repos_url":"https://api.github.com/users/gtmanfred/repos","events_url":"https://api.github.com/users/gtmanfred/events{/privacy}","received_events_url":"https://api.github.com/users/gtmanfred/received_events","type":"User","site_admin":false},"assignees":[{"login":"gtmanfred","id":732321,"node_id":"MDQ6VXNlcjczMjMyMQ==","avatar_url":"https://avatars0.githubusercontent.com/u/732321?v=4","gravatar_id":"","url":"https://api.github.com/users/gtmanfred","html_url":"https://github.com/gtmanfred","followers_url":"https://api.github.com/users/gtmanfred/followers","following_url":"https://api.github.com/users/gtmanfred/following{/other_user}","gists_url":"https://api.github.com/users/gtmanfred/gists{/gist_id}","starred_url":"https://api.github.com/users/gtmanfred/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gtmanfred/subscriptions","organizations_url":"https://api.github.com/users/gtmanfred/orgs","repos_url":"https://api.github.com/users/gtmanfred/repos","events_url":"https://api.github.com/users/gtmanfred/events{/privacy}","received_events_url":"https://api.github.com/users/gtmanfred/received_events","type":"User","site_admin":false}],"milestone":null,"comments":6,"created_at":"2018-10-19T00:01:06Z","updated_at":"2018-10-24T00:08:56Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/saltstack/salt/pulls/50124","html_url":"https://github.com/saltstack/salt/pull/50124","diff_url":"https://github.com/saltstack/salt/pull/50124.diff","patch_url":"https://github.com/saltstack/salt/pull/50124.patch"},"body":"### What does this PR do?\r\nboth cherrypy and saltnado now pass all args through parse_input to\r\nallow for behavior equivalent to salt cli yaml based arg parsing. This\r\nwill allow for pepper to behave more like salt cli, or direct api users.\r\n\r\nalso included is a minor feature to saltnado/cherrypy, a new X-Salt-Version\r\nheader for usage in pepper to determine if this feature among others\r\nis available to use.\r\n\r\n\r\n### What issues does this PR fix or reference?\r\nwill fix https://github.com/saltstack/pepper/issues/99 when equivalent pepper changes are made.\r\n\r\n### Previous Behavior\r\nyaml based foo=bar args are not passed through parse_input\r\n\r\n### New Behavior\r\nargs are passed through parse_input\r\n\r\n### Tests written?\r\nyes\r\n\r\n### Commits signed with GPG?\r\nNo\r\n\r\nPlease review [Salt's Contributing Guide](https://docs.saltstack.com/en/latest/topics/development/contributing.html) for best practices.\r\n\r\nSee GitHub's [page on GPG signing](https://help.github.com/articles/signing-commits-using-gpg/) for more information about signing commits with GPG.\r\n"},"comment":{"url":"https://api.github.com/repos/saltstack/salt/issues/comments/432465398","html_url":"https://github.com/saltstack/salt/pull/50124#issuecomment-432465398","issue_url":"https://api.github.com/repos/saltstack/salt/issues/50124","id":432465398,"node_id":"MDEyOklzc3VlQ29tbWVudDQzMjQ2NTM5OA==","user":{"login":"whiteinge","id":91293,"node_id":"MDQ6VXNlcjkxMjkz","avatar_url":"https://avatars2.githubusercontent.com/u/91293?v=4","gravatar_id":"","url":"https://api.github.com/users/whiteinge","html_url":"https://github.com/whiteinge","followers_url":"https://api.github.com/users/whiteinge/followers","following_url":"https://api.github.com/users/whiteinge/following{/other_user}","gists_url":"https://api.github.com/users/whiteinge/gists{/gist_id}","starred_url":"https://api.github.com/users/whiteinge/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/whiteinge/subscriptions","organizations_url":"https://api.github.com/users/whiteinge/orgs","repos_url":"https://api.github.com/users/whiteinge/repos","events_url":"https://api.github.com/users/whiteinge/events{/privacy}","received_events_url":"https://api.github.com/users/whiteinge/received_events","type":"User","site_admin":false},"created_at":"2018-10-24T00:08:56Z","updated_at":"2018-10-24T00:08:56Z","author_association":"CONTRIBUTOR","body":"Thanks for the ping, @rallytime. This functionality should already be baked into Salt and I did not include this functionality in salt-api on purpose for the reasons described below. That said, I'll happily defer to the wishes of the core team and any contributors that want to put in the actual work. Please consider or disregard as you see fit.\r\n\r\nParsing the `arg=val` strings into kwargs is a central workflow in Salt and thus should live in a central place and work consistently regardless if you are calling Salt from the CLI, from a REST API, from a Python wrapper, HTML form fields, a Slack bot, and many, many other examples we've seen. The best central place to do that is Salt's Python API (`LocalClient`, `RunnerClient`, `WheelClient`, etc). That is what the above-linked Pepper issue is discussing. All those client interfaces **should** run the `arg` (singular) array values through the utils function we have that splits `arg=val` strings apart and parses the right-hand side of that as YAML. I know for certain this has worked correctly in the past for LocalClient and RunnerClient (I don't recall offhand where WheelClient stood), because I have used it many, many times and even wrote tests for some of it. However this functionality has also regressed more than once (again the Pepper discussion) and I can't speak to the current state of things.\r\n\r\nhttps://github.com/saltstack/salt/blob/23e0d40d833c965fe573bee88c11be677f5b8e5c/tests/integration/client/test_runner.py#L108-L135\r\n\r\nIMO the best fix is to figure out how to prevent future regressions, to keep this functionality in the `*client` interfaces and to _not_ to bake this into salt-api.\r\n\r\nA concrete example of why this is best done in Salt instead of salt-api is [this discussion about the Slack engine](https://github.com/saltstack/salt/issues/39783#issuecomment-284116852). A Slack bot would be accepting `arg=val` strings because people are typing them into a chat interface -- just like the CLI, just like webhooks, just like Pepper, just like HTML forms, etc. It would be ideal for any module type that goes through the Python API (not just the REST API which of course uses the Python API, just like all the other module types) to not have to reinvent this over and over.\r\n\r\nIt is easier to put this into salt-api, for sure, and fixing this (and avoiding future regressions) in core may be quite difficult indeed (https://github.com/saltstack/salt/pull/39472). But I do feel that is the better solution.\r\n\r\nMy $0.02. I sincerely wish whoever takes either path good luck and godspeed. 😄 "}}
{"actor":{"display_login":"whiteinge"}}