feat(iot-actions): support for sending messages to iot-events by yamatatsu · Pull Request #19953 · aws/aws-cdk

yamatatsu · 2022-04-18T09:32:12Z

This PR includes to support the action for sending messages to IoT Events.

This feature is described this documentation.

I actually confirmed that the behavior of the action deployed by integ-test is all right.

All Submissions:

Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

This PR adds new unconventional dependencies following the process described here

New Features

Have you added the new feature to an integration test?
- Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

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

gitpod-io · 2022-04-18T09:32:20Z

TheRealAmazonKendra

Same comment as #19949, unfortunately changing to the new branch made this messy.

Pull request has been modified.

yamatatsu · 2022-07-31T13:36:21Z

I’ve rebased it!

TheRealAmazonKendra

Overall this is great work! Just a few comments inline.

TheRealAmazonKendra · 2022-08-01T20:41:45Z

packages/@aws-cdk/aws-iot-actions/lib/iotevents-put-message-action.ts

+   *
+   * When batchMode is true and the rule SQL statement evaluates to an Array,
+   * each Array element is treated as a separate message when Events by calling BatchPutMessage.
+   * The resulting array can't have more than 10 messages.


If the array does have more than 10 messages how/when does this fail? What's the user experience here? We should probably have some enforcement here, if possible.

Not explained in the documentation, I've experimented it with the below command:

aws iot-data publish --topic device/mydevice/data --qos 1 --payload (echo '[{"payload":{"deviceId":"001"}},{"payload":{"deviceId":"002"}},{"payload":{"deviceId":"003"}},{"payload":{"deviceId":"004"}},{"payload":{"deviceId":"005"}},{"payload":{"deviceId":"006"}},{"payload":{"deviceId":"007"}},{"payload":{"deviceId":"008"}},{"payload":{"deviceId":"009"}},{"payload":{"deviceId":"010"}},{"payload":{"deviceId":"011"}}]' | base64) --region us-east-1

and get below error message:

Failed to send message to Iot Events. The payload containing 11 messages cannot have more than 10 messages when batchMode=true..

I thing it is not possible to restrict it, because it depends to published payloads (instead of SQL) that is out of CDK.
WDYT?

TheRealAmazonKendra · 2022-08-01T20:44:06Z

packages/@aws-cdk/aws-iot-actions/lib/iotevents-put-message-action.ts

+export class IotEventsPutMessageAction implements iot.IAction {
+  private readonly batchMode?: boolean;
+  private readonly messageId?: string;
+  private readonly role?: iam.IRole;


What's the behavior if a role is not set?

If a role is not set, a new role will be created and be granted some policies needed.

It's explained in CommonActionProps.

aws-cdk/packages/@aws-cdk/aws-iot-actions/lib/common-action-props.ts

Lines 7 to 12 in aa887ee

/**

* The IAM role that allows access to AWS service.

*

* @default a new role will be created

*/

readonly role?: iam.IRole;

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

packages/@aws-cdk/aws-iot-actions/lib/iotevents-put-message-action.ts

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

Pull request has been modified.

TheRealAmazonKendra

I'm going to put this back into changes requested since the build is failing, that way I'll get notified when it's been updated.

Pull request has been modified.

TheRealAmazonKendra

Just one more comment. I don't really want to add exclusions to our lint rules.

TheRealAmazonKendra · 2022-08-04T15:03:39Z

packages/@aws-cdk/aws-iot/package.json

  },
