Skip to content
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/google: Move 404 checking into a function in provider.go, call it from instance and IGM #14190

Merged
merged 2 commits into from
May 4, 2017

Conversation

danawillow
Copy link
Contributor

Initial PR for #13943. It fixes the specific concern by adding in 404 checking for resource_compute_instance_group_manager, and also shows the new function being reused in resource_compute_instance (note that it also exposed a bug- if there is a 404, we should return nil instead of an error). If this looks good, I'll check it in and then open up a separate PR for the rest of the resources.

Copy link
Contributor

@paddycarver paddycarver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit.

@@ -250,3 +251,15 @@ func getNetworkNameFromSelfLink(network string) (string, error) {

return network, nil
}

func readError(err error, d *schema.ResourceData, resource string) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe handleNotFoundError? readError doesn't really indicate this is for 404s, to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

@paddycarver paddycarver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, merge at will.

@danawillow danawillow merged commit b7d0140 into hashicorp:master May 4, 2017
@danawillow danawillow deleted the is-13943 branch May 4, 2017 23:17
@ghost
Copy link

ghost commented Apr 13, 2020

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 ghost locked and limited conversation to collaborators Apr 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants