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

Using split instead of regex for simple key=value type tags. #1968

Merged
merged 4 commits into from
May 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 33 additions & 8 deletions samcli/cli/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,14 +204,19 @@ def convert(self, value, param, ctx):
value = (value,) if not isinstance(value, tuple) else value

for val in value:
groups = re.findall(self._pattern, val)

if not groups:
fail = True
for group in groups:
key, v = group
# assign to result['KeyName1'] = string and so on.
result[_unquote_wrapped_quotes(key)] = _unquote_wrapped_quotes(v)
# Using standard parser first. We should implement other type parser like JSON and Key=key,Value=val type format.
parsed, k, v = self._standard_key_value_parser(val)
Copy link
Contributor

Choose a reason for hiding this comment

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

should we check for more than 1 count, given that aws cli already does it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

aws-cli simply pick first =. I think our existing solution already handles more edge cases than aws-cli, we should keep using it until we implement separate parses.

Copy link
Contributor

Choose a reason for hiding this comment

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

right, but this solution only looks for one key value pair right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, we get list of key=value tags. We parse each one. They are already separated by command and handled by click.

if parsed:
result[_unquote_wrapped_quotes(k)] = _unquote_wrapped_quotes(v)
else:
groups = re.findall(self._pattern, val)

if not groups:
fail = True
for group in groups:
key, v = group
# assign to result['KeyName1'] = string and so on.
result[_unquote_wrapped_quotes(key)] = _unquote_wrapped_quotes(v)

if fail:
return self.fail(
Expand All @@ -221,3 +226,23 @@ def convert(self, value, param, ctx):
)

return result

@staticmethod
def _standard_key_value_parser(tag_value):
"""
Method to parse simple `Key=Value` type tags without using regex. This is similar to how aws-cli does this.
https://github.com/aws/aws-cli/blob/eff79a263347e8e83c8a2cc07265ab366315a992/awscli/customizations/cloudformation/deploy.py#L361
Parameters
----------
tag_value

Returns
-------

"""
equals_count = tag_value.count("=")
if equals_count != 1:
return False, None, None

splits = tag_value.split("=")
return True, splits[0], splits[1]
5 changes: 5 additions & 0 deletions tests/unit/cli/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,11 @@ def test_must_fail_on_invalid_format(self, input):
(("a=b", "c=d"), {"a": "b", "c": "d"}),
(('"a+-=._:/@"="b+-=._:/@" "--c="="=d/"',), {"a+-=._:/@": "b+-=._:/@", "--c=": "=d/"}),
(('owner:name="son of anton"',), {"owner:name": "son of anton"}),
(("a=012345678901234567890123456789",), {"a": "012345678901234567890123456789"}),
(
("a=012345678901234567890123456789", "c=012345678901234567890123456789"),
{"a": "012345678901234567890123456789", "c": "012345678901234567890123456789"},
),
(("",), {}),
]
)
Expand Down