Adding Dataplex Datascan as a resource #7911

shourya116 · 2023-05-10T06:43:02Z

Adding a new Dataplex resource: Datascan.

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 the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

`google_dataplex_datascan_*`

…to main

modular-magician · 2023-05-10T06:43:42Z

Hello! I am a robot who works on Magic Modules PRs.

I've detected that you're a community contributor. @megan07, a repository maintainer, has been assigned to assist you and help review your changes.

❓ First time contributing? Click here for more details

Your assigned reviewer will help review your code by:

Ensuring it's backwards compatible, covers common error cases, etc.
Summarizing the change into a user-facing changelog note.
Passes tests, either our "VCR" suite, a set of presubmit tests, or with manual test runs.

You can help make sure that review is quick by running local tests and ensuring they're passing in between each push you make to your PR's branch. Also, try to leave a comment with each push you make, as pushes generally don't generate emails.

If your reviewer doesn't get back to you within a week after your most recent change, please feel free to leave a comment on the issue asking them to take a look! In the absence of a dedicated review dashboard most maintainers manage their pending reviews through email, and those will sometimes get lost in their inbox.

megan07 · 2023-05-11T20:13:35Z

mmv1/products/dataplex/Datascan.yaml

+      The data source for DataScan.
+    properties:
+      - !ruby/object:Api::Type::String
+        name: 'entity'


can we make this required as well, please?

In this case, we plan to additional sources pretty soon.

And then the semantic will be "one-of" entity / new-property.

In this scenario, we shouldn't add required to entity right now, since dropping required later will be a backward-incompatible change, right?

megan07 · 2023-05-11T20:16:10Z

mmv1/products/dataplex/Datascan.yaml

+          Spec related to how often and when a scan should be triggered. If not specified, the default is OnDemand, which means the scan will not run until the user calls dataScans.run API.
+        properties:
+          - !ruby/object:Api::Type::NestedObject
+            name: 'onDemand'


Does onDemand have a nested cron string as well?

