Allow deletion of resource_bigquery_table when it still has associated resource tags #10568

wj-chen · 2024-04-30T22:49:14Z

Allow deletion of resource_bigquery_table when it still has associated resource tags.

Release Note Template for Downstream PRs (will be copied)

bigquery: added `allow_resource_tags_on_deletion` to `google_bigquery_table` to allow deletion of table when it still has associated resource tags (beta)

…d resource tags

github-actions · 2024-04-30T22:49:48Z

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

@shuyama1, 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 · 2024-04-30T23:00:39Z

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-beta provider: Diff ( 1 file changed, 10 deletions(-))

modular-magician · 2024-04-30T23:05:03Z

Tests analytics

Total tests: 115
Passed tests: 104
Skipped tests: 11
Affected tests: 0

Click here to see the affected service packages

bigquery

$\textcolor{green}{\textsf{All tests passed!}}$
View the build log

shuyama1 · 2024-05-02T17:31:56Z

mmv1/third_party/terraform/services/bigquery/resource_bigquery_table.go.erb

@@ -1832,18 +1832,6 @@ func resourceBigQueryTableDelete(d *schema.ResourceData, meta interface{}) error
 	if d.Get("deletion_protection").(bool) {
 		return fmt.Errorf("cannot destroy table %v without setting deletion_protection=false and running `terraform apply`", d.Id())
 	}
-	<% unless version == 'ga' -%>
-	if v, ok := d.GetOk("resource_tags"); ok {


I think this is a breaking change that should be avoided a minor release. This is Changing resource deletion behavior in https://googlecloudplatform.github.io/magic-modules/develop/breaking-changes/breaking-changes/#resource-level-breaking-changes.

We have a couple of options:

wait until the next major release, which is expected in a couple of months

introduce a virtual field to force delete the resource even when resource_tags are present and default it to false.

Please let me know your thoughts and if you have any questions. Thanks!

Would the combination of the points below make it eligible for a minor version release?

Feature was very newly added (released on 4/22)

Feature is in the beta provider

The new deletion behavior is more permissive instead of more restrictive

@wj-chen Thanks for the quick response.

Feature was very newly added (released on 4/22)

Feature is in the beta provider

The beta provider is also a stable surface that does not allow breaking changes outside of major releases. See go/terraform-launch-stages#google-beta

The new deletion behavior is more permissive instead of more restrictive

I'd still think it's breaking since it's changing the default deletion behavior and I'm a bit hesitant about changing it directly in a minor. However, if an urgent fix is needed in a minor, we could follow https://googlecloudplatform.github.io/magic-modules/develop/breaking-changes/make-a-breaking-change/#in-minor-releases - "this change can be made if the default behavior stays the same and new behavior can be enabled with a flag", such as adding a force_delete field, which is the option 2 I proposed in #10568 (comment). Please let me know what your thought on this approach and if it will work for your use case. Thanks!

I added a virtual field allow_resource_tags_on_deletion for this purpose and this PR is ready for review again.

Can you also describe the migration path from this to my original change? Would that be a candidate for the next major release?

Will discuss internally regarding the migration path for the next major release

Hi @shuyama1, I'd like to follow up on this discussion again since the submission window for the next major release is now open. We want to promote this field to the GA provider, and make the default value true in the GA provider from the get-go if possible.

Would it count as a breaking change if the promotion and default value update take place together? If not, we'll proceed with that since it's the most ideal. Updating the default value for the beta provider will be treated as a separate breaking change.

Otherwise, we would most likely promote first then make a breaking change to update the default value for both providers as a follow-up. This way the GA promotion won't be blocked by the next major release timeline.

@wj-chen Thank you for following up on this issue.

Our policy requires that the beta provider be a superset of the GA provider. This means the features and behaviors in GA should always be consistent with those in beta.

If we want to promote the feature to GA before changing the default in beta in 6.0.0 (as you correctly noted, this default value swap is a breaking change and must wait until the 6.0.0 major release), we need to use the current default value and update the default values for both GA and beta later in the 6.0.0 release.

Hope this answers your question and let me know if you have further questions.

In this case we'll proceed with promoting the feature to GA first, then making a breaking change to change the default value for the field for both GA and beta providers in the 6.0.0 release. Thank you for the guidance!

Perfect. Sounds good!

modular-magician · 2024-05-06T20:11:57Z

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, 3 insertions(+))
google-beta provider: Diff ( 1 file changed, 17 insertions(+), 5 deletions(-))

