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

Add VPN gateway data source #1071

Merged
merged 8 commits into from Feb 12, 2018

Conversation

jphalip
Copy link
Contributor

@jphalip jphalip commented Feb 11, 2018

No description provided.

Required: true,
},

"description": &schema.Schema{
Copy link
Contributor

Choose a reason for hiding this comment

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

We order these fields "Required, Optional, Computed" - A field which is both Optional and Computed counts as Optional for this ordering.


gateway, err := vpnGatewaysService.Get(project, region, name).Do()
if err != nil {
if gerr, ok := err.(*googleapi.Error); ok && gerr.Code == 404 {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you want handleNotFoundError.

Copy link
Contributor

Choose a reason for hiding this comment

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

This actually includes the 404 check - it returns the not-found error if the resource doesn't exist, and a regular error otherwise. Just if err != nil { return handleNotFoundError(...) }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've just pushed another change. Does that work?
Also, I had modeled the original error handling code on existing examples such as data_source_compute_network, data_source_compute_subnetwork or data_source_compute_global_address. There might be other such examples. Are those incorrect as well?

@jphalip
Copy link
Contributor Author

jphalip commented Feb 12, 2018

Thanks @ndmckinley, I've pushed the requested updates.


gateway, err := vpnGatewaysService.Get(project, region, name).Do()
if err != nil {
if gerr, ok := err.(*googleapi.Error); ok && gerr.Code == 404 {
Copy link
Contributor

Choose a reason for hiding this comment

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

This actually includes the 404 check - it returns the not-found error if the resource doesn't exist, and a regular error otherwise. Just if err != nil { return handleNotFoundError(...) }

}

return fmt.Errorf("Error reading VPN Gateway: %s", err)
// The resource doesn't exist anymore
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment out of date now - sorry for the nitpicks!

@nat-henderson
Copy link
Contributor

Thanks for working with me on that! Running the tests now, and will merge when they're done.

@jphalip
Copy link
Contributor Author

jphalip commented Feb 12, 2018

No worries! By the way, did you see my comment regarding the code for the network, subnetwork and global-address data sources? I had modeled my original error handling code based on those. Are they incorrect?

@nat-henderson
Copy link
Contributor

Thanks for calling that out! You're right, those did not move to the new-model 404 handler - we should fix it. :)

@jphalip
Copy link
Contributor Author

jphalip commented Feb 12, 2018

Ok I see, I'm happy to submit a separate PR for that. I'll ping you when it's done.

@nat-henderson
Copy link
Contributor

Oh wow, that's great! Thanks! :)

@nat-henderson nat-henderson merged commit f6c5f01 into hashicorp:master Feb 12, 2018
chrisst pushed a commit to chrisst/terraform-provider-google that referenced this pull request Nov 9, 2018
modular-magician added a commit to modular-magician/terraform-provider-google that referenced this pull request Sep 27, 2019
Signed-off-by: Modular Magician <magic-modules@google.com>
@ghost
Copy link

ghost commented Mar 29, 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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@hashicorp hashicorp locked and limited conversation to collaborators Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants