Conversation
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
|
@Sas-ry thanks for submitting the PR ;) hope the contribution team will give it a review soon |
| this.instanceTypes = props.instanceTypes ?? []; | ||
| this.instanceClasses = props.instanceClasses ?? []; | ||
| if (this.images?.find(image => image.imageType === EcsMachineImageType.ECS_AL2023) && | ||
| if (this.images?.find(image => image.imageType === EcsMachineImageType.ECS_AL2023 || image.imageType === EcsMachineImageType.ECS_AL2023_NVIDIA) && |
There was a problem hiding this comment.
if( [ EcsMachineImageType.ECS_AL2023, EcsMachineImageType.ECS_AL2023_NVIDIA ].includes(image.imageType)) ...`There was a problem hiding this comment.
@XaaXaaX
Thanks for your review comment!
I think your suggestion is better than the current code, so I’ve applied it.
|
Hi maintainers, |
kumvprat
left a comment
There was a problem hiding this comment.
The changes look good. I have added a couple of comments on the PR
Will also take a merge from main so that we can take benefit of latest changes in main branch
| this.instanceTypes = props.instanceTypes ?? []; | ||
| this.instanceClasses = props.instanceClasses ?? []; | ||
| if (this.images?.find(image => image.imageType === EcsMachineImageType.ECS_AL2023) && | ||
| if (this.images?.find(image => image.imageType !== undefined && |
There was a problem hiding this comment.
The changes look good but if we can make the checks cleaner it will be helpful in long run to reason about it.
Something like :
if (this.images?.some(image =>
image.imageType === EcsMachineImageType.ECS_AL2023 ||
image.imageType === EcsMachineImageType.ECS_AL2023_NVIDIA
) && (
this.instanceClasses.includes(ec2.InstanceClass.A1) ||
this.instanceTypes.some(instanceType =>
instanceType.sameInstanceClassAs(ec2.InstanceType.of(ec2.InstanceClass.A1, ec2.InstanceSize.LARGE))
)
)) {
throw new ValidationError('Amazon Linux 2023 does not support A1 instances.', this);
}
| imageType: EcsMachineImageType.ECS_AL2023_NVIDIA, | ||
| }, | ||
| ], | ||
| })).toThrow('Amazon Linux 2023 does not support A1 instances.'); |
There was a problem hiding this comment.
nit : Can we have a distinct error message so it's easier to debug which machine type cause test failures ? (something like NVIDIA in the message will be helpful)
|
|
||||||||||||||
|
|
||||||||||||||
Issue # (if applicable)
#35839
Reason for this change
As explained in #35839, Amazon Linux 2 will reach its end of life (EOL) at the end of June 2026. Therefore, we have added a new option, “ECS_AL2023_NVIDIA,” to enable the specification of Amazon Linux 2023.
Description of changes
This release adds support for ECS_AL2023_NVIDIA to AWS Batch Compute Environment, enabling the execution of GPU workloads on Amazon Linux 2023.
Describe any new or updated permissions being added
N/A - Supports ECS_AL2023_NVIDIA
Description of how you validated changes
Unit tests and integration tests have been updated, and it is now possible to verify that both pass.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license