Missing test report

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

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

resource "google_bigquery_table" "primary" {
  allow_resource_tags_on_deletion = # value needed
}

modular-magician · 2024-05-06T20:15:23Z

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, 3 insertions(+))
google-beta provider: Diff ( 1 file changed, 17 insertions(+), 5 deletions(-))

Missing test report

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

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

resource "google_bigquery_table" "primary" {
  allow_resource_tags_on_deletion = # value needed
}

modular-magician · 2024-05-06T20:15:47Z

Tests analytics

Total tests: 115
Passed tests: 104
Skipped tests: 11
Affected tests: 0

Click here to see the affected service packages

bigquery

$\textcolor{green}{\textsf{All tests passed!}}$
View the build log

modular-magician · 2024-05-06T20:19:07Z

Tests analytics

Total tests: 115
Passed tests: 104
Skipped tests: 11
Affected tests: 0

Click here to see the affected service packages

bigquery

$\textcolor{green}{\textsf{All tests passed!}}$
View the build log

modular-magician · 2024-05-07T18:33:01Z

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, 3 insertions(+))
google-beta provider: Diff ( 2 files changed, 23 insertions(+), 8 deletions(-))

modular-magician · 2024-05-07T18:34:51Z

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, 3 insertions(+))
google-beta provider: Diff ( 2 files changed, 23 insertions(+), 8 deletions(-))

modular-magician · 2024-05-07T18:37:24Z

Tests analytics

Total tests: 115
Passed tests: 104
Skipped tests: 11
Affected tests: 0

Click here to see the affected service packages

bigquery

$\textcolor{green}{\textsf{All tests passed!}}$
View the build log

modular-magician · 2024-05-07T18:38:32Z

Tests analytics

Total tests: 115
Passed tests: 104
Skipped tests: 11
Affected tests: 0

Click here to see the affected service packages

bigquery

$\textcolor{green}{\textsf{All tests passed!}}$
View the build log

shuyama1 · 2024-05-07T20:43:16Z

mmv1/third_party/terraform/services/bigquery/resource_bigquery_table.go.erb

+	if err := d.Set("allow_resource_tags_on_deletion", false); err != nil {
+		return nil, fmt.Errorf("Error setting allow_resource_tags_on_deletion: %s", err)
+	}


I think we may also need to mark it as beta only.

modular-magician · 2024-05-07T21:22:45Z

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-beta provider: Diff ( 2 files changed, 23 insertions(+), 8 deletions(-))

modular-magician · 2024-05-07T21:27:01Z

Tests analytics

Total tests: 115
Passed tests: 104
Skipped tests: 11
Affected tests: 0

Click here to see the affected service packages

bigquery

$\textcolor{green}{\textsf{All tests passed!}}$
View the build log

shuyama1

Thanks!

