feat(lambda-event-sources): expose `eventSourceMappingId` #5689

eliasdraexler · 2020-01-07T13:56:54Z

Commit Message

feat(lambda-event-sources): expose eventSourceMappingId (#5689)

Exposes the Ref of AWS::Lambda::EventSourceMapping as eventSourceMappingId property on the EventSourceMapping and further on the SqsEventSource, KinesisEventSource and DynamoEventSource

Closes #5430

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

aws-cdk-automation · 2020-01-07T14:13:24Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

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

…tSourceMapping Exposes the Ref of `AWS::Lambda::EventSourceMapping` as eventSourceMappingId property on the EventSourceMapping and further on the SqsEventSource, KinesisEventSource and DynamoEventSource Closes aws#5430

aws-cdk-automation · 2020-01-07T15:20:40Z

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 · 2020-01-08T14:23:06Z

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

Update the README.md with a few lines and a code snippet stating the existence of this method. Nothing big. Sufficient if the snippet shows an example with just one of these event sources.

nija-at · 2020-01-21T12:07:36Z

packages/@aws-cdk/aws-lambda/lib/event-source-mapping.ts

@@ -67,21 +67,28 @@ export interface EventSourceMappingProps extends EventSourceMappingOptions {
 * modify the Lambda's execution role so it can consume messages from the queue.
 */
 export class EventSourceMapping extends cdk.Resource {
+  /**
+   * The Ref of the EventSourceMapping


Suggested change

* The Ref of the EventSourceMapping

* The identifier for this EventSourceMapping

nija-at · 2020-01-21T12:09:18Z

packages/@aws-cdk/aws-lambda-event-sources/lib/dynamodb.ts

@@ -9,6 +9,8 @@ export interface DynamoEventSourceProps extends StreamEventSourceProps {
 * Use an Amazon DynamoDB stream as an event source for AWS Lambda.
 */
 export class DynamoEventSource extends StreamEventSource {
+  private _eventSourceMappingId: string | undefined = undefined;


Use ? to declare optional types. Same in other places.

Suggested change

private _eventSourceMappingId: string | undefined = undefined;

private _eventSourceMappingId?: string;

nija-at · 2020-01-21T12:13:13Z

packages/@aws-cdk/aws-lambda-event-sources/lib/dynamodb.ts


    this.table.grantStreamRead(target);
  }
+
+  /**
+   * The Ref of the EventSourceMapping


Update documentation per the other comment in the review

nija-at · 2020-01-21T12:15:06Z

packages/@aws-cdk/aws-lambda-event-sources/lib/dynamodb.ts

+  public get eventSourceMappingId(): string | undefined {
+    return this._eventSourceMappingId;
+  }


I would suggest that we throw an Error here instead of returning undefined, with the message 'DynamoEventSource is not yet bound to an event source mapping'.

nija-at · 2020-01-21T12:16:06Z

packages/@aws-cdk/aws-lambda-event-sources/test/test.dynamo.ts

+    fn.addEventSource(eventSource);
+
+    // THEN
+    test.notEqual(eventSource.eventSourceMappingId, undefined);


Use test.ok().

I would even suggest that you test for equality to the event source id that gets returned here - it'll be a cloudformation intrinsic of the form 'Fn::GetAtt: { ... }'.

In case, it starts with 'Token...', use stack.resolve(eventSource.eventSourceMappingId) to resolve the token and get the CF intrinsic.

I changed it to test.ok but could you maybe elaborate on how I should check equality of the eventSourceId?

stack.resolve(eventSource.eventSourceMappingId) returns {Ref: '...'} where shall I get the id that should be equal to this Ref value?

You cannot. The eventual evenSourceMappingId is generated by cloudformation and our unit tests don't interact with it. You can only check for direct equality with the Ref string.

nija-at · 2020-01-21T12:22:08Z

packages/@aws-cdk/aws-lambda-event-sources/lib/dynamodb.ts

+  /**
+   * The Ref of the EventSourceMapping
+   */
+  public get eventSourceMappingId(): string | undefined {


We should probably define this as a property on IEventSource so that all future event sources also implement.

I thought about it, but there are also classes like SnsEventSource & S3EventSource & ApiEventSource that implement the IEventSource interface and do not have an eventSourceMappingId.

Should I still add it to the interface?

Pull request has been modified.

aws-cdk-automation · 2020-01-28T14:27:35Z

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 · 2020-01-28T17:48:02Z

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 · 2020-02-26T09:58:19Z

@eliasdraexler - I pushed some updates to the README. It was easier to change them myself than provide them as comments. Hope you don't mind.

aws-cdk-automation · 2020-02-26T10:15:52Z

AWS CodeBuild CI Report

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

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

Pull request has been modified.

aws-cdk-automation · 2020-02-27T12:54:29Z

AWS CodeBuild CI Report

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

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

mergify · 2020-02-27T13:26:26Z

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

mergify · 2020-02-27T14:09:25Z

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 · 2020-02-27T14:10:20Z

AWS CodeBuild CI Report

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

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

aws-cdk-automation · 2020-02-27T14:12:53Z

AWS CodeBuild CI Report

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

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

eliasdraexler force-pushed the eliasdraexler/event-source-mapping-id branch from 720f0eb to 5f0c91b Compare January 7, 2020 14:44

SomayaB assigned nija-at Jan 7, 2020

SomayaB added draft pr/work-in-progress This PR is a draft and needs further work. and removed draft labels Jan 7, 2020

Merge branch 'master' into eliasdraexler/event-source-mapping-id

bde8aff

eliasdraexler marked this pull request as ready for review January 8, 2020 13:38

eliasdraexler requested review from eladb and nija-at as code owners January 8, 2020 13:38

nija-at previously requested changes Jan 21, 2020

View reviewed changes

fixed tests, throw error if eventSourceMapping is not bound

ebda1b4

Merge branch 'master' into eliasdraexler/event-source-mapping-id

0c48311

nija-at changed the title ~~feat(lambda-event-sources): add eventSourceMappingId property to Even…~~ feat(lambda-event-sources): expose eventSourceMappingId Feb 26, 2020

updates to the README

c4e65d9

nija-at previously approved these changes Feb 26, 2020

View reviewed changes

Merge branch 'master' into eliasdraexler/event-source-mapping-id

b417d46

nija-at approved these changes Feb 27, 2020

View reviewed changes

Merge branch 'master' into eliasdraexler/event-source-mapping-id

9102d3d

mergify bot merged commit 5ea2679 into aws:master Feb 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(lambda-event-sources): expose `eventSourceMappingId` #5689

feat(lambda-event-sources): expose `eventSourceMappingId` #5689

eliasdraexler commented Jan 7, 2020 •

edited by nija-at

Loading

aws-cdk-automation commented Jan 7, 2020

aws-cdk-automation commented Jan 7, 2020

aws-cdk-automation commented Jan 8, 2020

nija-at left a comment

nija-at Jan 21, 2020

nija-at Jan 21, 2020

nija-at Jan 21, 2020

nija-at Jan 21, 2020

nija-at Jan 21, 2020

eliasdraexler Jan 28, 2020

nija-at Jan 28, 2020

nija-at Jan 21, 2020

eliasdraexler Jan 28, 2020 •

edited

Loading

aws-cdk-automation commented Jan 28, 2020

aws-cdk-automation commented Jan 28, 2020

nija-at commented Feb 26, 2020 •

edited

Loading

aws-cdk-automation commented Feb 26, 2020

aws-cdk-automation commented Feb 27, 2020

mergify bot commented Feb 27, 2020

mergify bot commented Feb 27, 2020

aws-cdk-automation commented Feb 27, 2020

aws-cdk-automation commented Feb 27, 2020

	* The Ref of the EventSourceMapping
	* The identifier for this EventSourceMapping

	private _eventSourceMappingId: string \| undefined = undefined;
	private _eventSourceMappingId?: string;

feat(lambda-event-sources): expose eventSourceMappingId #5689

feat(lambda-event-sources): expose eventSourceMappingId #5689

Conversation

eliasdraexler commented Jan 7, 2020 • edited by nija-at Loading

Commit Message

aws-cdk-automation commented Jan 7, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jan 7, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jan 8, 2020

AWS CodeBuild CI Report

nija-at left a comment

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

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

Choose a reason for hiding this comment

eliasdraexler Jan 28, 2020 • edited Loading

Choose a reason for hiding this comment

aws-cdk-automation commented Jan 28, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jan 28, 2020

AWS CodeBuild CI Report

nija-at commented Feb 26, 2020 • edited Loading

aws-cdk-automation commented Feb 26, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Feb 27, 2020

AWS CodeBuild CI Report

mergify bot commented Feb 27, 2020

mergify bot commented Feb 27, 2020

aws-cdk-automation commented Feb 27, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Feb 27, 2020

AWS CodeBuild CI Report

feat(lambda-event-sources): expose `eventSourceMappingId` #5689

feat(lambda-event-sources): expose `eventSourceMappingId` #5689

eliasdraexler commented Jan 7, 2020 •

edited by nija-at

Loading

eliasdraexler Jan 28, 2020 •

edited

Loading

nija-at commented Feb 26, 2020 •

edited

Loading