feat(iot-actions): add support for DynamoDBv2 rule #20171

bobveringa · 2022-05-02T14:58:41Z

This merge request adds the dynamodbv2 IoT rule action as a new feature. This rule is useful for quickly logging device information into DynamoDB. Furthermore, it adds some other important functionality as IoT action.

Closes #20162

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-05-02T14:58:47Z

Add dynamoDB example to readme and update list.

Reorder items so that the example works as intended.

bobveringa · 2022-05-03T00:08:04Z

The build failed due to a bug I had already noticed and resolved (I think), but the last commit was not used for the build. I merged master into my branch to trigger the build process again. This should include the latest commit in which I resolved the issue.

Or if that 2nd commit also failed, I cannot find a build log for it. Which makes it hard to resolve the issue.

bobveringa · 2022-05-05T11:57:33Z

Is there any additional work I need to do for this PR?

yamatatsu

@bobveringa Thank you to work for this issue!
I propose some small changes 😄 .

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

packages/@aws-cdk/aws-iot-actions/test/dynamodb-v2/integ.dynamodb-v2-put-item-action.ts

Co-authored-by: Tatsuya Yamamoto <the.kinnikuman@gmail.com>

…odb-v2-put-item-action.ts Co-authored-by: Tatsuya Yamamoto <the.kinnikuman@gmail.com>

Integrate feedback from the review and re-run integration tests to validate no regressions have occurred.

bobveringa · 2022-05-06T08:45:40Z

Thanks for your review @yamatatsu, I have the changes and updated the integration tests template.

yamatatsu

Thank you for addressing my proposes! 😍 And sorry for my late reaction.
Once, I propose minor changes based on my experience of contributions to make smooth the review of the core maintainer. And I hope this PR will be merged 😌.

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

packages/@aws-cdk/aws-iot-actions/lib/dynamo-db-v2-put-item.ts

yamatatsu · 2022-05-26T13:31:41Z

packages/@aws-cdk/aws-iot-actions/lib/dynamo-db-v2-put-item.ts

@@ -0,0 +1,45 @@
+import * as dynamodb from '@aws-cdk/aws-dynamodb';


I think it is better to rename this file to dynamodb-v2-put-item.ts because existing files for DynamoDB in this repository are named dynamodb-xxx instead of dynamo-db-xxx.

I would actually go with dynamodbv2-put-item-action.ts

daschaa · 2022-07-10T16:24:54Z

@bobveringa Did you do any progress on this one ? :)

bobveringa · 2022-07-10T16:31:01Z

I have not yet have had the time to integrate this feedback. And due to the lack of interest from any maintainers or communication at all, I have not really been motivated to work on this solution. For our own project, we have figured out a way around it.

In general, I found the complete lack of communication very discouraging, and I have matters to work on that people are actually interested in.

daschaa · 2022-07-10T17:01:43Z

@bobveringa I'm really sorry to hear that you feel like nothing has happened here. It would be cool if you could still look at/resolve proposed adjustments. I guess the CDK just waited for your response on the suggestions.
Would be cool if you could take a look ❤️

bobveringa · 2022-07-10T18:57:25Z

I've added an item to look at this next sprint.

However, given this experience, it is unlikely that we will contribute changes in the feature. After 2 months, I feel like someone could have spent 30 seconds leaving a comment that changes are needed.

TheRealAmazonKendra

@bobveringa On behalf of the team, I apologize for your experience submitting the PR and the radio silence on our end. The quality of this change is precisely what I look for when I'm reviewing so I sincerely hope that you'll continue contributing in the future.

Besides the comment below about the name of the file, I will just +1 the suggested changes by @yamatatsu. Everything else looks great.

TheRealAmazonKendra · 2022-07-13T00:38:03Z

packages/@aws-cdk/aws-iot-actions/lib/dynamo-db-v2-put-item.ts

@@ -0,0 +1,45 @@
+import * as dynamodb from '@aws-cdk/aws-dynamodb';


I would actually go with dynamodbv2-put-item-action.ts

Co-authored-by: Tatsuya Yamamoto <the.kinnikuman@gmail.com>

Pull request has been modified.

Co-authored-by: Tatsuya Yamamoto <the.kinnikuman@gmail.com>

bobveringa · 2022-07-24T16:27:43Z

I am not certain why this build is failing. The logs don't really help narrow it down. I'll investigate this once I have more time to work on it. If anyone has a clue why this started failing, please let me know.

TheRealAmazonKendra · 2022-07-26T21:42:34Z

I am not certain why this build is failing. The logs don't really help narrow it down. I'll investigate this once I have more time to work on it. If anyone has a clue why this started failing, please let me know.

