feat: Add SQS option to SNS event #1065

53ningen · 2019-08-06T07:50:04Z

Issue #, if available:
#854

This pull request will resolve #854 partially.
I implemented simple boolean SqsSubscription option to SNSEvent at first.

Description of changes:
Added SQS option(SqsSubscription) to SNS event.

Description of how you validated changes:

make pr
verified transformed template in the steps written in DEVELOPMENT_GUIDE
confirmed that SAM template below could be deployed successfully.

AWSTemplateFormatVersion: '2010-09-09'
Transform: AWS::Serverless-2016-10-31

Resources:
  TweetTopic:
    Type: AWS::SNS::Topic
  WorkerFunction:
    Type: AWS::Serverless::Function
    Properties:
      CodeUri: worker_lambda/
      Handler: app.lambda_handler
      Timeout: 10
      Runtime: python3.7
      Events:
        MessageTopic:
          Type: SNS
          Properties:
            Topic: !Ref TweetTopic
            SqsSubscription: true

Checklist:

Write/update tests
make pr passes
Update documentation
Verify transformed template deploys and application functions as expected
[-] Add/update example to examples/2016-10-31

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

codecov-io · 2019-08-06T07:53:56Z

Codecov Report

Merging #1065 into develop will increase coverage by 0.03%.
The diff coverage is 97.82%.

@@             Coverage Diff             @@
##           develop    #1065      +/-   ##
===========================================
+ Coverage    94.72%   94.75%   +0.03%     
===========================================
  Files           70       71       +1     
  Lines         3430     3472      +42     
  Branches       675      678       +3     
===========================================
+ Hits          3249     3290      +41     
  Misses          93       93              
- Partials        88       89       +1

