Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

draft support for parsing slot results and appending text #52187

Merged
merged 13 commits into from Apr 9, 2019

Conversation

mchugh19
Copy link
Contributor

@mchugh19 mchugh19 commented Mar 14, 2019

What does this PR do?

Extends slot syntax to support parsing resulting dictionaries and appending text. Note: the appending process only functions on slot results which are strings.

Next steps:

  • Since python structures are now injected with raw unicode: {u'shell': u'/bin/bash'} we could also force to json output if desired.

What issues does this PR fix or reference?

#50462
#46771

Previous Behavior

Using __slots__ with modules which return dictionaries required custom modules to parse result
Also, unable to append text

New Behavior

Support dictionary results by appending the slot function with period delimited dictionary keys to traverse
__slot__:salt:user.info(root).shell returns /bin/bash

Support appending data by using a tilde delimiter followed by text
__slot__:salt:user.info(root).shell ~ /appended~ returns /bin/bash/appended~

Tests written?

Yes

Commits signed with GPG?

No

Copy link
Contributor

@dwoz dwoz left a comment

@mchugh19 Can you please add a test or two that cover this?

@mchugh19
Copy link
Contributor Author

@mchugh19 mchugh19 commented Mar 17, 2019

Tests have been added for the traverse and append functions.

In thinking about this a little bit more, the parsing for the append function looks for a tilde character and appends everything afterwards. It is possible that the slot text itself could contain that character as you might have cmd.run('ls ~user') or something. Should this behavior be updated to a more complicated regex, or maybe rather than just the single tilde character, the syntax could be forced to ~ with spaces on either side of the tilde? What do you all think it best?

Copy link
Contributor Author

@mchugh19 mchugh19 left a comment

Tests added

dwoz
dwoz approved these changes Mar 20, 2019
@mchugh19
Copy link
Contributor Author

@mchugh19 mchugh19 commented Apr 2, 2019

@DmitryKuzmenko you should take a look here as well

@thatch45 thatch45 merged commit 166a33a into saltstack:develop Apr 9, 2019
7 of 11 checks passed
@mchugh19 mchugh19 deleted the slot-extend branch Jul 4, 2019
mchugh19 pushed a commit to mchugh19/salt that referenced this issue Oct 13, 2019
draft support for parsing slot results and appending text
@waynew waynew added this to PR needs port to master in PRs to port to master Oct 17, 2019
@waynew waynew added the has master-port label Oct 17, 2019
@waynew waynew moved this from PR needs port to master to PR has port to master in PRs to port to master Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has master-port
Projects
PRs to port to master
  
PR has port to master
Development

Successfully merging this pull request may close these issues.

None yet

6 participants