Skip to content

Commit

Permalink
fix(core/events/appscaling): allow schedule in application autoscalin…
Browse files Browse the repository at this point in the history
…g and events to consider token in duration (#9413)

feat(events,applicationautoscaling): schedule to check for
minutes/hours/days, fixed cr comments, Fix for Issue: #9413

feat(events,applicationautoscaling): schedule fixed cr comments
  • Loading branch information
sneharathod authored and Sneha Solanki committed Mar 5, 2021
1 parent 80b10f8 commit fbd9d8b
Show file tree
Hide file tree
Showing 8 changed files with 160 additions and 7 deletions.
7 changes: 7 additions & 0 deletions packages/@aws-cdk/aws-applicationautoscaling/lib/schedule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ export abstract class Schedule {
* Construct a schedule from an interval and a time unit
*/
public static rate(duration: Duration): Schedule {
const validDurationUnit = ['minute', 'minutes', 'hour', 'hours', 'day', 'days'];
if (!validDurationUnit.includes(duration.unitLabel())) {
throw new Error("Allowed unit for scheduling is: 'minute', 'minutes', 'hour', 'hours', 'day' or 'days'");
}
if (duration.isUnresolved()) {
return new LiteralSchedule(`rate(${duration.formatTokenToNumber()})`);
}
if (duration.toSeconds() === 0) {
throw new Error('Duration cannot be 0');
}
Expand Down
27 changes: 24 additions & 3 deletions packages/@aws-cdk/aws-applicationautoscaling/test/test.cron.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Duration } from '@aws-cdk/core';
import { Duration, Stack, Lazy } from '@aws-cdk/core';
import { Test } from 'nodeunit';
import * as appscaling from '../lib';

Expand All @@ -15,8 +15,15 @@ export = {

'rate must be whole number of minutes'(test: Test) {
test.throws(() => {
appscaling.Schedule.rate(Duration.seconds(12345));
}, /'12345 seconds' cannot be converted into a whole number of minutes/);
appscaling.Schedule.rate(Duration.minutes(0.13456));
}, /'0.13456 minutes' cannot be converted into a whole number of seconds/);
test.done();
},

'rate must not be in seconds'(test: Test) {
test.throws(() => {
appscaling.Schedule.rate(Duration.seconds(1));
}, /Allowed unit for scheduling is: 'minute', 'minutes', 'hour', 'hours', 'day' or 'days'/);
test.done();
},

Expand All @@ -26,4 +33,18 @@ export = {
}, /Duration cannot be 0/);
test.done();
},

'rate can be token'(test: Test) {
const stack = new Stack();
const lazyDuration = Duration.minutes(Lazy.number({ produce: () => 5 }));
const rate = appscaling.Schedule.rate(lazyDuration);
test.equal('rate(5 minutes)', stack.resolve(rate).expressionString);
test.done();
},

