Add a datasource for google_compute_instance #1906
Conversation
rileykarson
force-pushed
the
data-source-compute-instance
branch
from
e7ce2f4 to
c0bb4b4
Compare
rileykarson
force-pushed
the
data-source-compute-instance
branch
from
c0bb4b4 to
b705cbf
Compare
|
Travis will maybe fail because of Updated google_compute_instance datasource to use the resources' schema. I kept the custom read and corresponding docs - we can't read in the state so a lot of deprecated fields just plain won't work. Some fields will "exist" to Terraform but are undocumented and have no value as a result. |
There was a problem hiding this comment.
I'm not wild about the tight coupling to the resource, but I also don't see any good alternative. I think you've made the best of an unfortunate situation, honestly. Could we add a comment above the resource schema, just documenting that the data source schema relies on it, so there's at least a higher chance of people remembering the data source when modifying the resource schema?
Other than that, I had a few minor comments. The big one is just one on design--I feel a lot of users will want to be able to retrieve the instance using the self_link, not just the name, and if we can make that straightforward, that's probably worthwhile to do.
|
|
||
| The following arguments are supported: | ||
|
|
||
| * `name` - (Required) The name of the instance. |
There was a problem hiding this comment.
Could we allow retrieval by self_link, as well? I think that'll be a highly requested use case.
There was a problem hiding this comment.
Sure!
To do this I took the logic from google_compute_region_instance_group's data source and parameterised it for consistency between the two. The test for that data source will fail but it was failing before and fails in the same way. Sorry for the scope of changes as a result of touching both resources here in this already large PR!
rileykarson
force-pushed
the
data-source-compute-instance
branch
from
b705cbf to
d0fdebc
Compare
* Add a compute datasource. * Add provider changes. * Add self_link support to google_compute_instance datasource. * Error check complex d.Set calls.
|
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
Add a datasource for GCE VM Instances (google_compute_instance).
As many deprecated/useless fields are removed as possible; a few are remaining with the "Removed" tag and are undocumented; they required editing the
google_compute_instanceresource (unless we wanted to copy a lot of almost identical read code) & would have bloated the PR which is already fairly large. If we want to remove them for, it should probably be done as a separate refactor.The test covers a slight modified
_basicconfig, and has attribute tests to smoke test a few nested fields as well as coverage of all the top-level ones. It's not much, but most of the code is shared withgoogle_compute_instanceanyways & no other data sources had that many tests. I can gladly add more, just let me know what you'd like more test coverage on!