Looks like it was just that the name of the file in the exports didn't get updated. I added that fix.

TheRealAmazonKendra

Looks like the names of the files aren't consistent. Could you please update those and then I think this looks good to go.

bobveringa · 2022-07-27T20:17:24Z

Looks like the names of the files aren't consistent. Could you please update those and then I think this looks good to go.

Do you mean the filename of the code and the filenames for the tests?

TheRealAmazonKendra · 2022-07-27T21:12:57Z

Looks like the names of the files aren't consistent. Could you please update those and then I think this looks good to go.

Do you mean the filename of the code and the filenames for the tests?

The tests. I also just noticed that the integ test isn't using our integ test construct so it's not actually testing against the expected output. Apologies for not noticing that sooner.

bobveringa · 2022-08-02T11:55:09Z

Looks like the names of the files aren't consistent. Could you please update those and then I think this looks good to go.

Do you mean the filename of the code and the filenames for the tests?

The tests. I also just noticed that the integ test isn't using our integ test construct so it's not actually testing against the expected output. Apologies for not noticing that sooner.

I'll make this change. In addition someone should update the other tests, as none of them use the integ test construct.

Pull request has been modified.

bobveringa · 2022-08-02T13:09:55Z

I think that should be all the changes. Please let me know if there is any more work that needs to be done for this issue.

TheRealAmazonKendra · 2022-08-02T22:45:09Z

@mergify update

mergify · 2022-08-02T22:45:21Z

update

✅ Branch has been successfully updated

TheRealAmazonKendra · 2022-08-02T23:10:26Z

Looks like the update wasn't quite right but I'm going to go ahead and update them since it will only take me a second. You're also right about us needing to update the other tests. We're slowly but surely making our way through those updates.

TheRealAmazonKendra · 2022-08-02T23:13:46Z

@Mergifyio update

mergify · 2022-08-02T23:14:20Z

update

✅ Branch has been successfully updated

TheRealAmazonKendra

As long as this build succeeds, I think this is good to go!

aws-cdk-automation · 2022-08-02T23:50:01Z

AWS CodeBuild CI Report

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

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

mergify · 2022-08-02T23:50:17Z

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

bobveringa · 2022-08-03T07:50:46Z

Alright, thank you very much for resolving that issue.

This merge request adds the dynamodbv2 IoT rule action as a new feature. This rule is useful for quickly logging device information into DynamoDB. Furthermore, it adds some other important functionality as IoT action. Closes aws#20162 ---- ### 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*

Add support for DynamoDBv2 iot rule action

232fa39

github-actions bot added the p2 label May 2, 2022

aws-cdk-automation requested a review from a team May 2, 2022 14:58

aws-cdk-automation assigned comcalvi May 2, 2022

skinny85 changed the title ~~feat(iot-actions-alpha): add support for dynamodbv2 rule~~ feat(iot-actions): add support for dynamodbv2 rule May 2, 2022

github-actions bot added the feature-request A feature should be added or improved. label May 2, 2022

skinny85 changed the title ~~feat(iot-actions): add support for dynamodbv2 rule~~ feat(iot-actions): add support for DynamoDBv2 rule May 2, 2022

bobveringa and others added 3 commits May 2, 2022 22:57

Update readme

2b5e27f

Add dynamoDB example to readme and update list.

Update readme

3ba08b8

Reorder items so that the example works as intended.

Merge branch 'master' into master

cf04975

yamatatsu reviewed May 6, 2022

View reviewed changes

bobveringa and others added 4 commits May 6, 2022 10:04

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

290b612

Co-authored-by: Tatsuya Yamamoto <the.kinnikuman@gmail.com>

Update packages/@aws-cdk/aws-iot-actions/test/dynamodb-v2/integ.dynam…

a50e3bb

…odb-v2-put-item-action.ts Co-authored-by: Tatsuya Yamamoto <the.kinnikuman@gmail.com>

Update packages/@aws-cdk/aws-iot-actions/test/dynamodb-v2/integ.dynam…

d3be03e

…odb-v2-put-item-action.ts Co-authored-by: Tatsuya Yamamoto <the.kinnikuman@gmail.com>

Integrate code review feedback

d6c4912

Integrate feedback from the review and re-run integration tests to validate no regressions have occurred.

yamatatsu approved these changes May 26, 2022

View reviewed changes

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

github-actions bot added the effort/small Small work item – less than a day of effort label Jul 13, 2022

Merge branch 'main' into master

0c06b10

TheRealAmazonKendra previously requested changes Jul 13, 2022

View reviewed changes

Apply suggestions from code review

90f4284

Co-authored-by: Tatsuya Yamamoto <the.kinnikuman@gmail.com>

bobveringa and others added 4 commits July 24, 2022 14:40

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

