feat(sagemaker): add containerStartupHealthCheckTimeoutInSeconds support for EndpointConfig#35626
Conversation
5f7170a to
dee1fb1
Compare
dee1fb1 to
2a74cef
Compare
abidhasan-aws
left a comment
There was a problem hiding this comment.
Hi @amandladev,
Thanks for your contribution. I have added a few comments.
You might also need to resolve conflict.
:)
| * The timeout value, in seconds, for your inference container to pass health check. | ||
| * @default - none | ||
| */ | ||
| readonly containerStartupHealthCheckTimeout?: cdk.Duration; |
There was a problem hiding this comment.
We should remove containerStartupHealthCheckTimeout from ProductionVariantProps and keep it only in InstanceProductionVariantProps.
ContainerStartupHealthCheckTimeoutInSeconds is an instance-only property, it's not supported for serverless endpoints.
Pull request has been modified.
|
|
||||||||||||||
|
|
||||||||||||||
|
Hello @abidhasan-aws ! Thanks a lot for the feedback! I’ve applied the suggested changes and I’m around if there’s anything else to tweak. |
|
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). |
Merge Queue Status✅ The pull request has been merged at 2ec085e This pull request spent 5 hours 25 minutes 55 seconds in the queue, including 32 minutes 41 seconds running CI. Required conditions to merge
|
|
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). |
|
Comments on closed issues and PRs are hard for our team to see. |
Implements container startup health check timeout configuration for SageMaker endpoint production variants as available in CloudFormation but missing in CDK constructs.
Issue #35566
Reason for this change
AWS SageMaker EndpointConfig supports ContainerStartupHealthCheckTimeoutInSeconds in CloudFormation to configure health check timeout for inference containers, but this property is not exposed in the CDK SageMaker L2 constructs. Users with models that require longer initialization time cannot configure appropriate health check timeouts, leading to premature health check failures.
Description of changes
Implements AWS SageMaker container startup health check timeout support in CDK SageMaker L2 constructs, enabling users to configure appropriate health check timeouts for inference containers:
Range: 60-3600 seconds (1 minute to 1 hour)
Type: cdk.Duration for intuitive time specification
Optional property maintaining backward compatibility
Usage Example:
Describe any new or updated permissions being added
N/A - No new IAM permissions required. Leverages existing SageMaker endpoint configuration permissions.
Description of how you validated changes
Unit tests: Added 5 comprehensive container startup health check timeout tests covering all validation scenarios:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license