Impacted Files	Coverage Δ
samtranslator/model/eventsources/push.py	`89.05% <100%> (+0.76%)`	⬆️
samtranslator/model/sqs.py	`93.33% <93.33%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7a9ddc...f36fd1a. Read the comment docs.

53ningen · 2019-08-06T07:57:21Z

samtranslator/model/eventsources/push.py

+        return SQSQueue(self.logical_id + 'Queue')
+
+    def _inject_sqs_event_source_mapping(self, function, role, queue_arn):
+        event_source = SQS(self.logical_id + 'EventSourceMapping')


Is this correct way to create LambdaEventSourceMapping resource?
Should I add LambdaEventSourceMapping and put linked_policy to IAM Role directory instead use SQS object?

I think the logic to create LambdaEventSourceMapping and linked_policy is completely same as SQS pull event. So I use SQS object.

It looks like that is the correct way to add this event source mapping 👍

53ningen · 2019-08-06T08:00:46Z

versions/2016-10-31.md

@@ -429,6 +429,7 @@ Property Name | Type | Description
 Topic | `string` | **Required.** Topic ARN.
 Region | `string` | Region.
 FilterPolicy | [Amazon SNS filter policy](https://docs.aws.amazon.com/sns/latest/dg/message-filtering.html) | Policy assigned to the topic subscription in order to receive only a subset of the messages.
+SqsSubscription | `boolean` | Indicates whether SQS option is enabled.


I'm sorry but I'm non-native english speaker so could anyone provide appropriate description for SqsSubscription?

Sure. I would say:

SqsSubscription | `boolean` | Set to `true` to enable batching SNS topic notifications in an SQS queue.

Thank you for giving me a hand!
I made suggested change in e7e2dd8.

keetonian

This looks like a great feature, thank you for contributing this! Just a few comments->

Could you also update https://github.com/awslabs/serverless-application-model/blob/develop/docs/internals/generated_resources.rst#sns to explain that a new SQS resource is created if SqsSubscription: true is used?

keetonian · 2019-08-09T18:07:59Z

versions/2016-10-31.md

@@ -429,6 +429,7 @@ Property Name | Type | Description
 Topic | `string` | **Required.** Topic ARN.
 Region | `string` | Region.
 FilterPolicy | [Amazon SNS filter policy](https://docs.aws.amazon.com/sns/latest/dg/message-filtering.html) | Policy assigned to the topic subscription in order to receive only a subset of the messages.
+SqsSubscription | `boolean` | Indicates whether SQS option is enabled.


Sure. I would say:

SqsSubscription | `boolean` | Set to `true` to enable batching SNS topic notifications in an SQS queue.

keetonian · 2019-08-09T18:17:28Z

samtranslator/model/eventsources/push.py

+        return SQSQueue(self.logical_id + 'Queue')
+
+    def _inject_sqs_event_source_mapping(self, function, role, queue_arn):
+        event_source = SQS(self.logical_id + 'EventSourceMapping')


It looks like that is the correct way to add this event source mapping 👍

53ningen

I have addressed your pull request comments.
Ready for review again.

praneetap · 2019-08-19T18:31:36Z

@53ningen Thank you for contributing this very important feature! Can you please add an example of how to use this in the examples folder? This is a great example of how to do it.

53ningen · 2019-08-25T08:08:55Z

Added an example of how to use new SqsSubscription option.
Ready for review again.

53ningen added 2 commits August 3, 2019 14:24

refactor: SNS._inject_subscription method

8bf2077

feat: Add SQS option to SNS event

8ec4b96

53ningen commented Aug 6, 2019

View reviewed changes

keetonian suggested changes Aug 9, 2019

View reviewed changes

Update versions/2016-10-31.md

e7e2dd8

53ningen commented Aug 10, 2019

View reviewed changes

Update docs/internals/generated_resources.rst

2ed258f

53ningen force-pushed the feature/sns-sqs-lambda branch from 19f3525 to 2ed258f Compare August 10, 2019 06:19

keetonian approved these changes Aug 15, 2019

View reviewed changes

praneetap self-requested a review August 16, 2019 17:58

praneetap self-assigned this Aug 16, 2019

praneetap assigned 53ningen and praneetap and unassigned praneetap and 53ningen Aug 19, 2019

keetonian and others added 2 commits August 25, 2019 16:52

Update generated_resources.rst

abd53c7

Added an example of how to use SqsSubscription option

f36fd1a

53ningen force-pushed the feature/sns-sqs-lambda branch from 0acd583 to f36fd1a Compare August 25, 2019 08:07

praneetap approved these changes Aug 27, 2019

View reviewed changes

praneetap merged commit b3e4b6c into aws:develop Aug 27, 2019

53ningen deleted the feature/sns-sqs-lambda branch August 27, 2019 04:03

praneetap added stage/waiting-for-release labels Aug 27, 2019

praneetap mentioned this pull request Sep 19, 2019

Release V1.15.0 #1147

Closed

53ningen mentioned this pull request Oct 5, 2019

Feature Request: Add SQS option to SNS event #854

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add SQS option to SNS event #1065

feat: Add SQS option to SNS event #1065

53ningen commented Aug 6, 2019 •

edited

Loading

codecov-io commented Aug 6, 2019 •

edited

Loading

53ningen Aug 6, 2019

keetonian Aug 9, 2019

53ningen Aug 6, 2019

keetonian Aug 9, 2019

53ningen Aug 10, 2019

keetonian left a comment

keetonian Aug 9, 2019

keetonian Aug 9, 2019

53ningen left a comment

praneetap commented Aug 19, 2019

53ningen commented Aug 25, 2019 •

edited

Loading

feat: Add SQS option to SNS event #1065

feat: Add SQS option to SNS event #1065

Conversation

53ningen commented Aug 6, 2019 • edited Loading

codecov-io commented Aug 6, 2019 • edited Loading

Codecov Report

53ningen Aug 6, 2019

Choose a reason for hiding this comment

keetonian Aug 9, 2019

Choose a reason for hiding this comment

53ningen Aug 6, 2019

Choose a reason for hiding this comment

keetonian Aug 9, 2019

Choose a reason for hiding this comment

53ningen Aug 10, 2019

Choose a reason for hiding this comment

keetonian left a comment

Choose a reason for hiding this comment

keetonian Aug 9, 2019

Choose a reason for hiding this comment

keetonian Aug 9, 2019

Choose a reason for hiding this comment

53ningen left a comment

Choose a reason for hiding this comment

praneetap commented Aug 19, 2019

53ningen commented Aug 25, 2019 • edited Loading

53ningen commented Aug 6, 2019 •

edited

Loading

codecov-io commented Aug 6, 2019 •

edited

Loading

53ningen commented Aug 25, 2019 •

edited

Loading