'rate can be in allowed type hours'(test: Test) {
test.equal('rate(1 hour)', appscaling.Schedule.rate(Duration.hours(1))
.expressionString);
test.done();
},
};
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-events/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ onCommitRule.addTarget(new targets.SnsTopic(topic, {
## Scheduling

You can configure a Rule to run on a schedule (cron or rate).
Rate must be specified in minutes, hours or days.

The following example runs a task every day at 4am:

Expand Down
7 changes: 7 additions & 0 deletions packages/@aws-cdk/aws-events/lib/schedule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ export abstract class Schedule {
* Construct a schedule from an interval and a time unit
*/
public static rate(duration: Duration): Schedule {
const validDurationUnit = ['minute', 'minutes', 'hour', 'hours', 'day', 'days'];
if (validDurationUnit.indexOf(duration.unitLabel()) === -1) {
throw new Error('Allowed unit for scheduling is: \'minute\', \'minutes\', \'hour\', \'hours\', \'day\', \'days\'');
}
if (duration.isUnresolved()) {
return new LiteralSchedule(`rate(${duration.formatTokenToNumber()})`);
}
if (duration.toSeconds() === 0) {
throw new Error('Duration cannot be 0');
}
Expand Down
33 changes: 33 additions & 0 deletions packages/@aws-cdk/aws-events/test/test.rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,39 @@ export = {
test.done();
},

'get rate as token'(test: Test) {
const app = new cdk.App();
const stack = new cdk.Stack(app, 'MyScheduledStack');
const lazyDuration = cdk.Duration.minutes(cdk.Lazy.number({ produce: () => 5 }));

new Rule(stack, 'MyScheduledRule', {
ruleName: 'rateInMinutes',
schedule: Schedule.rate(lazyDuration),
});

// THEN
expect(stack).to(haveResourceLike('AWS::Events::Rule', {
'Name': 'rateInMinutes',
'ScheduleExpression': 'rate(5 minutes)',
}));

test.done();
},

'Seconds is not an allowed value for Schedule rate'(test: Test) {
const lazyDuration = cdk.Duration.seconds(cdk.Lazy.number({ produce: () => 5 }));
test.throws(() => Schedule.rate(lazyDuration), /Allowed unit for scheduling is: 'minute', 'minutes', 'hour', 'hours', 'day', 'days'/);
test.done();
},

'Millis is not an allowed value for Schedule rate'(test: Test) {
const lazyDuration = cdk.Duration.millis(cdk.Lazy.number({ produce: () => 5 }));

// THEN
test.throws(() => Schedule.rate(lazyDuration), /Allowed unit for scheduling is: 'minute', 'minutes', 'hour', 'hours', 'day', 'days'/);
test.done();
},

'rule with physical name'(test: Test) {
// GIVEN
const stack = new cdk.Stack();
Expand Down
42 changes: 39 additions & 3 deletions packages/@aws-cdk/aws-events/test/test.schedule.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Duration } from '@aws-cdk/core';
import { Duration, Stack, Lazy } from '@aws-cdk/core';
import { Test } from 'nodeunit';
import * as events from '../lib';

Expand Down Expand Up @@ -33,8 +33,15 @@ export = {

'rate must be whole number of minutes'(test: Test) {
test.throws(() => {
events.Schedule.rate(Duration.seconds(12345));
}, /'12345 seconds' cannot be converted into a whole number of minutes/);
events.Schedule.rate(Duration.minutes(0.13456));
}, /'0.13456 minutes' cannot be converted into a whole number of seconds/);
test.done();
},

'rate must be whole number'(test: Test) {
test.throws(() => {
events.Schedule.rate(Duration.minutes(1/8));
}, /'0.125 minutes' cannot be converted into a whole number of seconds/);
test.done();
},

Expand All @@ -44,4 +51,33 @@ export = {
}, /Duration cannot be 0/);
test.done();
},

'rate can be from a token'(test: Test) {
const stack = new Stack();
const lazyDuration = Duration.minutes(Lazy.number({ produce: () => 5 }));
const rate = events.Schedule.rate(lazyDuration);
test.equal('rate(5 minutes)', stack.resolve(rate).expressionString);
test.done();
},

'rate can be in minutes'(test: Test) {
test.equal('rate(10 minutes)',
events.Schedule.rate(Duration.minutes(10))
.expressionString);
test.done();
},

'rate can be in days'(test: Test) {
test.equal('rate(10 days)',
events.Schedule.rate(Duration.days(10))
.expressionString);
test.done();
},

'rate can be in hours'(test: Test) {
test.equal('rate(10 hours)',
events.Schedule.rate(Duration.hours(10))
.expressionString);
test.done();
},
};
24 changes: 23 additions & 1 deletion packages/@aws-cdk/core/lib/duration.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Token } from './token';
import { Token, Tokenization } from './token';

/**
* Represents a length of time.
Expand Down Expand Up @@ -252,6 +252,28 @@ export class Duration {
}
return ret;
}

/**
* Checks if duration is a token or a resolvable object
*/
public isUnresolved() {
return Token.isUnresolved(this.amount);
}

/**
* Returns unit of the duration
*/
public unitLabel() {
return this.unit.label;
}

/**
* Returns stringified number of duration
*/
public formatTokenToNumber(): string {
const number = Tokenization.stringifyNumber(this.amount);
return `${number} ${this.unit.label}`;
}
}

/**
Expand Down
26 changes: 26 additions & 0 deletions packages/@aws-cdk/core/test/duration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,32 @@ nodeunitShim({

test.done();
},

'get unit label from duration'(test: Test) {
test.equal(Duration.minutes(Lazy.number({ produce: () => 10 })).unitLabel(), 'minutes');
test.equal(Duration.minutes(62).unitLabel(), 'minutes');
test.equal(Duration.seconds(10).unitLabel(), 'seconds');
test.equal(Duration.millis(1).unitLabel(), 'millis');
test.equal(Duration.hours(1000).unitLabel(), 'hours');
test.equal(Duration.days(2).unitLabel(), 'days');
test.done();
},

'format number token to number'(test: Test) {
const stack = new Stack();
const lazyDuration = Duration.minutes(Lazy.number({ produce: () => 10 }));
test.equal(stack.resolve(lazyDuration.formatTokenToNumber()), '10 minutes');
test.equal(Duration.hours(10).formatTokenToNumber(), '10 hours');
test.equal(Duration.days(5).formatTokenToNumber(), '5 days');
test.done();
},

'duration is unresolved'(test: Test) {
const lazyDuration = Duration.minutes(Lazy.number({ produce: () => 10 }));
test.equal(lazyDuration.isUnresolved(), true);
test.equal(Duration.hours(10).isUnresolved(), false);
test.done();
},
});

function floatEqual(test: Test, actual: number, expected: number) {
Expand Down

0 comments on commit fbd9d8b

Please sign in to comment.