Add Google Spanner Support (google_spanner_instance) #270
Conversation
nickithewatt
force-pushed
the
spanner-support-instance
branch
from
7ab852d to
121e64b
Compare
| Type: schema.TypeString, | ||
| Optional: true, | ||
| ForceNew: true, | ||
| }, |
There was a problem hiding this comment.
Add the computed state field.
google/resource_spanner_instance.go
Outdated
| d.Set("labels", instance.Labels) | ||
| d.Set("display_name", instance.DisplayName) | ||
| d.Set("num_nodes", instance.NodeCount) | ||
| d.SetId(id.terraformId()) |
There was a problem hiding this comment.
No need to set the id here. It will be set either in the import method or the create method.
google/resource_spanner_instance.go
Outdated
| uir.FieldMask = strings.Join(fieldMask, ",") | ||
| op, err := config.clientSpanner.Projects.Instances.Patch( | ||
| id.instanceUri(), uir).Do() | ||
|
|
There was a problem hiding this comment.
Add a block to handle the error from calling .Patch(...).Do() here.
google/resource_spanner_instance.go
Outdated
| "labels": { | ||
| Type: schema.TypeMap, | ||
| Optional: true, | ||
| ForceNew: true, |
There was a problem hiding this comment.
The patch API support labels updates. Remove this ForceNew and add support for it in the update method below.
google/resource_spanner_instance.go
Outdated
| fieldMask = append(fieldMask, "displayName") | ||
| uir.Instance.DisplayName = d.Get("display_name").(string) | ||
| } | ||
|
|
There was a problem hiding this comment.
Add a block for changes to labels
google/resource_spanner_instance.go
Outdated
| }, nil | ||
| } | ||
|
|
||
| func extractInstanceConfigFromApi(nameUri string) string { |
There was a problem hiding this comment.
Consider renaming to extractInstanceConfigFromUri(configUri string) string
google/resource_spanner_instance.go
Outdated
| return extractLastResourceFromUri(nameUri) | ||
| } | ||
|
|
||
| func extractInstanceNameFromApi(nameUri string) string { |
There was a problem hiding this comment.
Consider renaming to extractInstanceNameFromUri(nameUri string) string
|
|
||
| // Unit Tests | ||
|
|
||
| func TestExtractInstanceConfigFromApi_withFullPath(t *testing.T) { |
There was a problem hiding this comment.
Renaming test accordingly if you change the method name.
|
|
||
| ## Attributes Reference | ||
|
|
||
| No additional attributes are computed other than those defined above. |
There was a problem hiding this comment.
Don't forget to add the state computed attribute in this section.
|
Thanks, aiming to try and get round to these changes some time in the next few days.
|
nickithewatt
force-pushed
the
spanner-support-instance
branch
from
945b69e to
76b2e6f
Compare
|
@rosbo thanks for review: Changes made as requested. Let me know if there is anything else that needs doing on this :) |
|
make testacc TESTARGS="-run TestAccGoogleSpannerInstance_" TEST="./google" make testacc TESTARGS="-run TestAccSpannerInstance_" TEST="./google" |
|
Thank you so much for adding Spanner support to Terraform! |
|
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! |
ghost
locked and limited conversation to collaborators
This PR aims to addresses #67 which looks to add Google Spanner Support, specifically adding a new resource
google_spanner_instance. CC @rileykarson @danawillowIn order to make these PR's easier to digest and review, instead of creating one big PR with all Spanner support in it, I have created one for each new resource to be added. This is the first which adds support for the foundational
google_spanner_instanceresource. A separate one (#271) adds support for creating the Database within the Instance, i.e. thegoogle_spanner_databaseresource.