compute: added numeric_id to google_compute_subnetwork
#12285
Conversation
|
Hello! I am a robot. Tests will require approval from a repository maintainer to run. @BBBmau, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look. You can help make sure that review is quick by doing a self-review and by running impacted tests locally. |
modular-magician
added
the
awaiting-approval
Added `numeric_id` to `google_compute_subnetwork` resource and data source. Since Terraform uses `id` internally, follow the example of `google_compute_network`, and make `numeric_id` contain the `id` field from the API. Part of hashicorp/terraform-provider-google#20223
wyardley
force-pushed
the
wyardley/issues_20223_pt1
branch
from
a5e00b1
to
3312a35
Compare
| @@ -73,6 +73,9 @@ iam_policy: | |||
| custom_code: | |||
| extra_schema_entry: 'templates/terraform/extra_schema_entry/subnetwork.tmpl' | |||
| constants: 'templates/terraform/constants/subnetwork.tmpl' | |||
| decoder: 'templates/terraform/decoders/compute_subnetwork.go.tmpl' | |||
| encoder: 'templates/terraform/encoders/compute_subnetwork.go.tmpl' | |||
| update_encoder: 'templates/terraform/update_encoder/compute_subnetwork.go.tmpl' | |||
There was a problem hiding this comment.
these are all identical to the ones for compute_network; I'm assuming there's still a preference to have these be copied / pasted vs. sharing a common file with a more generic name?
| @@ -203,6 +206,10 @@ properties: | |||
| custom_expand: 'templates/terraform/custom_expand/resourceref_with_validation.go.tmpl' | |||
| resource: 'Network' | |||
| imports: 'selfLink' | |||
| - name: 'numericId' | |||
| type: String | |||
There was a problem hiding this comment.
It's an integer in the API, but followed the example from google_compute_network of making it a string, so I assume that's preferred?
| @@ -100,6 +105,10 @@ func dataSourceGoogleComputeSubnetworkRead(d *schema.ResourceData, meta interfac | |||
| return transport_tpg.HandleDataSourceNotFoundError(err, d, fmt.Sprintf("Subnetwork Not Found : %s", name), id) | |||
| } | |||
|
|
|||
| if err := d.Set("numeric_id", strconv.Itoa(int(subnetwork.Id))); err != nil { | |||
There was a problem hiding this comment.
I am assuming this is pretty reliably an int64; not sure if this is the right place to do the conversion and / or if any more error checking is needed here, but it seems to work and tests pass.
There was a problem hiding this comment.
if we mark numericId as an integer we shouldn't need to do this extra conversion unless there's something that's preventing integer to be used over string
There was a problem hiding this comment.
See comment above; as best I could tell, other similar fields (in particular, the one in google_compute_network that this is based on) are implemented as strings… I could make it a number in terraform if you can confirm that’s the right thing to do, but would be nice to understand why so many of them are already implemented as strings.
There was a problem hiding this comment.
Google project number, org ID, etc. also seems to be implemented as strings vs. int
modular-magician
added
service/compute-vpc
and removed
awaiting-approval
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: 1061 Click here to see the affected service packages
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
Added
numeric_idtogoogle_compute_subnetworkresource and data source.Since Terraform uses
idinternally, follow the example ofgoogle_compute_network, and makenumeric_idcontain theidfield from the API.As mentioned in the issue below, it would be very nice if there were a simpler way to just say "map
idin the API tonumericIdhere", but guessing that's not possible?Part of hashicorp/terraform-provider-google#20223
Release Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.