-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(rds): timeout
and timeoutAction
properties to ServerlessCluster
#28534
feat(rds): timeout
and timeoutAction
properties to ServerlessCluster
#28534
Conversation
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.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request
. Additionally, if clarification is needed add Clarification Request
to a comment.
5091b1f
to
e39be9c
Compare
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
7ff08ca
to
a3b6616
Compare
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.
some comments mostly on naming and typos. Everything else looks good @badmintoncryer :)
* | ||
* @default - 300 (5 minutes) | ||
*/ | ||
readonly secondsBeforeTimeout? : Duration; |
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.
readonly secondsBeforeTimeout? : Duration; | |
readonly secondsBeforeTimeout?: Duration; |
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.
I think that this property needs to be changed to just timeout
. Since we're providing a Duration here, a user could provide minutes if they wanted to. readonly timeout?: Duration
makes the most sense
1f2cd3b
to
0dbf982
Compare
@kaizencc Thank you for your review! I have changed the variable name to 'timeout' and accordingly updated the tests and README as well. |
secondsBeforeTimeout
and timeoutAction
properties to ServerlessClustertimeout
and timeoutAction
properties to ServerlessCluster
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.
Thanks @badmintoncryer. Looks good, just need to update the integ test to get the build succeeding.
timeout
and timeoutAction
properties to ServerlessClustertimeout
and timeoutAction
properties to ServerlessCluster
Co-authored-by: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com>
Co-authored-by: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com>
Co-authored-by: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com>
Co-authored-by: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com>
9034014
to
e1234da
Compare
@kaizencc I fixed integ test. |
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.
A few review comments that I will try to merge to your branch and hopefully I didnt break the build
ScalingConfiguration: { | ||
AutoPause: true, | ||
MaxCapacity: 32, | ||
MinCapacity: 8, | ||
SecondsUntilAutoPause: 600, | ||
TimeoutAction: 'ForceApplyCapacityChange', | ||
}, |
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.
Whatever timeout maps to is missing from this assertion
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.
If all goes well, then approved :)
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). |
…ter (aws#28534) This pull request introduces two new properties to the `ServerlessCluster` class in the AWS CDK RDS package: `secondsBeforeTimeout` and `timeoutAction`. The `secondsBeforeTimeout` property allows users to specify the amount of time that Aurora Serverless v1 will attempt to find a scaling point to perform seamless scaling before enforcing the timeout action. The default value is 300 seconds (5 minutes). The `timeoutAction` property allows users to specify the action to take when the timeout is reached. Users can choose between `ForceApplyCapacityChange`, which will force the capacity to the specified value as soon as possible, even without a scaling point, and `RollbackCapacityChange`, which will ignore the capacity change if a scaling point is not found. The default behavior is `RollbackCapacityChange`. These enhancements provide users with more control over the scaling behavior of their Aurora Serverless clusters. Closes aws#27183 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This pull request introduces two new properties to the
ServerlessCluster
class in the AWS CDK RDS package:secondsBeforeTimeout
andtimeoutAction
.The
secondsBeforeTimeout
property allows users to specify the amount of time that Aurora Serverless v1 will attempt to find a scaling point to perform seamless scaling before enforcing the timeout action. The default value is 300 seconds (5 minutes).The
timeoutAction
property allows users to specify the action to take when the timeout is reached. Users can choose betweenForceApplyCapacityChange
, which will force the capacity to the specified value as soon as possible, even without a scaling point, andRollbackCapacityChange
, which will ignore the capacity change if a scaling point is not found. The default behavior isRollbackCapacityChange
.These enhancements provide users with more control over the scaling behavior of their Aurora Serverless clusters.
Closes #27183
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license