+  "awslint": {
+    "exclude": [
+      "no-unused-type:@aws-cdk/aws-iot.ActionConfig"


What's causing this violation? I generally don't think we should be adding lint exclusions but instead should make sure the code aligns to them.

@TheRealAmazonKendra
I've researched it.
It was caused by addind @internal to method IAction._bind(), the message is:

error: [awslint:no-unused-type:@aws-cdk/aws-iot.ActionConfig] type or enum is not used by exported classes (declared at lib/action.ts:20)

It appears that IAction._bind() is no longer listed in Type.ownMethods.
This caused ActionConfig to be markded as unused-type.

Changing ActionConfig to not export will suppress this violation. However, if we do so, ActionConfig will not be able to be used in aws-iot-actions.

Like this, when an interfaces is extended across packages, adding lint exclusions seems reasonable.
WDYT?

mergify · 2022-08-08T21:17:36Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2022-08-08T21:54:02Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 96c2936
Result: SUCCEEDED
Build Logs (available for 30 days)

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

mergify · 2022-08-08T21:54:22Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

) This PR includes to support the action for sending messages to IoT Events. This feature is described [this documentation](https://docs.aws.amazon.com/iot/latest/developerguide/iotevents-rule-action.html). I actually confirmed that the behavior of the action deployed by integ-test is all right. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

aws-cdk-automation requested a review from a team April 18, 2022 09:32

github-actions bot added the p2 label Apr 18, 2022

TheRealAmazonKendra changed the base branch from v1-main to main July 13, 2022 00:06

TheRealAmazonKendra previously requested changes Jul 14, 2022

View reviewed changes

feat(iot-actions): Support to send message to IoT Events

f4f2061

yamatatsu force-pushed the iot-actions-iotevents branch from c99cb36 to f4f2061 Compare July 30, 2022 05:19

yamatatsu added 2 commits July 30, 2022 17:04

add assertion to integ-test

2895b0b

remove assertion from integ-test

aa887ee

yamatatsu requested a review from TheRealAmazonKendra July 31, 2022 12:07

TheRealAmazonKendra changed the title ~~feat(iot-actions): Support to send message to IoT Events~~ feat(iot-actions): support to send message to iot-events Aug 1, 2022

TheRealAmazonKendra changed the title ~~feat(iot-actions): support to send message to iot-events~~ feat(iot-actions): support for sending messages to iot-events Aug 1, 2022

TheRealAmazonKendra previously requested changes Aug 1, 2022

View reviewed changes

TheRealAmazonKendra reviewed Aug 1, 2022

View reviewed changes

packages/@aws-cdk/aws-iot-actions/lib/iotevents-put-message-action.ts Outdated Show resolved Hide resolved

TheRealAmazonKendra reviewed Aug 1, 2022

View reviewed changes

packages/@aws-cdk/aws-iot-actions/README.md Outdated Show resolved Hide resolved

yamatatsu added 3 commits August 3, 2022 22:30

address comments

c328fc8

address comment

2971295

Merge branch 'main' into iot-actions-iotevents

bb815e6

address comment

23363b8

TheRealAmazonKendra previously requested changes Aug 3, 2022

View reviewed changes

fix README for rosetta

598dc01

TheRealAmazonKendra requested changes Aug 4, 2022

View reviewed changes

yamatatsu requested a review from TheRealAmazonKendra August 5, 2022 13:11

TheRealAmazonKendra approved these changes Aug 8, 2022

View reviewed changes

Merge branch 'main' into iot-actions-iotevents

96c2936

mergify bot merged commit 35fc169 into aws:main Aug 8, 2022

	/**
	* The IAM role that allows access to AWS service.
	*
	* @default a new role will be created
	*/
	readonly role?: iam.IRole;

Conversation

yamatatsu commented Apr 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

All Submissions:

Adding new Unconventional Dependencies:

New Features

Uh oh!

gitpod-io bot commented Apr 18, 2022

Uh oh!

TheRealAmazonKendra left a comment

Choose a reason for hiding this comment

Uh oh!

yamatatsu commented Jul 31, 2022

Uh oh!

TheRealAmazonKendra left a comment

Choose a reason for hiding this comment

Uh oh!

TheRealAmazonKendra Aug 1, 2022

Choose a reason for hiding this comment

Uh oh!

yamatatsu Aug 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheRealAmazonKendra Aug 1, 2022

Choose a reason for hiding this comment

Uh oh!

yamatatsu Aug 3, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TheRealAmazonKendra left a comment

Choose a reason for hiding this comment

Uh oh!

TheRealAmazonKendra left a comment

Choose a reason for hiding this comment

Uh oh!

TheRealAmazonKendra Aug 4, 2022

Choose a reason for hiding this comment

Uh oh!

yamatatsu Aug 5, 2022

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Aug 8, 2022

Uh oh!

aws-cdk-automation commented Aug 8, 2022

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Aug 8, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yamatatsu commented Apr 18, 2022 •

edited

Loading

yamatatsu Aug 3, 2022 •

edited

Loading