-
Notifications
You must be signed in to change notification settings - Fork 220
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 docs and validation for endpoint patterns #2069
Add docs and validation for endpoint patterns #2069
Conversation
…lEndpoints and standardPartitionalEndpoints
...c/main/java/software/amazon/smithy/rulesengine/aws/validators/EndpointModifierValidator.java
Outdated
Show resolved
Hide resolved
Matcher m = PATTERN.matcher(endpoint); | ||
while (m.find()) { | ||
if (!SUPPORTED_PATTERNS.contains(m.group())) { | ||
events.add(danger( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can these be aggregated to a list so that one event is emitted for an invalid endpoint pattern with any amount of unsupported labels in it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah - will combine.
ServiceShape serviceShape, StandardRegionalEndpointsTrait regionalEndpoints) { | ||
List<ValidationEvent> events = new ArrayList<>(); | ||
|
||
regionalEndpoints.getRegionSpecialCases().forEach((region, specialCases) -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since region
looks unused, you could iterate over the values()
instead of the whole map.
for (RegionSpecialCase specialCase : regionalEndpoints.getRegionSpecialCases().values())
nit: also prefer for
loops to forEach
streaming iteration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feedback applies to the Partitional
branch as well, with the naming replaced.
|
||
private List<ValidationEvent> validatePartitionalEndpointPatterns( | ||
ServiceShape serviceShape, StandardPartitionalEndpointsTrait partitionalEndpoints) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: rm
Description of changes:
Add docs and validation for endpoint patterns used by standardRegionalEndpoints and standardPartitionalEndpoints.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.