-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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.
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
compute: added numeric_id
to google_compute_network
data source
#12339
compute: added numeric_id
to google_compute_network
data source
#12339
Conversation
Hello! I am a robot. Tests will require approval from a repository maintainer to run. @rileykarson, 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. |
@rileykarson This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 1 week. Please take a look! Use the label |
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 2 weeks. Please take a look! Use the label |
Add `numeric_id` to `google_compute_network` data source, matching the resource. Switch `numeric_id` to to be an integer internally within the `google_compute_network` resource for consistency with the API and with `google_compute_subnetwork`. Followup to GoogleCloudPlatform#12285 Part of hashicorp/terraform-provider-google#20223
c4d50f1
to
4175e36
Compare
Hi @rileykarson - does this look right now? |
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.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks right! Running tests. We could also add network_id
at-will, which could be a proper int, and deprecate numeric_id
for it.
Honestly I'd probably advocate for just keeping numeric_id
deprecated but around for a while as the cost to keep it is low and I'd prefer less churn, although that trades off some clarity for new users.
Tests analyticsTotal tests: 1069 Click here to see the affected service packages
🟢 All tests passed! View the build log |
Add
numeric_id
togoogle_compute_network
data source, matching the resource.SwitchScrapped -- see comment below; other discussion herenumeric_id
to to be an integer internallyI did some tests with thecompute_subnetwork
resource, and it doesn't seem like the switch to an integer breaks string interpolation, though it's possible this could be considered breaking in some other, more limited ways?Followup to #12285
Part of hashicorp/terraform-provider-google#20223
Release Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.