fix(events,applicationautoscaling): specifying a schedule rate in seconds results in an error #13689
Conversation
|
|
github-actions
bot
added
@aws-cdk/aws-applicationautoscaling
| if (duration.isUnresolved()) { | ||
| const validDurationUnit = ['minute', 'minutes', 'hour', 'hours', 'day', 'days']; | ||
| if (!validDurationUnit.includes(duration.unitLabel())) { | ||
| throw new Error("Allowed units for scheduling are: 'minute', 'minutes', 'hour', 'hours', 'day' or 'days'"); | ||
| } | ||
| return new LiteralSchedule(`rate(${duration.formatTokenToNumber()})`); | ||
| } |
There was a problem hiding this comment.
| if (duration.isUnresolved()) { | |
| const validDurationUnit = ['minute', 'minutes', 'hour', 'hours', 'day', 'days']; | |
| if (!validDurationUnit.includes(duration.unitLabel())) { | |
| throw new Error("Allowed units for scheduling are: 'minute', 'minutes', 'hour', 'hours', 'day' or 'days'"); | |
| } | |
| return new LiteralSchedule(`rate(${duration.formatTokenToNumber()})`); | |
| } | |
| if (duration.isUnresolved()) { | |
| return new LiteralSchedule(`rate(${duration.formatTokenToNumber()})`); | |
| } | |
| const validDurationUnit = ['minute', 'minutes', 'hour', 'hours', 'day', 'days']; | |
| if (!validDurationUnit.includes(duration.unitLabel())) { | |
| throw new Error("Allowed units for scheduling are: 'minute', 'minutes', 'hour', 'hours', 'day' or 'days'"); | |
| } |
There was a problem hiding this comment.
GitHub is being weird, so I'll just comment here. It seems like there is still a problem in code, if duration is unresolved the validation will still happen?
There was a problem hiding this comment.
if duration is unresolved the validation will still happen?
Yes, that's the intended change here. If duration is a token, it is not possible to perform unit conversion. So the original units specified during construction of the Duration object will be used. However, AWS events has a fixed set of units and hence the validation is required.
If Duration is not a token, then the units can be converted from the unsupported set of units to the supported set of units.
Makes sense?
There was a problem hiding this comment.
Thanks! I missed that duration.isUnresolved() is calling the isUnresolved method from the Duration class rather than the Token.isUnresolved method
|
Thank you for contributing! Your pull request will be updated from master 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 master 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 master 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 master 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 |
…onds results in an error (#13689) A previous change - b1449a1 - introduced a validation that should have been applied only when Duration is specified as a token. Instead, it was applied for non token values as well. Adjusting the validation so it only applies to when the duration is a token. fixes #13566 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…onds results in an error (#13689) A previous change - b1449a1 - introduced a validation that should have been applied only when Duration is specified as a token. Instead, it was applied for non token values as well. Adjusting the validation so it only applies to when the duration is a token. fixes #13566 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…onds results in an error (aws#13689) A previous change - b1449a1 - introduced a validation that should have been applied only when Duration is specified as a token. Instead, it was applied for non token values as well. Adjusting the validation so it only applies to when the duration is a token. fixes aws#13566 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
A previous change - b1449a1 - introduced a validation that should have
been applied only when Duration is specified as a token. Instead, it was
applied for non token values as well.
Adjusting the validation so it only applies to when the duration is a
token.
fixes #13566
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license