-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Fix queue url generation for redrivePolicy in SNS events. #7277
Conversation
I have tested this locally with one of my projects and I could deploy it sucessfully after this fix 👍 |
@@ -335,6 +335,11 @@ module.exports = { | |||
getTopicLogicalId(topicName) { | |||
return `SNSTopic${this.normalizeTopicName(topicName)}`; | |||
}, | |||
getTopicDLQPolicyLogicalId(functionName, topicName) { |
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've created this, since the previously generated names for the queue policy looked wrong (were referencing eventMapping) since they were designed for a Queue and not for a QueuePolicy
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.
Thnank you @tcastelli looks good. Let me know what you think about my simplification proposal
Just updated the PR with latest changes requested. Please take a look when you can @medikoo |
Codecov Report
@@ Coverage Diff @@
## master #7277 +/- ##
==========================================
+ Coverage 87.95% 87.96% +<.01%
==========================================
Files 240 240
Lines 8969 8972 +3
==========================================
+ Hits 7889 7892 +3
Misses 1080 1080
Continue to review full report at Codecov.
|
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.
Looks great @tcastelli ! Taking this in
Fixes the error introduced in #7239 by not generating the queue Url and allowing to pass Ref as an input. Now the deadLetterTargetArn can be either defined as a string or using Fn::GetAtt, and the url will be generated automatically from that.
Still needs more testing but can be reviewed for structural decissions
Closes #7276