…d resource tags (GoogleCloudPlatform#10568)

allow deletion of resource_bigquery_table when it still has associate…

e2f1507

…d resource tags

github-actions bot requested a review from shuyama1 April 30, 2024 22:49

modular-magician added the service/bigquery label Apr 30, 2024

modular-magician requested a review from sachinpro April 30, 2024 23:00

sachinpro approved these changes May 2, 2024

View reviewed changes

shuyama1 reviewed May 2, 2024

View reviewed changes

github-actions bot requested a review from shuyama1 May 6, 2024 20:01

add a virtual field allow_resource_tags_on_deletion

1409171

wj-chen force-pushed the table-tags branch from 36fb43a to 1409171 Compare May 6, 2024 20:04

modular-magician requested a review from sachinpro May 6, 2024 20:12

wj-chen added 2 commits May 7, 2024 18:21

add new field to unit test config

19a5db6

fix indentation

e30d3db

shuyama1 reviewed May 7, 2024

View reviewed changes

add version guard to import logic

49351c2

github-actions bot requested a review from shuyama1 May 7, 2024 21:13

shuyama1 approved these changes May 7, 2024

View reviewed changes

shuyama1 merged commit 3254c2c into GoogleCloudPlatform:main May 7, 2024
14 checks passed

BBBmau pushed a commit to BBBmau/magic-modules that referenced this pull request May 8, 2024

Allow deletion of resource_bigquery_table when it still has associate…

5ac67cc

…d resource tags (GoogleCloudPlatform#10568)

pawelJas pushed a commit to pawelJas/magic-modules that referenced this pull request May 16, 2024

Allow deletion of resource_bigquery_table when it still has associate…

f3a6cfe

…d resource tags (GoogleCloudPlatform#10568)

pengq-google pushed a commit to pengq-google/magic-modules that referenced this pull request May 21, 2024

Allow deletion of resource_bigquery_table when it still has associate…

c9ed94a

…d resource tags (GoogleCloudPlatform#10568)

Cheriit pushed a commit to Cheriit/magic-modules that referenced this pull request Jun 4, 2024

Allow deletion of resource_bigquery_table when it still has associate…

80c933d

…d resource tags (GoogleCloudPlatform#10568)

pcostell pushed a commit to pcostell/magic-modules that referenced this pull request Jul 16, 2024

Allow deletion of resource_bigquery_table when it still has associate…

f8091c0

…d resource tags (GoogleCloudPlatform#10568)

wj-chen mentioned this pull request Jul 22, 2024

Add deprecation message for allow_resource_tags_on_deletion in google_bigquery_table #11228

Merged

wj-chen mentioned this pull request Aug 8, 2024

Remove allow_resource_tags_on_deletion from google_bigquery_table #11393

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow deletion of resource_bigquery_table when it still has associated resource tags #10568

Allow deletion of resource_bigquery_table when it still has associated resource tags #10568

wj-chen commented Apr 30, 2024 •

edited by shuyama1

Loading

github-actions bot commented Apr 30, 2024

modular-magician commented Apr 30, 2024

modular-magician commented Apr 30, 2024

shuyama1 May 2, 2024

wj-chen May 2, 2024

shuyama1 May 6, 2024

wj-chen May 6, 2024

shuyama1 May 7, 2024

wj-chen Jul 1, 2024

shuyama1 Jul 2, 2024

wj-chen Jul 2, 2024

shuyama1 Jul 2, 2024

modular-magician commented May 6, 2024

modular-magician commented May 6, 2024

modular-magician commented May 6, 2024

modular-magician commented May 6, 2024

modular-magician commented May 7, 2024

modular-magician commented May 7, 2024

modular-magician commented May 7, 2024

modular-magician commented May 7, 2024

shuyama1 May 7, 2024

wj-chen May 7, 2024

modular-magician commented May 7, 2024

modular-magician commented May 7, 2024

shuyama1 left a comment

Allow deletion of resource_bigquery_table when it still has associated resource tags #10568

Allow deletion of resource_bigquery_table when it still has associated resource tags #10568

Conversation

wj-chen commented Apr 30, 2024 • edited by shuyama1 Loading

github-actions bot commented Apr 30, 2024

modular-magician commented Apr 30, 2024

Diff report

modular-magician commented Apr 30, 2024

Tests analytics

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

modular-magician commented May 6, 2024

Diff report

Missing test report

modular-magician commented May 6, 2024

Diff report

Missing test report

modular-magician commented May 6, 2024

Tests analytics

modular-magician commented May 6, 2024

Tests analytics

modular-magician commented May 7, 2024

Diff report

modular-magician commented May 7, 2024

Diff report

modular-magician commented May 7, 2024

Tests analytics

modular-magician commented May 7, 2024

Tests analytics

Choose a reason for hiding this comment

Choose a reason for hiding this comment

modular-magician commented May 7, 2024

Diff report

modular-magician commented May 7, 2024

Tests analytics

shuyama1 left a comment

Choose a reason for hiding this comment

wj-chen commented Apr 30, 2024 •

edited by shuyama1

Loading