chore(ec2): dl2q instance type, IntanceType fixes#29481
chore(ec2): dl2q instance type, IntanceType fixes#29481mergify[bot] merged 5 commits intoaws:mainfrom
IntanceType fixes#29481Conversation
paulhcsun
left a comment
There was a problem hiding this comment.
Just a couple questions.
| * High performance computing powered by AWS Trainium | ||
| */ | ||
| TRN1 = 'trn1', | ||
| TRAINING_ACCELERATOR1 = 'training-accelerator1', |
There was a problem hiding this comment.
Where are these descriptions pulled from? Some of them seem mismatched after the changes, or were they incorrect before?
There was a problem hiding this comment.
Neither trn1 nor trn1n had a symbolic "alias" given to them when they were added, unlike every other instance class. I added one, didn't get it from anywhere really, looked up the AWS docs and tried to match the other's.
| * Network-optimized high performance computing powered by AWS Trainium | ||
| */ | ||
| TRN1N = 'trn1n', |
There was a problem hiding this comment.
Description for this has changed.
There was a problem hiding this comment.
Yeah, the trn1n description didn't have the network optimization discriminator, both trn1* had the same description:
aws-cdk/packages/aws-cdk-lib/aws-ec2/lib/instance-types.ts
Lines 614 to 622 in fac4a9c
There was a problem hiding this comment.
Gotcha, thanks for clarifying. Waiting for one other PR to merge in and will approve this after that.
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Issue # (if applicable)
None
Reason for this change
Follow up to #29427. Added a missing instance type, and minor fixes to the
IntanceTypeclassDescription of changes
I'd missed an instance type,
dl2q, which was neither inus-east-1orus-east-2, but inus-west-2.I've also added a couple of missing symbolic names, as well as fixed some differing comments between the key and its symbolic value (e.g.
M3andSTANDARD3)I also re-ordered a couple of enum values, when the symbolic value was separated from its match
Description of how you validated changes
Compared the CDK to the SDK to find the missing instance. Programmatically iterated over the comments of
IntanceTypeto make sure the comments of the symbolic keys matched the one below.Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license