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-destinations): option to auto-extract the payload when using LambdaDestination #5503

Merged
merged 13 commits into from
Jan 17, 2020
Merged

feat(lambda-destinations): option to auto-extract the payload when using LambdaDestination #5503

merged 13 commits into from
Jan 17, 2020

Conversation

jogold
Copy link
Contributor

@jogold jogold commented Dec 20, 2019 •
edited
Loading

Add option to send only the response payload when using LambdaDestination.

When set to true, the default event bus is set as destination and a rule
that extracts the payload triggers the destination function.

This allows to easily "chain" Lambda functions without having to deal with the
event format in the runtime code.

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

@jogold
feat(lambda): asynchronous lambda chain
bb17d88 
Add a `LambdaChain` construct to chain asynchronous invocations of Lambda
functions.

Asynchronously invoking the first function starts the execution chain. The
response payload of a successful invocation of a function in the chain is
passed as request payload to the next function automatically using
EventBridge.

Allows to start a series of long running processes.

The poor man's step function?
@jogold
Copy link
Contributor Author

jogold commented Dec 20, 2019 •
edited
Loading

Does it fit in here (cdk)? Maybe something for @aws-cdk/aws-lambda-patterns...

@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 reviewed Jan 9, 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.

This is an interesting pattern Jonathan.

CDK packages will always carry constructs that abstract away the CloudFormation resource constructs (i.e., Cfn*). Beyond this, we would also carry patterns that we think are of high demand or highly common to CDK customers. We've drawn this line based on the cost of ownership and maintenance.

I suggest that you could perhaps maintain and publish your own CDK patterns module, of which this could be the first. When this is ready, we'll be totally happy to accept a PR which adds a link to your module into lambda module's README under a new section called something like 'Related Projects'.

packages/@aws-cdk/aws-lambda-destinations/lib/chain.ts Outdated Show resolved Hide resolved
@jogold
Copy link
Contributor Author

jogold commented Jan 9, 2020

This is an interesting pattern Jonathan.

CDK packages will always carry constructs that abstract away the CloudFormation resource constructs (i.e., Cfn*). Beyond this, we would also carry patterns that we think are of high demand or highly common to CDK customers. We've drawn this line based on the cost of ownership and maintenance.

Perfectly get that and I agree that this PR goes a bit too far here. This was anyway fun to develop. Rest assured that I don't take this personally of course.

I suggest that you could perhaps maintain and publish your own CDK patterns module, of which this could be the first. When this is ready, we'll be totally happy to accept a PR which adds a link to your module into lambda module's README under a new section called something like 'Related Projects'.

Had also this discussion with @eladb yesterday. Not sure I have time to do this right now, but noted!

@jogold jogold changed the title feat(lambda): asynchronous lambda chain feat(lambda-destinations): LambdaPayloadDestination Jan 9, 2020
@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

eladb
eladb reviewed Jan 12, 2020
View reviewed changes
Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

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

I am wondering about LambdaPayloadDestination... Would it make sense to hide this behind an option of LambdaDestination instead of introducing another destination type?

packages/@aws-cdk/aws-lambda-destinations/lib/event-bridge.ts Outdated Show resolved Hide resolved
@jogold
Copy link
Contributor Author

jogold commented Jan 12, 2020

Would it make sense to hide this behind an option of LambdaDestination instead of introducing another destination type

OK. Any ideas for the name of this option?

@jogold jogold changed the title feat(lambda-destinations): LambdaPayloadDestination feat(lambda-destinations): option to send response only with LambdaDestination Jan 13, 2020
@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

eladb
eladb reviewed Jan 16, 2020
View reviewed changes
Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

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

LGTM, @nija-at ?

@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 changed the title feat(lambda-destinations): option to send response only with LambdaDestination feat(lambda-destinations): option to auto-extract the payload when using LambdaDestination Jan 16, 2020
@mergify mergify bot dismissed nija-at’s stale review January 17, 2020 13:55

Pull request has been modified.

@jogold
Copy link
Contributor Author

jogold commented Jan 17, 2020

@nija-at improved README, JSDoc and integ test.

@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 17, 2020
View reviewed changes
packages/@aws-cdk/aws-lambda-destinations/README.md Show resolved Hide resolved
packages/@aws-cdk/aws-lambda-destinations/README.md Show resolved Hide resolved
packages/@aws-cdk/aws-lambda-destinations/README.md Show resolved Hide resolved
packages/@aws-cdk/aws-lambda-destinations/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-lambda-destinations/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-lambda-destinations/README.md
* For EventBridge, the JSON is passed as the `Detail` in the PutEvents call. The source is `lambda`,
and detail type is either `Lambda Function Invocation Result - Success` or `Lambda Function Invocation Result – Failure`.
The resource fields contain the function and destination ARNs.

Copy link
Contributor

@nija-at nija-at Jan 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Can you add a couple of sentences here on how all this connects to the CDK? As in, when a customer connects SqsDestination, LambdaDestination, etc., what should they expect as the sqs message, the lambda payload, etc.?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • For SNS/SQS, the invocation record JSON is passed as the Message to the destination.
  • For Lambda, the invocation record JSON is passed as the payload to the function.
  • For EventBridge, the invocation record JSON ...

Isn't this the same?

Maybe like this?

  • For SNS/SQS (SnsDestionation/SqsDestination), the invocation record JSON is passed as the Message to the destination.
  • For Lambda (LambdaDestination), the invocation record JSON is passed as the payload to the function.
  • For EventBridge (EventBridgeDestination), the invocation record JSON ...

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes, that should be good enough.

packages/@aws-cdk/aws-lambda-destinations/lib/lambda.ts Show resolved Hide resolved
@jogold @nija-at
Apply suggestions from code review
bcec18f 
Co-Authored-By: Niranjan Jayakar <16217941+nija-at@users.noreply.github.com>
@mergify mergify bot dismissed nija-at’s stale review January 17, 2020 14:45

Pull request has been modified.

@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

@mergify
Copy link
Contributor

mergify bot commented Jan 17, 2020

Thank you for contributing! Your pull request is now being automatically merged.

@mergify mergify bot merged commit 321372f into aws:master Jan 17, 2020
@jogold jogold deleted the lambda-chain branch January 17, 2020 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eladb eladb eladb left review comments

@nija-at nija-at nija-at approved these changes

Assignees

@nija-at nija-at

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jogold @aws-cdk-automation @eladb @nija-at