fix(aws-kinesis): remove default shard count when stream mode is on-demand and set default mode to provisioned #18221

igilham · 2021-12-30T09:30:13Z

Change the default Kinesis Data Stream's stream mode to provisioned from undefined to make the active configuration more explicit in the resulting CloudFormation templates.

Fix an issue whereby the shard count is always set when the stream mode is set to on-demand, which is invalid. Shard count still defaults to 1 in provisioned mode, but is left undefined in on-demand mode.

Add validation for the above so that an error is thrown from CDK when specifying on-demand mode with a shard count.

Fixes #18139

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

Co-authored-by: AWS CDK Team <aws-cdk@amazon.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Align the import prefix for this additional class. Otherwise copying the code will not compile. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Explicitly declaring ShardCount with a default value can cause errors when using the on-demand stream mode. Instead, we can rely on CloudFormation's implicit defaults. part of #18139

gitpod-io · 2021-12-30T09:30:16Z

mergify · 2021-12-30T09:30:46Z

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

igilham · 2021-12-30T12:21:15Z

The changes so far should fix the basic problem and allow the stream mode to be used.

I haven't figured out how to do validation yet so any help would be appreciated.

igilham · 2021-12-30T15:15:45Z

It looks like the dynamodb module just adds various guard statements and to the L2 construct constructor to prevent misuse of conflicting configuration options. I'll do the same.

* Document that streamMode and shardCount conflict * Add a guard to prevent specifying shard count when stream mode is on-demand * Restore default shard count of 1 when stream mode is provisioned * Establish default stream mode of provisioned These changes make the stream configuration explicit in the generated CloudFormation templates again, matching the design of DynamoDB. Fixes #18139

message for error when setting streamMode to on-demand and also specifying shardCount

…mand

igilham · 2021-12-30T17:27:50Z

Okay, I think I'm ready for the first round of reviews.

d4r3topk · 2021-12-30T18:43:27Z

packages/@aws-cdk/aws-kinesis/test/stream.test.ts

+        shardCount: 2,
+        streamMode: StreamMode.ON_DEMAND,
+      });
+    }).toThrow('streamMode must be set to PROVISIONED (default) when specifying shardCount');


You can use streamMode must be set to ${StreamMode.PROVISIONED} (default) when specifying shardCount to keep the value of the enum even if it is changed in the future.

d4r3topk · 2021-12-30T18:46:48Z

packages/@aws-cdk/aws-kinesis/test/integ.stream-dashboard.expected.json

@@ -4,6 +4,9 @@
      "Type": "AWS::Kinesis::Stream",
      "Properties": {
        "ShardCount": 1,
+        "StreamModeDetails": {
+          "StreamMode": "PROVISIONED"


Check for ${StreamMode.PROVISIONED} for the exact enum values and same for ON_DEMAND occurences

I've made this change in the unit tests. It's not applicable to these JSON files.

packages/@aws-cdk/aws-kinesis/lib/stream.ts

d4r3topk · 2021-12-30T18:48:08Z

packages/@aws-cdk/aws-kinesis/lib/stream.ts

@@ -752,9 +755,15 @@ export class Stream extends StreamBase {
      physicalName: props.streamName,
    });

-    const shardCount = props.shardCount || 1;
+    let shardCount = props.shardCount;
+    let streamMode = props.streamMode ?? StreamMode.PROVISIONED;


Can be constant

In this commit, you are making two changes, ie. the fix and the default to PROVISIONED capacity. You can include that in the commit details.

I've updated the description. Hopefully that's a bit clearer.

igilham · 2022-01-04T09:48:11Z

I'm back from today if there's any more feedback on this.

peterwoodworth

This looks great to me, I wish I saw this before I submitted my own PR! Thank you for the contribution 🙂

Thank you also to @d4r3topk for the initial review!

mergify · 2022-01-06T00:38:09Z

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-cdk-automation · 2022-01-06T01:23:37Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: ccf2d61
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2022-01-06T01:23:59Z

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).

Since v1.139.0 the `StreamMode` of a Kinesis stream is `PROVISIONED` by default. The stream mode can be one of two options: - On-demand - data streams with an on-demand mode require no capacity planning and automatically scale to handle gigabytes of write and read throughput per minute. With the on-demand mode, Kinesis Data Streams automatically manages the shards in order to provide the necessary throughput. - Provisioned - for the data streams with a provisioned mode, you must specify the number of shards for the data stream. The total capacity of a data stream is the sum of the capacities of its shards. You can increase or decrease the number of shards in a data stream as needed. Provisioned matches our use of Kinesis today, so the default makes sense. See: - aws/aws-cdk#18221 - https://docs.aws.amazon.com/streams/latest/dev/how-do-i-size-a-stream.html

…emand and set default mode to provisioned (aws#18221) Change the default Kinesis Data Stream's stream mode to provisioned from undefined to make the active configuration more explicit in the resulting CloudFormation templates. Fix an issue whereby the shard count is always set when the stream mode is set to on-demand, which is invalid. Shard count still defaults to `1` in provisioned mode, but is left undefined in on-demand mode. Add validation for the above so that an error is thrown from CDK when specifying on-demand mode with a shard count. Fixes aws#18139 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

aws-cdk-automation and others added 3 commits December 30, 2021 09:13

feat(cfnspec): cloudformation spec v47.0.0 (#17353)

716cd19

Co-authored-by: AWS CDK Team <aws-cdk@amazon.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

docs(ecs): improve code sample import (#16831)

940c4b5

Align the import prefix for this additional class. Otherwise copying the code will not compile. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

fix(aws-kinesis): remove explicit default shard count

46de780

Explicitly declaring ShardCount with a default value can cause errors when using the on-demand stream mode. Instead, we can rely on CloudFormation's implicit defaults. part of #18139

github-actions bot assigned otaviomacedo Dec 30, 2021

github-actions bot added the @aws-cdk/aws-kinesis Related to Amazon Kinesis label Dec 30, 2021

igilham changed the title ~~Fix-kinesis-on-demand~~ fix(aws-kinesis): remove default shard count Dec 30, 2021

igilham added 6 commits December 30, 2021 09:50

test(aws-kinesis): remove shard count from integ

0f5c93c

test(aws-kinesisfirehose): remove extraneous shard count

b0c4611

test(aws-dynamodb): remove unused shard count

81f919c

test(aws-lambda-event-sources): remove unused shard count

97ef85e

test(aws-lambda-event-sources): remove unused shard count

34e7be1

test(aws-events-targets): remove unused shard count

e57e4e0

igilham changed the title ~~fix(aws-kinesis): remove default shard count~~ fix(aws-kinesis): remove default shard count when stream mode is provisioned Dec 30, 2021

igilham added 6 commits December 30, 2021 15:55

chore(aws-kinesis): simplify error message

51fd986

message for error when setting streamMode to on-demand and also specifying shardCount

test(aws-kinesis): add missing props to tests

90a6a4d

test(aws-kinesis): fix test descriptions

e036153

fix(aws-kinesis): issue setting shard count when stream mode is on-de…

547d306

…mand

test(aws-kinesis): try using equality assertion for error message

7c0489f

test: fix typo in expected templates

2537e89

igilham marked this pull request as ready for review December 30, 2021 17:27

d4r3topk reviewed Dec 30, 2021

View reviewed changes

igilham added 3 commits December 31, 2021 09:29

chore(aws-kinesis): change let to const

d0e0f1c

chore(aws-kinesis): update error text

640eb4c

test(aws-kinesis): use enum over literal for expectations

fba4cba

igilham changed the title ~~fix(aws-kinesis): remove default shard count when stream mode is provisioned~~ fix(aws-kinesis): remove default shard count when stream mode is on-demand and set default mode to provisioned Dec 31, 2021

peterwoodworth approved these changes Jan 6, 2022

View reviewed changes

Merge branch 'master' into fix-kinesis-on-demand

ccf2d61

mergify bot merged commit cac11bb into aws:master Jan 6, 2022

igilham deleted the fix-kinesis-on-demand branch January 6, 2022 09:23

akash1810 mentioned this pull request Jan 26, 2022

chore(deps)!: Update aws-cdk libraries to 1.140.0 guardian/cdk#1060

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(aws-kinesis): remove default shard count when stream mode is on-demand and set default mode to provisioned #18221

fix(aws-kinesis): remove default shard count when stream mode is on-demand and set default mode to provisioned #18221

igilham commented Dec 30, 2021 •

edited

Loading

gitpod-io bot commented Dec 30, 2021

mergify bot commented Dec 30, 2021

igilham commented Dec 30, 2021

igilham commented Dec 30, 2021

igilham commented Dec 30, 2021

d4r3topk Dec 30, 2021

d4r3topk Dec 30, 2021

igilham Dec 31, 2021

d4r3topk Dec 30, 2021

d4r3topk Dec 30, 2021

igilham Dec 31, 2021

igilham commented Jan 4, 2022

peterwoodworth left a comment

mergify bot commented Jan 6, 2022

aws-cdk-automation commented Jan 6, 2022

mergify bot commented Jan 6, 2022

fix(aws-kinesis): remove default shard count when stream mode is on-demand and set default mode to provisioned #18221

fix(aws-kinesis): remove default shard count when stream mode is on-demand and set default mode to provisioned #18221

Conversation

igilham commented Dec 30, 2021 • edited Loading

gitpod-io bot commented Dec 30, 2021

mergify bot commented Dec 30, 2021

igilham commented Dec 30, 2021

igilham commented Dec 30, 2021

igilham commented Dec 30, 2021

d4r3topk Dec 30, 2021

Choose a reason for hiding this comment

d4r3topk Dec 30, 2021

Choose a reason for hiding this comment

igilham Dec 31, 2021

Choose a reason for hiding this comment

d4r3topk Dec 30, 2021

Choose a reason for hiding this comment

d4r3topk Dec 30, 2021

Choose a reason for hiding this comment

igilham Dec 31, 2021

Choose a reason for hiding this comment

igilham commented Jan 4, 2022

peterwoodworth left a comment

Choose a reason for hiding this comment

mergify bot commented Jan 6, 2022

aws-cdk-automation commented Jan 6, 2022

AWS CodeBuild CI Report

mergify bot commented Jan 6, 2022

igilham commented Dec 30, 2021 •

edited

Loading