Initial SNS topic / subscription support #1974
Conversation
|
Thanks, did you have any chance to look at #1952 ? |
|
Yet another coincidence .. John tells me he's working on something, and a separate PR lands for that thing days/hours before John sends his 😄 Sorry for the trouble @johnewart – one way or the other we'll get SNS and SQS in :P |
|
@radeksimko Mine uses the ARN as the ID, which is arguably better since it's guaranteed to be unique, and also supports subscriptions. |
|
@catsby I think I'm somehow controlling the universe with my mind; maybe I'm secretly Professor X and I just don't know it yet... |
|
Errr, @johnewart -- what? Mine uses ARN as the ID: https://github.com/hashicorp/terraform/pull/1952/files#diff-d2b599683afe94c602d86b9526c9b552R52. Check line 52 of the resource. It wouldn't make a whit of sense to use anything else. Regardless go ahead and use this. It provides support for subscriptions and various other SNS topic properties. |
|
Also are you adding support to AS groups for SNS notifications? I've got that tentatively in flight but have no real interest in mucking with it if someone's already on it. |
|
@ctiwald Sorry, I misread it when I was looking over your PR briefly; I (incorrectly) interpreted that the "seeker" as a way to fetch the ARN for a topic name. I hadn't gotten to ASG integration but it should be pretty easy to add for both SNS and SQS now that there's the SQS resource that has been merged. |
|
No worries at all. I feel foolish not seeing that "GetTopicAttributes" API endpoint. Would love to see the ASG stuff added. I'm happy to contribute it but probably won't get around to it until late this week. If you all are devoting resources to it, that's cool too. |
|
Thanks @johnewart – is this good to go from your end, then? The code looks great 👍 Can you please rebase and make sure to run |
…or new AWS SDK errors; updated documentation.
|
Updated with the new AWS SDK error changes, plus added |
|
|
||
| d.SetId(*output.TopicARN) | ||
|
|
||
| // Write the ARN to the 'arn' field for export |
There was a problem hiding this comment.
AFAIK id is being exported by default, therefore this seems to be a bit superfluous, or am I wrong?
There was a problem hiding this comment.
Which part? Exporting ARN?
|
LGTM, thanks everyone! |
Initial SNS topic / subscription support
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
Initial SNS topic and subscription support