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

Added capability to transform_array function to escape colons #3902

Closed
wants to merge 1 commit into from

Conversation

Mierdin
Copy link
Member

@Mierdin Mierdin commented Dec 7, 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 use cases 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 PR builds on the work in #3670 and uses a negative lookbehind regular expression to ensure that the colon in question isn't preceded by a backslash: \. So, the user can place this behind the colon, and this conversion will be skipped.

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

Mierdin commented Dec 7, 2017

Some internal feedback seemed to indicate that this approach wasn't ideal for some users. Should ponder the approach further - maybe instead of allowing escape, provide a CLI option to st2 run to skip this conversion entirely.

@Kami
Copy link
Member

Kami commented Dec 11, 2017

Good catch 👍

And yeah we have a lot of code which allows users to specify complex type in an "easier" manner, but in general that's quite hard and there are a lot of edge cases.

Are we targeting this for v2.5.1?

mockarg = mock.Mock()
mockarg.inherit_env = False
mockarg.parameters = [
'param_array=foo\:bar,foo2:bar2',
Copy link
Member

Choose a reason for hiding this comment

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

If we decide to go with this approach, some more test cases would be great (e.g. more than two items, etc.)

@Kami Kami added this to the 2.5.1 milestone Dec 11, 2017
@Mierdin
Copy link
Member Author

Mierdin commented Dec 12, 2017

Closing in favor of #3909

@Mierdin Mierdin closed this Dec 12, 2017
@LindsayHill LindsayHill deleted the escape-colon-formatter branch October 3, 2018 19:04
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

2 participants