Skip to content

Commit

Permalink
fix(kinesis): retention period does not use Duration type (#7037)
Browse files Browse the repository at this point in the history
fix(kinesis): retention period does not use Duration type

Rationale: Consistency with how time intervals are defined across the construct library.
Also fixed previously broken validation on retention period and updated tests.

Closes #7036 
    
BREAKING CHANGE:
`retentionPeriodHours` is now `retentionPeriod` and of type `Duration`
  • Loading branch information
shivlaks authored Mar 27, 2020
1 parent 53efde5 commit 1186227
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 24 deletions.
12 changes: 6 additions & 6 deletions packages/@aws-cdk/aws-kinesis/lib/stream.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as iam from '@aws-cdk/aws-iam';
import * as kms from '@aws-cdk/aws-kms';
import { Construct, IResource, Resource, Stack } from '@aws-cdk/core';
import { Construct, Duration, IResource, Resource, Stack } from '@aws-cdk/core';
import { CfnStream } from './kinesis.generated';

/**
Expand Down Expand Up @@ -185,9 +185,9 @@ export interface StreamProps {

/**
* The number of hours for the data records that are stored in shards to remain accessible.
* @default 24
* @default Duration.hours(24)
*/
readonly retentionPeriodHours?: number;
readonly retentionPeriod?: Duration;

/**
* The number of shards for the stream.
Expand Down Expand Up @@ -261,9 +261,9 @@ export class Stream extends StreamBase {
});

const shardCount = props.shardCount || 1;
const retentionPeriodHours = props.retentionPeriodHours || 24;
if (retentionPeriodHours < 24 && retentionPeriodHours > 168) {
throw new Error("retentionPeriodHours must be between 24 and 168 hours");
const retentionPeriodHours = props.retentionPeriod?.toHours() ?? 24;
if (retentionPeriodHours < 24 || retentionPeriodHours > 168) {
throw new Error(`retentionPeriod must be between 24 and 168 hours. Received ${retentionPeriodHours}`);
}

const { streamEncryption, encryptionKey } = this.parseEncryption(props);
Expand Down
30 changes: 12 additions & 18 deletions packages/@aws-cdk/aws-kinesis/test/test.stream.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { expect } from '@aws-cdk/assert';
import * as iam from '@aws-cdk/aws-iam';
import * as kms from '@aws-cdk/aws-kms';
import { App, Stack } from '@aws-cdk/core';
import { App, Duration, Stack } from '@aws-cdk/core';
import { Test } from 'nodeunit';
import { Stream, StreamEncryption } from '../lib';

Expand Down Expand Up @@ -64,8 +64,8 @@ export = {
"uses explicit retention period"(test: Test) {
const stack = new Stack();

new Stream(stack, 'MyStream', {
retentionPeriodHours: 168
new Stream(stack, "MyStream", {
retentionPeriod: Duration.hours(168)
});

expect(stack).toMatch({
Expand All @@ -83,23 +83,17 @@ export = {
test.done();
},
"retention period must be between 24 and 168 hours"(test: Test) {
test.throws({
block: () => {
new Stream(new Stack(), 'MyStream', {
retentionPeriodHours: 169
});
},
message: "retentionPeriodHours must be between 24 and 168 hours"
});
test.throws(() => {
new Stream(new Stack(), 'MyStream', {
retentionPeriod: Duration.hours(169)
});
}, /retentionPeriod must be between 24 and 168 hours. Received 169/);

test.throws({
block: () => {
test.throws(() => {
new Stream(new Stack(), 'MyStream', {
retentionPeriodHours: 23
});
},
message: "retentionPeriodHours must be between 24 and 168 hours"
});
retentionPeriod: Duration.hours(23)
});
}, /retentionPeriod must be between 24 and 168 hours. Received 23/);

test.done();
},
Expand Down

0 comments on commit 1186227

Please sign in to comment.