Skip to content

Conversation

@NandiniAgrawal15
Copy link
Contributor

compute: fixed endpoint type  in `google_compute_wire_group`

This doesn't constitute a breaking change, since without this, users were unable to add an acceptable endpoint value themselves.

@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 1 file changed, 21 insertions(+))
google-beta provider: Diff ( 3 files changed, 152 insertions(+), 8 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 61 insertions(+), 5 deletions(-))

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

  • Field endpoints.endpoint added as required on pre-existing resource google_compute_wire_group - reference
  • Field endpoints changed from TypeMap to TypeSet on google_compute_wire_group - reference

If you believe this detection to be incorrect please raise the concern with your reviewer.
If you intend to make this change you will need to wait for a major release window.
An override-breaking-change label can be added to allow merging.

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_compute_wire_group (3 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_compute_wire_group" "primary" {
  endpoints {
    endpoint = # value needed
    interconnects {
      interconnect      = # value needed
      interconnect_name = # value needed
      vlan_tags         = # value needed
    }
  }
}

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 1225
Passed tests: 1141
Skipped tests: 83
Affected tests: 1

Click here to see the affected service packages
  • compute
#### Action taken
Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccComputeWireGroup_update

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🟢 Tests passed during RECORDING mode:
TestAccComputeWireGroup_update [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.


🟢 All tests passed!

View the build log or the debug log for each test

@NandiniAgrawal15
Copy link
Contributor Author

NandiniAgrawal15 commented Jun 11, 2025

The previous change is yet to be released [https://github.com//pull/13944]. Is there some way we can exempt this from being labelled as breaking change ? If so, any directions would be helpful. Thanks !

@NandiniAgrawal15 NandiniAgrawal15 marked this pull request as ready for review June 11, 2025 17:11
@github-actions github-actions bot requested a review from hao-nan-li June 11, 2025 17:12
@github-actions
Copy link

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

Googlers: For automatic test runs see go/terraform-auto-test-runs.

@hao-nan-li, 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.

Copy link
Contributor

@hao-nan-li hao-nan-li left a comment

Choose a reason for hiding this comment

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

Is this change trying to fix a problem that has not been released yet? In that case I don't think this is a breaking-change.

Could you also tag this PR to the previously merged one? I just need to verify a bit and see if it's safe to merge this in.

@hao-nan-li
Copy link
Contributor

hao-nan-li commented Jun 11, 2025

@NickElliot FYI since you had context with #13944

@hao-nan-li hao-nan-li requested a review from NickElliot June 11, 2025 17:27
@NandiniAgrawal15
Copy link
Contributor Author

The Resource WireGroup was added via #13944. This change is yet to be released.The field Endpoint should have a different type.

@github-actions github-actions bot requested a review from hao-nan-li June 11, 2025 17:28
@hao-nan-li hao-nan-li added the override-breaking-change Allows a potential breaking change to be merged label Jun 12, 2025
hao-nan-li
hao-nan-li previously approved these changes Jun 12, 2025
@hao-nan-li hao-nan-li dismissed their stale review June 12, 2025 18:09

Decided to rollback the previous commit, so this PR is no longer necessary

@hao-nan-li hao-nan-li removed the override-breaking-change Allows a potential breaking change to be merged label Jun 12, 2025
@NickElliot NickElliot removed their request for review June 13, 2025 21:18
@github-actions
Copy link

@hao-nan-li This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

@github-actions
Copy link

@GoogleCloudPlatform/terraform-team @hao-nan-li This PR has been waiting for review for 1 week. Please take a look! Use the label disable-review-reminders to disable these notifications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants