feat(iot): add Action to put record to Kinesis Data stream #18321

yamatatsu · 2022-01-08T15:40:33Z

Fixes #17703

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

gitpod-io · 2022-01-08T15:40:35Z

packages/@aws-cdk/aws-iot-actions/lib/kinesis-put-record-action.ts

skinny85

Very high quality contribution as always, @yamatatsu! A few minor comments.

skinny85 · 2022-01-10T21:21:56Z

packages/@aws-cdk/aws-iot-actions/README.md

+  sql: iot.IotSql.fromStringAsVer20160323("SELECT * FROM 'device/+/data'"),
+  actions: [
+    new actions.KinesisPutRecordAction(stream, {
+      partitionKey: '${timestamp()}', // optional property


Let's mention what's the default.

skinny85 · 2022-01-10T22:00:52Z

packages/@aws-cdk/aws-iot-actions/lib/kinesis-put-record-action.ts

+   * The partition key is usually composed of an expression (for example, ${topic()} or ${timestamp()}).
+   * For more information @see https://docs.aws.amazon.com/kinesis/latest/APIReference/API_PutRecord.html#API_PutRecord_RequestParameters
+   *
+   * @default - None


Can you elaborate what does "None" mean here?

Sorry, I tried to elaborate, but I could not😞.

Proberbly, IoT Core rule is fill default value (e.g. MQTT payload JSON string or hash of it) because PartitionKey is required in this API Reference.
But there is no description about it in IoT Core rule documentation and CloudFormation Documentation

Did you try deploying an IoT app without this property filled?

Maybe this is simply a mistake in the CloudFormation resource, and this should actually be required?

Did you try deploying an IoT app without this property filled?

Yes I tried and the deploying secceeded.

Again, I deployed, and I tested following for confirmation to be spreaded records to shards:

Prepare

Deploy Kinesis Stream with 10 shards

Test1

Prepare: Deploy IoT Core Kinesis Rule action with ${timestamp()} partitionKey

Send 10 same payloads

Test2

Prepare: Deploy IoT Core Kinesis Rule action with no partitionKey

Send 10 same payloads

Test3

Prepare: Deploy IoT Core Kinesis Rule action with no partitionKey

Send 10 payloads different each time

Results:

Test1: Records was spreaded to each shards

Test2: All records was put in same record

Test3: All records was put in same record

The IoT Core may be filling in a static partitionKey if it is not specified.
But this is just a guess...

OK. I think that's a good guess. Let's put that in the docs - it's certainly better than just "None", which gives you no clue what that actually means 🙂.

Thanks for the detailed testing!

Thank you!

OK. If we beleave this guess, I think it is better that fill default value. Because in Kinesis Stream, it is pretty not helpful to not spread records by shards.

I found that ${newuuid} is suggested in Web Console.

This is the documentation of ${newuuid}.

I try to add default value ${newuuid}. But if you have any opinions, please feel free to tell me😉 .

I don't hate it, but I'm a little concerned that the L2 won't have all of the capabilities of the underlying service (in this case, leaving the partition key as empty).

Does the Console allow you to leave it as empty, or do you have to provide a value?

the L2 won't have all of the capabilities of the underlying service

It is so reasonable I think! So I will never mind to revert my commit that add default value.

Does the Console allow you to leave it as empty, or do you have to provide a value?

The Console requre PartitionKey, but CloudFormation does not. For keeping compatibility for CloudFormation, let's not add a default value, instead add a detailed description in JSDoc and Readme?

Let's do this.

Make this field required, like it is in the console. But, allow specifying an empty string there (''). If an empty string is passed, then we will actually pass undefined to the underlying CloudFormation resource for that field. Of course, we will need to cover that in our documentation.

This way, we will be close to the Console experience, while covering the entire service capabilities.

What do you think about this @yamatatsu?

It's good I think! 👍🏻
OK, I will do it!

packages/@aws-cdk/aws-iot-actions/lib/kinesis-put-record-action.ts

skinny85 · 2022-01-10T22:27:17Z

packages/@aws-cdk/aws-iot-actions/test/kinesis-stream/integ.kinesis-put-record-action.ts

+      errorAction: new actions.CloudWatchLogsAction(
+        new logs.LogGroup(this, 'Logs', { removalPolicy: cdk.RemovalPolicy.DESTROY }),
+      ),


Do we need this in this test?

Oh it is not needed, but for confirmation that it is work on aws 😅 .
I'll remove it!

packages/@aws-cdk/aws-iot-actions/test/kinesis-stream/integ.kinesis-put-record-action.ts

packages/@aws-cdk/aws-iot-actions/test/kinesis-stream/kinesis-put-record-action.test.ts

…nesis-put-record-action.ts Co-authored-by: Adam Ruka <adamruka85@gmail.com>

Pull request has been modified.

…u/aws-cdk into aws-iot-actions-kinesis

…on` (#18356) By #18321 (comment) BREAKING CHANGE: the class `FirehoseStreamAction` has been renamed to `FirehosePutRecordAction` ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

skinny85

Looks great @yamatatsu, thanks for your patience in seeing this one through to the end!

mergify · 2022-01-18T23:06:06Z

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-19T14:44:31Z

AWS CodeBuild CI Report

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

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

mergify · 2022-01-19T14:44:57Z

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

…on` (aws#18356) By aws#18321 (comment) BREAKING CHANGE: the class `FirehoseStreamAction` has been renamed to `FirehosePutRecordAction` ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

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

feat(iot): add Action to put record to Kinesis Data stream

c1799a3

github-actions bot assigned skinny85 Jan 8, 2022

github-actions bot added the @aws-cdk/aws-iot Related to AWS IoT label Jan 8, 2022

yamatatsu added 3 commits January 9, 2022 01:02

remove unused package

9249bd6

rename integ test file

c1c4bd4

remove verbose code in integ test

d64b3ff

yamatatsu commented Jan 8, 2022

View reviewed changes

packages/@aws-cdk/aws-iot-actions/lib/kinesis-put-record-action.ts Show resolved Hide resolved

skinny85 previously requested changes Jan 10, 2022

View reviewed changes

Update packages/@aws-cdk/aws-iot-actions/test/kinesis-stream/integ.ki…

4b362f1

…nesis-put-record-action.ts Co-authored-by: Adam Ruka <adamruka85@gmail.com>

yamatatsu added 2 commits January 11, 2022 12:41

address comments

e32e2d7

Merge branch 'aws-iot-actions-kinesis' of https://github.com/yamatats…

dd3edbb

…u/aws-cdk into aws-iot-actions-kinesis

yamatatsu mentioned this pull request Jan 11, 2022

fix(iot): FirehoseStreamAction is now called FirehosePutRecordAction #18356

Merged

yamatatsu added 6 commits January 12, 2022 10:28

Merge branch 'master' into aws-iot-actions-kinesis

1457dcc

Merge branch 'master' into aws-iot-actions-kinesis

7851981

fix integ test snapshot

1e6c4a8

add default value for partitionKey

b2cd603

fix readme

b74dc6c

address comments

9fe5bb1

yamatatsu requested a review from skinny85 January 17, 2022 12:30

skinny85 approved these changes Jan 18, 2022

View reviewed changes

Merge branch 'master' into aws-iot-actions-kinesis

103df62

mergify bot merged commit 1480213 into aws:master Jan 19, 2022

renovate bot mentioned this pull request Mar 17, 2022

Update aws cdk to v1.149.0 cohalz/cirrocumulus#130

Open

1 task

yamatatsu deleted the aws-iot-actions-kinesis branch April 6, 2022 15:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(iot): add Action to put record to Kinesis Data stream #18321

feat(iot): add Action to put record to Kinesis Data stream #18321

yamatatsu commented Jan 8, 2022

gitpod-io bot commented Jan 8, 2022

skinny85 left a comment

skinny85 Jan 10, 2022

skinny85 Jan 10, 2022

yamatatsu Jan 11, 2022 •

edited

Loading

skinny85 Jan 11, 2022

yamatatsu Jan 12, 2022 •

edited

Loading

skinny85 Jan 12, 2022

yamatatsu Jan 13, 2022

skinny85 Jan 13, 2022 •

edited

Loading

yamatatsu Jan 14, 2022

skinny85 Jan 14, 2022 •

edited

Loading

yamatatsu Jan 17, 2022

skinny85 Jan 10, 2022

yamatatsu Jan 11, 2022

skinny85 left a comment

mergify bot commented Jan 18, 2022

aws-cdk-automation commented Jan 19, 2022

mergify bot commented Jan 19, 2022

feat(iot): add Action to put record to Kinesis Data stream #18321

feat(iot): add Action to put record to Kinesis Data stream #18321

Conversation

yamatatsu commented Jan 8, 2022

gitpod-io bot commented Jan 8, 2022

skinny85 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yamatatsu Jan 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yamatatsu Jan 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skinny85 Jan 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skinny85 Jan 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

mergify bot commented Jan 18, 2022

aws-cdk-automation commented Jan 19, 2022

AWS CodeBuild CI Report

mergify bot commented Jan 19, 2022

yamatatsu Jan 11, 2022 •

edited

Loading

yamatatsu Jan 12, 2022 •

edited

Loading

skinny85 Jan 13, 2022 •

edited

Loading

skinny85 Jan 14, 2022 •

edited

Loading