Update ARN delimiter parsing in endpoint provider #2831
Conversation
dlm6693
force-pushed
the
arn-delimiter-endpoints
branch
from
b4fc59f to
21cb317
Compare
Codecov ReportBase: 93.53% // Head: 93.53% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## develop #2831 +/- ##
========================================
Coverage 93.53% 93.53%
========================================
Files 63 63
Lines 13382 13382
========================================
Hits 12517 12517
Misses 865 865
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
As discussed elsewhere already, using the regex in this case is approximately 3x slower in my tests than the non-regex solution:
"abc:def/ghi".replace(":", "/"). split("/")
['abc', 'def', 'ghi']If you decide to stick with the regex implementation, a nit: ARN_DELIMITER_RE would be more precise if called ARN_RESOURCE_DELIMITER_RE because "ARN delimiter" usually refers to the : that separate the top-level parts of the ARN.
| @@ -236,8 +237,7 @@ def aws_parse_arn(self, value): | |||
| arn_dict["accountId"] = arn_dict.pop("account") | |||
|
|
|||
| resource = arn_dict.pop("resource") | |||
| delimiter = ":" if ":" in resource else "/" | |||
| arn_dict["resourceId"] = resource.split(delimiter) | |||
| arn_dict["resourceId"] = re.split(ARN_DELIMITER_RE, resource) | |||
There was a problem hiding this comment.
I'm not sure if there is a difference, but why not use split on the compiled object?
| arn_dict["resourceId"] = re.split(ARN_DELIMITER_RE, resource) | |
| arn_dict["resourceId"] = ARN_DELIMITER_RE.split(resource) |
|
@jonemo thanks for the feedback. I did some testing of my own and I do also see that the standard string splitting in Python is much faster than using regex. We needed to get this merged before today's release otherwise I would've waited for more feedback, but I'll put up a followup PR that we can merge later today or tomorrow. |
* release-1.29.28: Bumping version to 1.29.28 Update to latest partitions and endpoints Update to latest models Update ARN delimiter parsing in endpoint provider (#2831)
Specifically, the previous implementation did not handle ARNs that use both colon and slashes as delimeters. This is a port from the original implementation across the following PRs: * boto/botocore#2831 * boto/botocore#2834
This addresses a bug for
aws_parse_arninendpoint_provider.RuleSetStandardLibrary. Currently theresourceIdportion of a parsed ARN was only thought to either have a/or:as a delimiter. However, in services such askinesis, they can have both.Current Behavior
arn:aws:kinesis:1234567890:us-east-1:thing/foo:bar->{...'resourceId': ['thing/foo', 'bar']}New Behavior
arn:aws:kinesis:1234567890:us-east-1:thing/foo:bar->{...'resourceId': ['thing', 'foo', 'bar']}