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.

Sign up for GitHub

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

Jump to bottom

Implement Filestore Instance. #2088

Merged
merged 3 commits into from
Sep 22, 2018
Merged

Implement Filestore Instance. #2088

merged 3 commits into from
Sep 22, 2018

Conversation

modular-magician
Copy link
Collaborator

@modular-magician modular-magician commented Sep 20, 2018 •
edited by nat-henderson
Loading

/cc @ndmckinley

Fixes #1774.

@ghost ghost added the size/xs label Sep 20, 2018
@modular-magician modular-magician mentioned this pull request Sep 20, 2018
Merged
@nat-henderson
Copy link
Contributor 

==> Checking that code complies with gofmt requirements...                     
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google -v -run=TestAccFilestoreInstance -timeout 120m                                                           
=== RUN   TestAccFilestoreInstance_basic                                       
=== PAUSE TestAccFilestoreInstance_basic                                       
=== RUN   TestAccFilestoreInstance_update                                      
=== PAUSE TestAccFilestoreInstance_update                                      
=== CONT  TestAccFilestoreInstance_basic                                       
=== CONT  TestAccFilestoreInstance_update                                      
--- PASS: TestAccFilestoreInstance_basic (267.75s)                             
--- PASS: TestAccFilestoreInstance_update (329.30s)                            
PASS                                   
ok      github.com/terraform-providers/terraform-provider-google/google 329.319s

danawillow
danawillow suggested changes Sep 21, 2018
View reviewed changes
google/resource_file_instance_test.go Outdated
`, name)
}

func testAccFilestoreInstance_update2(name string) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not immediately seeing the difference between this func and the first update one, what's getting updated?

Copy link
Contributor

Choose a reason for hiding this comment

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

The labels, if I had remembered to remove the labels so I could check that that worked. :)

google/resource_redis_instance.go Outdated
func expandRedisInstanceRegion(v interface{}, d *schema.ResourceData, config *Config) (interface{}, error) {
return v, nil
}

func resourceRedisInstanceEncoder(d *schema.ResourceData, meta interface{}, obj map[string]interface{}) (map[string]interface{}, error) {
delete(obj, "region")
Copy link
Contributor

Choose a reason for hiding this comment

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

this custom encoder can probably be deleted now that we don't add it in the first place!

website/docs/r/file_instance.html.markdown Outdated

To get more information about Instance, see:

* [API documentation](https://cloud.google.com/memorystore/docs/redis/reference/rest/)
Copy link
Contributor

Choose a reason for hiding this comment

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

this needs to be updated

website/docs/r/file_instance.html.markdown Outdated
## Example Usage

### Basic Usage
```hcl
Copy link
Contributor

Choose a reason for hiding this comment

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

fill in?

website/docs/r/file_instance.html.markdown Outdated
* `name` -
(Required)
The resource name of the instance, in the format
`projects/{project_id}/locations/{locationId}/instances/{instanceId}`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this actually the case? I think we allow just {instanceId}, and if not, we should

website/docs/r/file_instance.html.markdown Outdated
A Google Cloud Filestore instance.
---

# google\_file\_instance
Copy link
Contributor

Choose a reason for hiding this comment

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

I kind of want this to be named google_filestore_instance. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I really do - what do we do to change the name away from the actual API name? I couldn't figure that one out just from looking, and I didn't spend more than a few minutes poking around.

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 it's the prefix field in api.yaml (or at least I hope that's all that field does).

@nat-henderson
Copy link
Contributor

Agh, I always forget about the autodocs. In progress.

@ghost ghost added the size/xxl label Sep 21, 2018
@nat-henderson
Copy link
Contributor

@danawillow there we go! probably! :)

TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google -v -run=TestAccFilestoreInstance -timeout 120m
=== RUN   TestAccFilestoreInstance_basic
=== PAUSE TestAccFilestoreInstance_basic
=== RUN   TestAccFilestoreInstance_update
=== PAUSE TestAccFilestoreInstance_update
=== CONT  TestAccFilestoreInstance_basic
=== CONT  TestAccFilestoreInstance_update
--- PASS: TestAccFilestoreInstance_basic (266.92s)
--- PASS: TestAccFilestoreInstance_update (343.88s)
PASS
ok      github.com/terraform-providers/terraform-provider-google/google 343.895s

danawillow
danawillow approved these changes Sep 21, 2018
View reviewed changes
Copy link
Contributor

@danawillow danawillow left a comment

Choose a reason for hiding this comment

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

Don't forget to also add an entry to the docs sidebar!

@danawillow
Copy link
Contributor

Oh also, put fixes #1774 somewhere :)

@ghost ghost added the size/xxl label Sep 22, 2018
@nat-henderson
Copy link
Contributor

Done!

@nat-henderson nat-henderson merged commit 0790c96 into hashicorp:master Sep 22, 2018
chrisst pushed a commit to chrisst/terraform-provider-google that referenced this pull request Nov 9, 2018
@modular-magician @nat-henderson
Implement Filestore Instance. (hashicorp#2088)
917e287
@ghost
Copy link

ghost commented Nov 16, 2018

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 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Nov 16, 2018
@modular-magician modular-magician deleted the codegen-pr-482 branch November 16, 2024 20:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nat-henderson nat-henderson nat-henderson left review comments

@danawillow danawillow danawillow approved these changes

Assignees
No one assigned
Labels
size/xxl
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@modular-magician @nat-henderson @danawillow