-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
provider/aws: SNS Topic State Refreshing correctly #4891
Conversation
@@ -163,6 +165,12 @@ func resourceAwsSnsTopicRead(d *schema.ResourceData, meta interface{}) error { | |||
}) | |||
|
|||
if err != nil { | |||
if ec2err, ok := err.(awserr.RequestFailure); ok && ec2err.StatusCode() == http.StatusNotFound { |
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.
Usually (at least from what I've seen elsewhere in the codebase) we check ec2err.Code()
(which in this case would be NotFound
) which would help you avoiding that extra import of net/http
.
Also (but that's really a nitpick), this is a generic AWS error, has nothing to do with EC2. I know this pattern is sprinkled throughout the codebase with EC2 in the name which makes me think we should probably rename all of those variables.
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.
Makes sense @radeksimko - will fix and re-push first thing
throw an error: * aws_sns_topic_subscription.checker: NotFound: Subscription does not * exist status code: 404, request id: b8ca0c27-1a62-57b3-8b96-43038a0ead86 Terraform wasn't refreshing the state when the topic gave a 404
9210201
to
91cb65d
Compare
@radeksimko ok, this was a pretty simple change to make - just repushed it |
LGTM, feel free to merge once Travis turns to green. |
Also after merging don't forget to update the Changelog.md, we typically do this manually via github web interface (to avoid conflicts). 😉 |
provider/aws: SNS Topic State Refreshing correctly
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. |
Fixes #4888
Provider/aws - SNS Topics deleted from the UI were causing Terraform to throw an error:
status code: 404, request id: b8ca0c27-1a62-57b3-8b96-43038a0ead86
Terraform wasn't refreshing the state when the topic gave a 404