feat: Add sns notification support to Parser utility #206#207
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #207 +/- ##
========================================
Coverage 99.87% 99.88%
========================================
Files 67 69 +2
Lines 2499 2543 +44
Branches 108 109 +1
========================================
+ Hits 2496 2540 +44
Misses 3 3
Continue to review full report at Codecov.
|
| from typing_extensions import Literal | ||
|
|
||
|
|
||
| class SqsMsgAttributeModel(BaseModel): |
There was a problem hiding this comment.
Meant SNS instead of SQS?
| class SqsMsgAttributeModel(BaseModel): | |
| class SnsMsgAttributeModel(BaseModel): |
heitorlessa
left a comment
There was a problem hiding this comment.
It looks good to me except FIFO if we want to be future proof
| Value: str | ||
|
|
||
|
|
||
| class SnsNotificationModel(BaseModel): |
There was a problem hiding this comment.
We need groupId and dedupId optional fields that might be added when FIFO is enabled - Haven't tried yet myself.
Publisher reference: https://docs.aws.amazon.com/sns/latest/dg/fifo-topic-code-examples.html#fifo-topic-java-publish
There was a problem hiding this comment.
Resolving it as @risenberg-cyberark confirmed Lambda cannot directly subscribe to SNS FIFO topics, it'd have to go through SQS FIFO, in which case our model already covers it.
heitorlessa
left a comment
There was a problem hiding this comment.
Thanks for adding support to SNS @risenberg-cyberark :) much appreciated. Merging it now
Issue #, if available:
Description of changes:
Checklist
Breaking change checklist
RFC issue #:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.