-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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(core): Introduced Duration class #2857
Conversation
This can be used to model durations in a user-friendly way, while allowing coercion into the desired/required time unit at the usage site. There is now a default requirement that properties exposed by the APIs and that have a duration-like name (ending in `duration`, `period`, `timeout` or `ttl`) have a type of `@aws-cdk/cdk.Duration` and do not contain a unit suffix (`Days`, `Millis`, `Seconds`, `Sec`, ...). BREAKING CHANGE: Properties throughout the AWS Construct Libraries that represent lengths of time have been re-typed to be `@aws-cdk/cdk.Duration` instead of `number`, and were renamed to exclude any unit indication.
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.
just realized I had some comments pending
packages/@aws-cdk/aws-applicationautoscaling/lib/target-tracking-scaling-policy.ts
Show resolved
Hide resolved
1. Dropped the free-floating toSeconds function 2. Added test for unresolved tokens in constructor 3. Made .toString() return a breaking token.
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.
Looks super slick... See my comment about tokens. I am okay if we want to introduce it at a subsequent PR, but make sure to open an issue.
@@ -88,8 +88,14 @@ | |||
"@aws-cdk/aws-s3": "^0.35.0", | |||
"@aws-cdk/cdk": "^0.35.0" | |||
}, | |||
"awslint": { | |||
"exclude": [ | |||
"duration-prop-type:@aws-cdk/aws-cloudtrail.TrailProps.cloudWatchLogsRetentionTimeDays", |
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.
Should we just call this cloudWatchLogsRetention
?
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 reckon we should!
* docs(ecr): fix deprecated requirement maxImageAgeDays - on [Automatically clean up repositories](https://docs.aws.amazon.com/cdk/api/latest/docs/aws-ecr-readme.html\#automatically-clean-up-repositories) of Amazon ECR docs, there is deprecated property using on addLifecycleRule example - due to introduce of Duration Class(#2857), it should be maxImageAge rather than maxImageAgeDays * chore(ecr): fix deprecated requirement maxImageAgeDays - maxImageAgeDays were deprecated since #2857, but still used on comments. - should be chaged to maxImageAge - fixes #3273
* docs(ecr): fix deprecated requirement maxImageAgeDays - on [Automatically clean up repositories](https://docs.aws.amazon.com/cdk/api/latest/docs/aws-ecr-readme.html\#automatically-clean-up-repositories) of Amazon ECR docs, there is deprecated property using on addLifecycleRule example - due to introduce of Duration Class(aws#2857), it should be maxImageAge rather than maxImageAgeDays * chore(ecr): fix deprecated requirement maxImageAgeDays - maxImageAgeDays were deprecated since aws#2857, but still used on comments. - should be chaged to maxImageAge - fixes aws#3273
Fixed example to align with changes from [this commit](aws#2857)
Fixed example to align with changes from [this commit](#2857)
This can be used to model durations in a user-friendly way, while
allowing coercion into the desired/required time unit at the usage site.
There is now a default requirement that properties exposed by the APIs
and that have a duration-like name (ending in
duration
,period
,timeout
orttl
) have a type of@aws-cdk/cdk.Duration
and do notcontain a unit suffix (
Days
,Millis
,Seconds
,Sec
, ...).BREAKING CHANGE: Properties throughout the AWS Construct Libraries that
represent lengths of time have been re-typed to be
@aws-cdk/cdk.Duration
instead ofnumber
, and wererenamed to exclude any unit indication.
Pull Request Checklist
design
folderBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.