aws: Improve platform lookup handling #757
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Brian Barrett <[email protected]>
Refactor the get_platform_data() call to expose the critical pieces to allow for unit testing. Signed-off-by: Brian Barrett <[email protected]>
Writing regexes is hard. Making sure your regexes work without a test is harder. So we should have a unit test for the code to do instance type -> config block lookup. Signed-off-by: Brian Barrett <[email protected]>
mozarhua
previously approved these changes
Dec 20, 2024
|
bot:aws:retest |
aws-nslick
reviewed
Dec 21, 2024
aws-nslick
reviewed
Dec 21, 2024
Add an explicit (optional) field for the platform regex, and make the name an id rather than a regex. This means that we can change the regex without having to update all the unit tests to match. Signed-off-by: Brian Barrett <[email protected]>
Add a regex to match all instances p5 or later in the p-series. Moved to extended regex compilation to make the regex slightly more sane. Signed-off-by: Brian Barrett <[email protected]>
bwbarrett
force-pushed
the
cleanup/platform-regex-life-is-better-with-unit-tests
branch
from
6d676a7 to
0fea146
Compare
aws-nslick
approved these changes
Dec 21, 2024
rajachan
approved these changes
Jan 6, 2025
bwbarrett
deleted the
cleanup/platform-regex-life-is-better-with-unit-tests
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was originally just a patch to add unit tests for the regex handling, but also tagged on a commit to change the p5 match to match all p5 or later platforms, because it's a reasonable example. Ended up splitting the name into a name and regex, otherwise every time you changed the regex (like the p5 extension), you had to update the unit test, and that was just annoying. Especially during the inevitable messing up regex cycle.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.