Skip to content
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

looker: required oauth_config in google_looker_instance #12214

Merged

Conversation

wyardley
Copy link
Contributor

@wyardley wyardley commented Oct 31, 2024

Make oauth_config required in google_looker_instance

Fixes hashicorp/terraform-provider-google#20140

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

looker: made `oauth_config` a required field in `google_looker_instance`, as creating this resource without that field always triggers an API error

Make `oauth_config` required in `google_looker_instance`
Fixes hashicorp/terraform-provider-google#20140
@github-actions github-actions bot requested a review from SarahFrench October 31, 2024 22:31
Copy link

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

@SarahFrench, 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 modular-magician added the awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests label Oct 31, 2024
Copy link

github-actions bot commented Nov 5, 2024

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

@modular-magician modular-magician added service/looker-core and removed awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests labels Nov 5, 2024
@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 ( 2 files changed, 35 insertions(+), 35 deletions(-))
google-beta provider: Diff ( 2 files changed, 35 insertions(+), 35 deletions(-))

Breaking Change(s) Detected

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

  • Field oauth_config changed from optional to required on google_looker_instance - 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.

Copy link
Contributor

@SarahFrench SarahFrench left a comment

Choose a reason for hiding this comment

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

Thanks for your PR!

Making a field swap from optional to required qualifies as a breaking change. We're ok to make a change like this in a minor release only if we follow this guidance, i.e. confirm that there's no valid way to provision a google_looker_instance resource without setting this field.

From looking at the linked issue it appears that the most minimal provisioning of google_looker_instance still requires oauth_config to be present.

I'll wait on the automation to complete on this PR, but I think based on the above we're ok to merge this PR into a minor release.

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 8
Passed tests: 8
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages
  • looker

🟢 All tests passed!

View the build log

@SarahFrench SarahFrench added the override-breaking-change Allows a potential breaking change to be merged label Nov 5, 2024
@SarahFrench
Copy link
Contributor

/gcbrun

@modular-magician modular-magician added awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests and removed awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests labels Nov 5, 2024
@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 ( 2 files changed, 35 insertions(+), 35 deletions(-))
google-beta provider: Diff ( 2 files changed, 35 insertions(+), 35 deletions(-))

Breaking Change(s) Detected

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

  • Field oauth_config changed from optional to required on google_looker_instance - 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.

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 8
Passed tests: 8
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages
  • looker

🟢 All tests passed!

View the build log

Copy link
Contributor

@SarahFrench SarahFrench left a comment

Choose a reason for hiding this comment

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

Approving given checks passing & determining this resource cannot be provisioned without this field present

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
override-breaking-change Allows a potential breaking change to be merged service/looker-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

google_looker_instance oauth_config is not marked as required
3 participants