-
Notifications
You must be signed in to change notification settings - Fork 211
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.
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
Support reading S3 Event messages from SNS fan-out #2622
Support reading S3 Event messages from SNS fan-out #2622
Conversation
…ssage is wrapped in the Message key. Signed-off-by: David Venable <[email protected]>
} | ||
|
||
private boolean isSnsWrappedMessage(final JsonNode parsedNode) { | ||
return parsedNode.has(SNS_MESSAGE_KEY); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also validate the type of the message or else we are trying to parse any JOSN node with Message
? Looks like the JSON object will have "Type" : "Notification"
field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is fine if the Type doesn't match. If the data comes and there is a Message
which is not what we expect, we will still have a JSON parsing exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, just looked at the failure scenario looks like we log the following if there is a failure.
LOG.error("SQS message with message ID:{} has invalid body which cannot be parsed into S3EventNotification. {}.", message.messageId(), e.getMessage());
} | ||
|
||
private boolean isSnsWrappedMessage(final JsonNode parsedNode) { | ||
return parsedNode.has(SNS_MESSAGE_KEY); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, just looked at the failure scenario looks like we log the following if there is a failure.
LOG.error("SQS message with message ID:{} has invalid body which cannot be parsed into S3EventNotification. {}.", message.messageId(), e.getMessage());
Signed-off-by: David Venable <[email protected]>
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #2622 +/- ##
============================================
+ Coverage 93.45% 93.53% +0.08%
- Complexity 2185 2220 +35
============================================
Files 258 258
Lines 6107 6230 +123
Branches 497 512 +15
============================================
+ Hits 5707 5827 +120
+ Misses 268 266 -2
- Partials 132 137 +5 |
Description
Adds a new class for parsing for S3 Event notifications. Detect if the message was wrapped by SNS to SQS by checking for the presence of a
Message
key. Then parse that as a string.Issues Resolved
Resolves #2604
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.