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

Add flag to opt-in to dict conversion when colons are detected #3909

Merged
merged 2 commits into from Dec 12, 2017

Conversation

Mierdin
Copy link
Member

@Mierdin Mierdin commented Dec 12, 2017

#3670 introduced a cool new capability to split strings in an array parameter on a colon : character in order to specify a dictionary more easily.

However, this breaks a few corner cases, such as those where the user wants to provide an array of strings that all contain : and doesn't want them split. For instance, some network interfaces have a colon in their name and it shouldn't be split up.

This kicked off a long discussion internally, and we've decided to go for a more robust, exhaustive method of dealing with more complicated data types, likely to be implemented in the 2.6 release timeframe, perhaps 2.7.

Until that time, we need to take a stopgap measure to retain the functionality for those that need it, while getting things working again for some of the corner cases that don't want it. To that end, this PR implements a new flag --auto-dict, which you MUST specify to get the functionality introduced by #3670. The default functionality for st2 run will be to NOT interpret colons, but revert back to the way this was handled prior to #3670.

Note again that this flag is temporary and should not be relied upon long-term. Eventually, it will be replaced with more permanent, exhaustive handling of complex datatypes for action parameters.

Usage

I put together a simple action that just returns its input values so we can see how the parameters have or haven't been affected by the use of this new flag. Note that without the flag, the default is to behave as it was before - it will not split the list items into dicts, but rather leave them as strings:

$ st2 run testpack.pythontest testparam=key1:value1,key2:value2
.
id: 5a30328a32ed350bde0a7ea5
status: succeeded
parameters:
  testparam:
  - key1:value1
  - key2:value2
result:
  exit_code: 0
  result:
  - key1:value1
  - key2:value2
  stderr: ''
  stdout: ''

However, this functionality can be opted into by using the --auto-dict flag:

$ st2 run testpack.pythontest testparam=key1:value1,key2:value2 --auto-dict
.
id: 5a30329232ed350bde0a7ea8
status: succeeded
parameters:
  testparam:
  - key1: value1
    key2: value2
result:
  exit_code: 0
  result:
  - key1: value1
    key2: value2
  stderr: ''
  stdout: ''

Signed-off-by: Matt Oswalt <matt@keepingitclassless.net>
Copy link
Contributor

@humblearner humblearner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@samirsss
Copy link
Member

@Mierdin quick question about the behavior from the Web ui? How will this get handled?

@Mierdin
Copy link
Member Author

Mierdin commented Dec 12, 2017

@samirsss WebUI is unaffected, and is unrelated to this change as well as #3670

@Mierdin Mierdin merged commit 5df0efd into master Dec 12, 2017
Mierdin added a commit that referenced this pull request Dec 12, 2017
Signed-off-by: Matt Oswalt <matt@keepingitclassless.net>
@Mierdin Mierdin deleted the disable-dict-conversion-client branch December 12, 2017 23:06
humblearner added a commit that referenced this pull request Dec 13, 2017
Mierdin added a commit that referenced this pull request Dec 14, 2017
Signed-off-by: Matt Oswalt <matt@keepingitclassless.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants