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

Remove dataprofileresult and dataqualityresult #8583

Conversation

saurabh-net
Copy link
Contributor

@saurabh-net saurabh-net commented Aug 8, 2023

Fix import issue for Datascan Dataprofile scans with existing jobs.
The TopN value message was incorrectly encoded as a map instead of as a array.

Removing the dataprofileresult and dataqualityresult message entirely, since it's irrelevant in the context of the Terraform lifecycle of a resource.

We'd like to keep the YAML spec as simple as possible.

Part of hashicorp/terraform-provider-google#15448

If this PR is for Terraform, I acknowledge that I have:

  • Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
  • Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
  • Generated Terraform providers, and ran make test and make lint in the generated providers to ensure it passes unit and linter tests.
  • Ran relevant acceptance tests using my own Google Cloud project and credentials (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
  • Read Write release notes before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

dataplex: removed `data_profile_result` and `data_quality_result` from `google_dataplex_scan`

@modular-magician
Copy link
Collaborator

Hello! I am a robot. It looks like you are a community contributor. @c2thorn, 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.

@saurabh-net
Copy link
Contributor Author

Had an earlier discussion on this PR at #8578

@modular-magician
Copy link
Collaborator

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

Breaking Change(s) Detected

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

  • Field data_profile_result.profile.fields.mode within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.name within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.distinct_ratio within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.double_profile.average within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.double_profile.max within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.double_profile.min within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.double_profile.quartiles within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.double_profile.standard_deviation within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.double_profile within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.integer_profile.average within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.integer_profile.max within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.integer_profile.min within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.integer_profile.quartiles within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.integer_profile.standard_deviation within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.integer_profile within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.null_ratio within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.string_profile.average_length within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.string_profile.max_length within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.string_profile.min_length within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.string_profile within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.top_n_values.count within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.top_n_values.value within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile.top_n_values within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.profile within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields.type within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile.fields within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.profile within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.row_count within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.scanned_data.incremental_field.end within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.scanned_data.incremental_field.field within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.scanned_data.incremental_field.start within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.scanned_data.incremental_field within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result.scanned_data within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_profile_result within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.dimensions.passed within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.dimensions within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.passed within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.row_count within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.evaluated_count within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.failing_rows_query within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.null_count within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.pass_ratio within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.passed_count within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.passed within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.column within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.dimension within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.ignore_null within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.non_null_expectation within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.range_expectation.max_value within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.range_expectation.min_value within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.range_expectation.strict_max_enabled within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.range_expectation.strict_min_enabled within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.range_expectation within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.regex_expectation.regex within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.regex_expectation within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.row_condition_expectation.sql_expression within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.row_condition_expectation within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.set_expectation.values within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.set_expectation within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.statistic_range_expectation.max_value within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.statistic_range_expectation.min_value within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.statistic_range_expectation.statistic within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.statistic_range_expectation.strict_max_enabled within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.statistic_range_expectation.strict_min_enabled within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.statistic_range_expectation within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.table_condition_expectation.sql_expression within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.table_condition_expectation within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.threshold within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule.uniqueness_expectation within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules.rule within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.rules within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.scanned_data.incremental_field.end within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.scanned_data.incremental_field.field within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.scanned_data.incremental_field.start within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.scanned_data.incremental_field within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result.scanned_data within resource google_dataplex_datascan was either removed or renamed - reference
  • Field data_quality_result within resource google_dataplex_datascan was either removed or renamed - 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.

Diff report

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

Terraform GA: Diff ( 2 files changed, 1644 deletions(-))
Terraform Beta: Diff ( 2 files changed, 1644 deletions(-))
TF Conversion: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

@saurabh-net
Copy link
Contributor Author

Additional context for this change --
Currently import Datascan fails for Profile type datascans with existing jobs created against them.
This is because of a misconfigured output-only field in the API YAML.

I originally filed a PR fixing that field, but later realized that the entire job-result field is not useful in the context of Terraform.
Hence we are looking to remove the JobResult field.

This field stores the result of the latest Job that run under a given Datascan definition. It exists primarily for UX convenience, so that users executing a GET on the Datascan, can quickly look up the status of the most recent run. This is especially handy in our UI.


The current broken import behavior is blocking a customer from adopting Datacan, since they have a workflow where they use Terraform import for some internal validation.

Current error:

panic: interface conversion: interface {} is []interface {}, not map[string]interface {}

goroutine 231 [running]:
github.com/hashicorp/terraform-provider-google/google/services/dataplex.flattenDataplexDatascanDataProfileResultProfileFieldsProfileTopNValues(...)
        github.com/hashicorp/terraform-provider-google/google/services/dataplex/resource_dataplex_datascan.go:2239
github.com/hashicorp/terraform-provider-google/google/services/dataplex.flattenDataplexDatascanDataProfileResultProfileFieldsProfile({0x10570ed60?, 0x1400199d5f0}, 0x104d1d0e2?, 0x7?)
        github.com/hashicorp/terraform-provider-google/google/services/dataplex/resource_dataplex_datascan.go:2192 +0x5c0
github.com/hashicorp/terraform-provider-google/google/services/dataplex.flattenDataplexDatascanDataProfileResultProfileFields({0x1054e01e0?, 0x1400217b200?}, 0x104d1afaa?, 0x6?)
        github.com/hashicorp/terraform-provider-google/google/services/dataplex/resource_dataplex_datascan.go:2161 +0x2a8
github.com/hashicorp/terraform-provider-google/google/services/dataplex.flattenDataplexDatascanDataProfileResultProfile({0x10570ed60?, 0x1400199d590}, 0x104d1d0e2?, 0x7?)
        github.com/hashicorp/terraform-provider-google/google/services/dataplex/resource_dataplex_datascan.go:2142 +0x98
github.com/hashicorp/terraform-provider-google/google/services/dataplex.flattenDataplexDatascanDataProfileResult({0x10570ed60?, 0x1400199d560}, 0x104d37527?, 0x11?)
        github.com/hashicorp/terraform-provider-google/google/services/dataplex/resource_dataplex_datascan.go:2123 +0x108
github.com/hashicorp/terraform-provider-google/google/services/dataplex.resourceDataplexDatascanRead(0x14001df5e80, {0x105b9e520?, 0x14001503200})
        github.com/hashicorp/terraform-provider-google/google/services/dataplex/resource_dataplex_datascan.go:1138 +0xf64
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).read(0x105be6e38?, {0x105be6e38?, 0x14001728e70?}, 0xd?, {0x105b9e520?, 0x14001503200?})
        github.com/hashicorp/terraform-plugin-sdk/[email protected]/helper/schema/resource.go:712 +0x134
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).RefreshWithoutUpgrade(0x140011d9ea0, {0x105be6e38, 0x14001728e70}, 0x140005d35f0, {0x105b9e520, 0x14001503200})
        github.com/hashicorp/terraform-plugin-sdk/[email protected]/helper/schema/resource.go:1015 +0x468
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).ReadResource(0x14000204198, {0x105be6e38?, 0x14001728d50?}, 0x140007595c0)
        github.com/hashicorp/terraform-plugin-sdk/[email protected]/helper/schema/grpc_provider.go:613 +0x400
github.com/hashicorp/terraform-plugin-mux/tf5muxserver.muxServer.ReadResource({0x14001407a70, 0x14001407ad0, {0x1400162ba00, 0x2, 0x2}, {0x0, 0x0, 0x0}, {0x0, 0x0, ...}, ...}, ...)
        github.com/hashicorp/[email protected]/tf5muxserver/mux_server_ReadResource.go:26 +0xdc
github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).ReadResource(0x140014a32c0, {0x105be6e38?, 0x14001728330?}, 0x14001d6ac00)
        github.com/hashicorp/[email protected]/tfprotov5/tf5server/server.go:748 +0x3e8
github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_ReadResource_Handler({0x105b22c80?, 0x140014a32c0}, {0x105be6e38, 0x14001728330}, 0x14000333880, 0x0)
        github.com/hashicorp/[email protected]/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:349 +0x170
google.golang.org/grpc.(*Server).processUnaryRPC(0x140017f94a0, {0x105beefa0, 0x14001070680}, 0x14001567680, 0x1400191ba70, 0x106f6fc70, 0x0)
        google.golang.org/[email protected]/server.go:1337 +0xc64
google.golang.org/grpc.(*Server).handleStream(0x140017f94a0, {0x105beefa0, 0x14001070680}, 0x14001567680, 0x0)
        google.golang.org/[email protected]/server.go:1714 +0x82c
google.golang.org/grpc.(*Server).serveStreams.func1.1()
        google.golang.org/[email protected]/server.go:959 +0x84
created by google.golang.org/grpc.(*Server).serveStreams.func1
        google.golang.org/[email protected]/server.go:957 +0x16c

Error: The terraform-provider-google_v4.72.0_x5 plugin crashed!

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 2921
Passed tests 2618
Skipped tests: 303
Affected tests: 0

$\textcolor{green}{\textsf{All tests passed in REPLAYING mode.}}$
View the build log

@c2thorn
Copy link
Member

c2thorn commented Aug 9, 2023

Hi @saurabh-net
I've read the context and it seems fine to remove. A couple of additional things from these steps: https://googlecloudplatform.github.io/magic-modules/develop/make-a-breaking-change/#removing-a-field

  1. Can you make another PR on the main branch to deprecate the fields and write an entry in the 5.0.0 upgrade guide explaining? The YAML lines needed would be Deprecated: <deprecation message
  2. Can you file an issue to track this and mark this PR as closing it?

@saurabh-net
Copy link
Contributor Author

Created a PR for deprecating these fields first here: #8593

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants