Add data source for forwarding rule #1078
Conversation
|
|
||
| "load_balancing_scheme": &schema.Schema{ | ||
| Type: schema.TypeString, | ||
| Optional: true, |
There was a problem hiding this comment.
Should these also be Computed? And are they really Optional? It looks like they'll be overwritten as soon as the read happens, regardless of their value.
There was a problem hiding this comment.
Which parameters are you exactly referring to? Note: Here I've reused the same schema as in resource_compute_forwarding_rule.go.
There was a problem hiding this comment.
Right, but as this is a data source, fundamentally all the parameters except the name, region, and project are Computed - no changes are made based on the user setting them.
There was a problem hiding this comment.
Ok, so just to be clear, are you saying that this should be the schema? https://gist.github.com/jphalip/c0409e5b3e898898ce7706eb48b83950
There was a problem hiding this comment.
That looks right to me, yes - I think the tests should pass with that schema, and that accurately represents what this does.
| d.Set("project", project) | ||
| d.Set("region", region) | ||
|
|
||
| d.SetId(frule.Name) |
There was a problem hiding this comment.
This is a tad silly in this instance, but we have a convention of preferring the SetId to go right after the confirmation of successful read. That way the item is refreshable in the event of a shutdown or panic from some other thread, rather than requiring recreation.
There was a problem hiding this comment.
Just pushed an update for that. Does that work?
|
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
locked and limited conversation to collaborators
No description provided.