cac3414

Co-authored-by: Tatsuya Yamamoto <the.kinnikuman@gmail.com>

Renamed file to follow AWS convention

c6ae409

Merge branch 'master' of https://github.com/bobveringa/aws-cdk

c1bddeb

Merge branch 'aws:main' into master

dbe3dd4

TheRealAmazonKendra added 2 commits July 26, 2022 14:39

fix export

c9db00d

fix export name

6d2d87a

TheRealAmazonKendra previously requested changes Jul 27, 2022

View reviewed changes

Merge branch 'aws:main' into master

c14b234

Changed filenames to match formatting and added IntegTest construct

c802989

Merge branch 'main' into master

db1cb20

update tests

8b0f3ea

Merge branch 'main' into master

28cd711

TheRealAmazonKendra approved these changes Aug 2, 2022

View reviewed changes

mergify bot merged commit a57dec3 into aws:main Aug 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(iot-actions): add support for DynamoDBv2 rule #20171

feat(iot-actions): add support for DynamoDBv2 rule #20171

bobveringa commented May 2, 2022 •

edited by skinny85

Loading

gitpod-io bot commented May 2, 2022

bobveringa commented May 3, 2022 •

edited

Loading

bobveringa commented May 5, 2022

yamatatsu left a comment

bobveringa commented May 6, 2022

yamatatsu left a comment

yamatatsu May 26, 2022

TheRealAmazonKendra Jul 13, 2022

daschaa commented Jul 10, 2022

bobveringa commented Jul 10, 2022

daschaa commented Jul 10, 2022

bobveringa commented Jul 10, 2022

TheRealAmazonKendra left a comment

TheRealAmazonKendra Jul 13, 2022

bobveringa commented Jul 24, 2022

TheRealAmazonKendra commented Jul 26, 2022

TheRealAmazonKendra left a comment

bobveringa commented Jul 27, 2022

TheRealAmazonKendra commented Jul 27, 2022

bobveringa commented Aug 2, 2022

bobveringa commented Aug 2, 2022

TheRealAmazonKendra commented Aug 2, 2022

mergify bot commented Aug 2, 2022

TheRealAmazonKendra commented Aug 2, 2022

TheRealAmazonKendra commented Aug 2, 2022

mergify bot commented Aug 2, 2022

TheRealAmazonKendra left a comment

aws-cdk-automation commented Aug 2, 2022

mergify bot commented Aug 2, 2022

bobveringa commented Aug 3, 2022

		@@ -0,0 +1,45 @@
		import * as dynamodb from '@aws-cdk/aws-dynamodb';

feat(iot-actions): add support for DynamoDBv2 rule #20171

feat(iot-actions): add support for DynamoDBv2 rule #20171

Conversation

bobveringa commented May 2, 2022 • edited by skinny85 Loading

All Submissions:

Adding new Unconventional Dependencies:

New Features

gitpod-io bot commented May 2, 2022

bobveringa commented May 3, 2022 • edited Loading

bobveringa commented May 5, 2022

yamatatsu left a comment

Choose a reason for hiding this comment

bobveringa commented May 6, 2022

yamatatsu left a comment

Choose a reason for hiding this comment

yamatatsu May 26, 2022

Choose a reason for hiding this comment

TheRealAmazonKendra Jul 13, 2022

Choose a reason for hiding this comment

daschaa commented Jul 10, 2022

bobveringa commented Jul 10, 2022

daschaa commented Jul 10, 2022

bobveringa commented Jul 10, 2022

TheRealAmazonKendra left a comment

Choose a reason for hiding this comment

TheRealAmazonKendra Jul 13, 2022

Choose a reason for hiding this comment

bobveringa commented Jul 24, 2022

TheRealAmazonKendra commented Jul 26, 2022

TheRealAmazonKendra left a comment

Choose a reason for hiding this comment

bobveringa commented Jul 27, 2022

TheRealAmazonKendra commented Jul 27, 2022

bobveringa commented Aug 2, 2022

bobveringa commented Aug 2, 2022

TheRealAmazonKendra commented Aug 2, 2022

mergify bot commented Aug 2, 2022

✅ Branch has been successfully updated

TheRealAmazonKendra commented Aug 2, 2022

TheRealAmazonKendra commented Aug 2, 2022

mergify bot commented Aug 2, 2022

✅ Branch has been successfully updated

TheRealAmazonKendra left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented Aug 2, 2022

AWS CodeBuild CI Report

mergify bot commented Aug 2, 2022

bobveringa commented Aug 3, 2022

bobveringa commented May 2, 2022 •

edited by skinny85

Loading

bobveringa commented May 3, 2022 •

edited

Loading