onDemand (and several RuleType fields below are currently an empty message with no properties;
The idea is to keep it as a message so that we can add fields to them later if needed.

User still needs to specify that they want onDemand instead of Scheduled though.

For empty messages in the API, should we just have NestedObject with no property in the YAML?

mmv1/products/dataplex/Datascan.yaml

megan07 · 2023-05-11T20:39:02Z

mmv1/products/dataplex/Datascan.yaml

+                        Whether each value needs to be strictly lesser than ('<') the maximum, or if equality is allowed.
+                        Only relevant if a maxValue has been defined. Default = false.
+                - !ruby/object:Api::Type::NestedObject
+                  name: 'nonNullExpectation'


we'll need the nested values list here too

i believe this is an empty message, so it should be fine?

megan07 · 2023-05-11T20:39:35Z

mmv1/products/dataplex/Datascan.yaml

+                      description: |
+                        A regular expression the column value is expected to match.
+                - !ruby/object:Api::Type::NestedObject
+                  name: 'uniquenessExpectation'


we'll want the nested object defined here as well

i believe this is an empty message, so it should be fine?

megan07 · 2023-05-11T20:40:54Z

mmv1/products/dataplex/Datascan.yaml

+                Whether the rule passed or failed.
+            - !ruby/object:Api::Type::String
+              name: 'evaluatedCount'
+              default_values: 0


if this is output, we shouldn't have a default here

megan07 · 2023-05-11T20:42:17Z

mmv1/products/dataplex/Datascan.yaml

+            description: |
+              The range denoted by values of an incremental field
+            properties:
+              - !ruby/object:Api::Type::string


should we capitalize string here and with its siblings?

megan07 · 2023-05-11T20:46:45Z

mmv1/templates/terraform/examples/dataplex_datascan_primary.tf.erb

+  display_name = "Test Datascan"
+
+  data = {
+	entity = "projects/<%= ctx[:test_env_vars]['project_name'] %>/locations/us-central1/lakes/amandeep-dev-lake/zones/rawzone/entities/austin_311"


should we not hardcode these values?

just to make sure I understand, we have to create a datascan against a source

are you suggesting we create a new data source (entity) and then reference that as the entity in the datascan
(or possibly reference a public entity)?

Here, it looks like there's a hard-coded entity, which the tests wont have reference to during automated tests, and hence will fail?

saurabh-net

Thanks for the comments @megan07! I left a few questions.

mmv1/products/dataplex/Datascan.yaml

saurabh-net · 2023-05-16T05:31:23Z

mmv1/products/dataplex/Datascan.yaml

+               name: 'threshold'
+               description: |
+                The minimum ratio of passing_rows / total_rows required to pass this rule, with a range of [0.0, 1.0]. 0 indicates default value (i.e. 1.0).
+               default_value: 0


(will double check this and update here)

The API behavior is that specifying 0, or not specifying a value at all, both cause the system to interpret the threshold as 1.0.

The API Read behavior however should return whatever the user specified --- i.e. 0 / unspecified.

saurabh-net · 2023-05-16T05:32:12Z

mmv1/products/dataplex/Datascan.yaml

+             - !ruby/object:Api::Type::NestedObject
+               name: 'nonNullExpectation'
+               description: |
+                 ColumnMap rule which evaluates whether each column value is null.


This is another example of an empty message in the API; We may add future fields / properties to this message later, but it's currently empty;

saurabh-net · 2023-05-16T05:38:18Z

mmv1/products/dataplex/Datascan.yaml

+                        Whether each value needs to be strictly lesser than ('<') the maximum, or if equality is allowed.
+                        Only relevant if a maxValue has been defined. Default = false.
+                - !ruby/object:Api::Type::NestedObject
+                  name: 'nonNullExpectation'


i believe this is an empty message, so it should be fine?

saurabh-net · 2023-05-16T05:38:21Z

mmv1/products/dataplex/Datascan.yaml

+                      description: |
+                        A regular expression the column value is expected to match.
+                - !ruby/object:Api::Type::NestedObject
+                  name: 'uniquenessExpectation'


i believe this is an empty message, so it should be fine?

saurabh-net · 2023-05-16T05:38:27Z

mmv1/products/dataplex/Datascan.yaml

+                Whether the rule passed or failed.
+            - !ruby/object:Api::Type::String
+              name: 'evaluatedCount'
+              default_values: 0


saurabh-net · 2023-05-16T05:38:33Z

mmv1/products/dataplex/Datascan.yaml

+            description: |
+              The range denoted by values of an incremental field
+            properties:
+              - !ruby/object:Api::Type::string


saurabh-net · 2023-05-16T05:41:43Z

mmv1/templates/terraform/examples/dataplex_datascan_primary.tf.erb

+  display_name = "Test Datascan"
+
+  data = {
+	entity = "projects/<%= ctx[:test_env_vars]['project_name'] %>/locations/us-central1/lakes/amandeep-dev-lake/zones/rawzone/entities/austin_311"


just to make sure I understand, we have to create a datascan against a source

are you suggesting we create a new data source (entity) and then reference that as the entity in the datascan
(or possibly reference a public entity)?

Here, it looks like there's a hard-coded entity, which the tests wont have reference to during automated tests, and hence will fail?

saurabh-net

Thanks for the comments @megan07! I left a few questions.

megan07 · 2023-05-23T16:35:12Z

mmv1/templates/terraform/examples/dataplex_datascan_primary.tf.erb

@@ -5,7 +5,7 @@ resource "google_dataplex_datascan" "<%= ctx[:primary_resource_id] %>" {
  display_name = "Test Datascan"

  data = {
-	entity = "projects/<%= ctx[:test_env_vars]['project_name'] %>/locations/us-central1/lakes/amandeep-dev-lake/zones/rawzone/entities/austin_311"
+	entity = "projects/dataplex-clouddq/locations/us-central1/lakes/amandeep-dev-lake/zones/rawzone/entities/austin_311"


i'm not sure we want to make this change, we want to keep the ctx[:test_env_vars]['project_name'] and probably do something similar with locations, lakes, zones, and entities. Does that make sense? I'm imagining

"projects/<%= ctx[:test_env_vars]['project_name'] %>/locations/ctx[:test_env_vars]['region']/lakes/ctx[:vars]['lake']/zones/ctx[:vars]['zone']/entities/ctx[:vars]['entity']"

where region is defined in the test_env_vars and lake, zone and entity are defined nearby, but in a similar vars block.

Let me know if you have questions around that! Thanks!

megan07 · 2023-05-24T20:12:42Z

@saurabh-net (edit: referenced the wrong user, oops!) sorry! i must have missed this comment yesterday in my review.

are you suggesting we create a new data source (entity) and then reference that as the entity in the datascan
(or possibly reference a public entity)?

yes, i supposed if this is a hard-coded public entity, that will suffice. i'm not super familiar with the resource to say which is better, but typically, if possible in terraform, we'd create the entity to reference and then add that here.

Here, it looks like there's a hard-coded entity, which the tests wont have reference to during automated tests, and hence will fail?

that's the concern, we want our tests to be able to access the entity referenced here

* Configuration for traffic director Mesh resource. * Add more tests for Mesh resource * Use new provider for test * Configuration for service binding. * Add hand written test for service binding. * Revert "Add hand written test for service binding." This reverts commit 367449a. * Update service binding yaml. * reduce timeouts. * Update mmv1/products/networkservices/ServiceBinding.yaml Co-authored-by: Sam Levenick <[email protected]> * Service binding update test. * Fix compile error. * Mark the Service Binding resource immutable. * Add configuration for server tls policy. * Remove extra files. * Add tests. * Revert "Remove extra files." This reverts commit 0791d44. * Update mmv1/third_party/terraform/tests/resource_network_security_server_tls_policy_test.go.erb Co-authored-by: Sarah French <[email protected]> * Address PR comments. * Server Tls Policy * Address PR comments. * Address PR comments. --------- Co-authored-by: Madhura Phadnis <[email protected]> Co-authored-by: Sam Levenick <[email protected]> Co-authored-by: Sarah French <[email protected]>

…oogleCloudPlatform#7858) * Add ICEBERG as a supported external data format for bigquery tables * updated descriptions to link api reference

…dPlatform#7895) Co-authored-by: James Edouard <[email protected]>

…oken docs link (GoogleCloudPlatform#7998)

…m#8002)

…oogleCloudPlatform#7996)

* Fix path for yaml * Use config * Add container to call workflow * Apply suggestions from code review Co-authored-by: Scott Suarez <[email protected]> --------- Co-authored-by: Scott Suarez <[email protected]>

* Move files to tpgiamresource package * Add docs * Use type alias

…ion_config. (GoogleCloudPlatform#7964)

* fixed accesscontextmanager spec requirements fixes hashicorp/terraform-provider-google#14612 * fixed google_access_context_manager_service_perimeter as well

Fixes hashicorp/terraform-provider-google#7745

…m#7993)

…ogleCloudPlatform#8007)

…reasing timeouts (GoogleCloudPlatform#8018)

megan07 · 2023-05-25T16:28:13Z

Hi @shourya116 , would you please rebase your changes with main? I think that will make the review a bit easier for me. Thanks!

saurabh-net · 2023-05-26T21:39:27Z

Hey @megan07 @shuyama1 @shourya116 ,
I'll be taking over this PR at #8010

We can move this PR to draft mode for now.

rileykarson · 2023-07-06T17:52:49Z

Superseded by #8010

shourya116 added 4 commits May 10, 2023 06:01

Adding datascan IAM example

b4be09d

Merge branch 'main' of https://github.com/shourya116/magic-modules in…

1f7b1af

…to main

empty commit

48f64ae

adding datascan full resource

0fd2094

modular-magician requested a review from megan07 May 10, 2023 06:43

modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label May 10, 2023

shourya116 changed the title ~~Full resource~~ Adding Dataplex Datascan as a resource May 10, 2023

megan07 suggested changes May 11, 2023

View reviewed changes

saurabh-net reviewed May 16, 2023

View reviewed changes

shourya116 added 2 commits May 17, 2023 16:28

Working on indentation

50de962

indentation has been fixed

b8305db

modular-magician requested a review from megan07 May 17, 2023 17:09

shourya116 added 2 commits May 22, 2023 17:28

set exclude resource to false'

82ffa86

updated examples

9c150ba

megan07 reviewed May 23, 2023

View reviewed changes

shourya116 added 6 commits May 25, 2023 06:30

adding datascan full resource

a80a9a8

Working on indentation

50ac4db

indentation has been fixed

fe1d6de

set exclude resource to false'

6e35f54

added Resources as a property for the DataResouce

33e6cfc

added Resource as a property for Dataresource

9b787da

modular-magician requested a review from megan07 May 25, 2023 06:52

maphad and others added 5 commits May 25, 2023 13:07

Skip sweeper in beta version (GoogleCloudPlatform#7907)

1a3f44e

Unskip compute instance test (GoogleCloudPlatform#7908)

4d06c6e

Add ICEBERG as a supported external data format for bigquery tables (G…

d14e9cb

…oogleCloudPlatform#7858) * Add ICEBERG as a supported external data format for bigquery tables * updated descriptions to link api reference

Add support for retriggering notifications in AlertPolicy (GoogleClou…

e90deca

…dPlatform#7895) Co-authored-by: James Edouard <[email protected]>

NickElliot and others added 22 commits May 25, 2023 13:07

Updating documentation to display an import_format functionality + br…

bb615e1

…oken docs link (GoogleCloudPlatform#7998)

Remove sample for Storage release (GoogleCloudPlatform#7944)

72f070c

Updated description of 'query_string_length' (GoogleCloudPlatform#7987)

08c1d3a

Skip Sweeper for firestore_document (GoogleCloudPlatform#7991)

235cdd7

Update hostname from required to default_from_api (GoogleCloudPlatfor…

b217aee

…m#8002)

Added nil guard to blue green settings in google_container_node_pool (G…

9ad94d6

…oogleCloudPlatform#7996)

Add looker service (GoogleCloudPlatform#7995)

9f973ab

Pre build validate (GoogleCloudPlatform#7961)

0fad3d3

* Fix path for yaml * Use config * Add container to call workflow * Apply suggestions from code review Co-authored-by: Scott Suarez <[email protected]> --------- Co-authored-by: Scott Suarez <[email protected]>

Move files to tpgiamresource package (GoogleCloudPlatform#7984)

24de88d

* Move files to tpgiamresource package * Add docs * Use type alias

Resolve yaml lint errors (GoogleCloudPlatform#8004)

b8727f9

Add support for idle_timeout/running_timeout in workstations_workstat…

ecc16fa

…ion_config. (GoogleCloudPlatform#7964)

service account sweeper (GoogleCloudPlatform#7997)

475e03a

Fixed accesscontextmanager spec requirements (GoogleCloudPlatform#7952)

bd1667b

* fixed accesscontextmanager spec requirements fixes hashicorp/terraform-provider-google#14612 * fixed google_access_context_manager_service_perimeter as well

Support container analysis note IAM (GoogleCloudPlatform#8006)

b2790ed

Fixes hashicorp/terraform-provider-google#7745

added regression testing for google_client_config (GoogleCloudPlatfor…

ca93417

…m#7993)

Add self to OnVacation list... out 5/29-6/19 (GoogleCloudPlatform#8005)

a97526c

Allow specifying regional L7 ILB in dns_record_set routing policy (Go…

a82063e

…ogleCloudPlatform#8007)

increase timeout for compute_security_policy (GoogleCloudPlatform#8020)

7e14faa

fix TestAccAlloydbInstance_updateDatabaseFlagInPrimaryInstance by inc…

c4438b9

…reasing timeouts (GoogleCloudPlatform#8018)

adding datascan full resource

e060a3c

Working on indentation

49a662f

set exclude resource to false'

6bd71f5

saurabh-net mentioned this pull request May 30, 2023

Adding Dataplex Datascan as a resource #8010

Merged

5 tasks

rileykarson closed this Jul 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding Dataplex Datascan as a resource #7911

Adding Dataplex Datascan as a resource #7911

shourya116 commented May 10, 2023

modular-magician commented May 10, 2023

megan07 May 11, 2023

saurabh-net May 16, 2023

megan07 May 11, 2023

saurabh-net May 16, 2023

megan07 May 11, 2023

saurabh-net May 16, 2023

megan07 May 11, 2023

saurabh-net May 16, 2023

megan07 May 11, 2023

saurabh-net May 16, 2023

megan07 May 11, 2023

saurabh-net May 16, 2023

megan07 May 11, 2023

saurabh-net May 16, 2023

saurabh-net left a comment

saurabh-net May 16, 2023

saurabh-net May 16, 2023

saurabh-net May 16, 2023

saurabh-net May 16, 2023

saurabh-net May 16, 2023

saurabh-net May 16, 2023

saurabh-net May 16, 2023

saurabh-net left a comment

megan07 May 23, 2023

megan07 commented May 24, 2023 •

edited

Loading

megan07 commented May 25, 2023

saurabh-net commented May 26, 2023

rileykarson commented Jul 6, 2023

Adding Dataplex Datascan as a resource #7911

Adding Dataplex Datascan as a resource #7911

Conversation

shourya116 commented May 10, 2023

modular-magician commented May 10, 2023

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

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

saurabh-net left a 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

saurabh-net left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

megan07 commented May 24, 2023 • edited Loading

megan07 commented May 25, 2023

saurabh-net commented May 26, 2023

rileykarson commented Jul 6, 2023

megan07 commented May 24, 2023 •

edited

Loading