Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(lambda-event-sources): failure handling for stream event sources #5929

Merged
merged 47 commits into from
Mar 16, 2020
Merged

feat(lambda-event-sources): failure handling for stream event sources #5929

merged 47 commits into from
Mar 16, 2020

Conversation

xerofun
Copy link
Contributor

@xerofun xerofun commented Jan 23, 2020 •
edited by nija-at
Loading

Closes #5236

Commit Message

feat(lambda-event-sources): failure handling for stream event sources

Co-authored-by: Niranjan Jayakar <[email protected]>

closes #5236

End Commit Message

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

@xerofun xerofun requested a review from nija-at January 23, 2020 13:26
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@xerofun xerofun requested a review from SomayaB January 23, 2020 20:31
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@SomayaB SomayaB removed their request for review January 23, 2020 21:32
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

nija-at
nija-at previously requested changes Jan 27, 2020
View reviewed changes
Copy link
Contributor

@nija-at nija-at left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xerofun -

First off, thanks for submitting this PR!

My my initial comments are below and @duarten has some as well. Take your time to address them. This PR is likely going to need 2-4 iterations until the code is ready to be merged.

You will need to also update the README.md file in the appropriate packages and describe how these new features should be used, along with code snippets or examples.

packages/@aws-cdk/aws-lambda-event-sources/lib/stream.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-lambda-event-sources/lib/stream.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-lambda-event-sources/package.json Outdated Show resolved Hide resolved
@mergify mergify bot dismissed nija-at’s stale review January 27, 2020 13:25

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

nija-at
nija-at previously requested changes Jan 29, 2020
View reviewed changes
Copy link
Contributor

@nija-at nija-at left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Please add documentation of this feature to the README with a code snippet.

  2. Beyond unit tests, how did you test that your changes actually work when deployed to CloudFormation?

    I suggest that you create an integration test that sets up a stream (say, kinesis) configured with a lambda function to read from the stream as an event source and an queue as a failure destination and deploy it.

    [optional] set it up in a way that the function fails and the queue has the failed messages, which can be verified. The test should have a few lines of comments on how to actually test that this worked.

    You should be able to run cdk deploy on this integration test, followed by a set of AWS CLI commends to confirm everything worked.

    Here are a few examples - integ.destinations.ts, integ.token-authorizer.ts

UPDATE: Marked one item as optional in my above comment.

packages/@aws-cdk/aws-lambda-event-sources/lib/stream.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-lambda-event-sources/lib/stream.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-lambda-event-sources/lib/stream.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-lambda-event-sources/lib/stream.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-lambda-event-sources/lib/stream.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-lambda-event-sources/lib/stream.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-lambda/lib/event-source-mapping.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-lambda/lib/event-source-mapping.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-lambda/lib/event-source-mapping.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-lambda-event-sources/lib/stream.ts Outdated Show resolved Hide resolved
@mergify mergify bot dismissed nija-at’s stale review January 30, 2020 13:35

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: fb6957b
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 43cd63b
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 303aabb
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

nija-at
nija-at previously requested changes Mar 10, 2020
View reviewed changes
Copy link
Contributor

@nija-at nija-at left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment switching IResource to EventSourceMapping

@mergify mergify bot dismissed nija-at’s stale review March 10, 2020 08:45

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: c3f4dfe
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 2d2e1d7
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify
Copy link
Contributor

mergify bot commented Mar 11, 2020

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
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 9d0d2f0
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@xerofun
Copy link
Contributor Author

xerofun commented Mar 12, 2020

@nija-at looks like there was a hiccup in the merge build - is there anything i can do to give it a nudge?

nija-at
nija-at previously approved these changes Mar 13, 2020
View reviewed changes
@mergify mergify bot dismissed nija-at’s stale review March 13, 2020 14:03

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: e2dc206
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: b6e842f
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@nija-at
Copy link
Contributor

nija-at commented Mar 16, 2020

Build failure because of the ongoing CDK release. Closing and re-opening this PR to re-trigger the build.

@nija-at nija-at closed this Mar 16, 2020
@nija-at nija-at reopened this Mar 16, 2020
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: f149ceb
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@nija-at nija-at merged commit 5028009 into aws:master Mar 16, 2020
@snyk-bot snyk-bot mentioned this pull request Apr 10, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexD1979 AlexD1979 AlexD1979 left review comments

@duarten duarten duarten left review comments

@nija-at nija-at nija-at left review comments

Assignees

@nija-at nija-at

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Lambda Event Source - failure handling for kinesis and dynamodb event sources
5 participants
@xerofun @aws-cdk-automation @nija-at @duarten @AlexD1979