Add a new resource TargetSite. #11669

jialei-chen · 2024-09-09T22:23:03Z

Add a new resource TargetSite to product Discovery Engine.

Release Note Template for Downstream PRs (will be copied)

`google_discovery_engine_target_site`

github-actions · 2024-09-09T22:23:44Z

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

@slevenick, 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-09-09T22:35: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 ( 5 files changed, 837 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 5 files changed, 837 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 89 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 119 insertions(+))

modular-magician · 2024-09-09T22:38:02Z

Tests analytics

Total tests: 11
Passed tests: 10
Skipped tests: 0
Affected tests: 1

Click here to see the affected service packages

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

Get to know how VCR tests work

modular-magician · 2024-09-09T22:38:21Z

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccDiscoveryEngineTargetSite_discoveryengineTargetsiteBasicExample[Error message] [Debug log]

$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$

View the build log or the debug log for each test

modular-magician · 2024-09-09T23:07:53Z

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 ( 5 files changed, 837 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 5 files changed, 837 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 89 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 119 insertions(+))

modular-magician · 2024-09-09T23:10:35Z

Tests analytics

Total tests: 11
Passed tests: 10
Skipped tests: 0
Affected tests: 1

Click here to see the affected service packages

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

Get to know how VCR tests work

modular-magician · 2024-09-09T23:10:55Z

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccDiscoveryEngineTargetSite_discoveryengineTargetsiteBasicExample[Error message] [Debug log]

$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$

View the build log or the debug log for each test

jialei-chen · 2024-09-09T23:35:00Z

[Solved]
Hi reviewer @slevenick , nice to e-meet you. I ran automatic tests locally and tried to fix the failure myself, but didn't solve it. Then I created this PR and hope to get some ideas of how to set system-generated resource id properly.

First, I tried to set self_link and delete_url to 'projects/{{project}}/locations/{{location}}/collections/default_collection/dataStores/{{data_store_id}}/siteSearchEngine/targetSites/{{target_site_id}}'. The resource id target_site_id is system-generated (via create API call) instead of user-specified, it seems it doesn't make sense to list target_site_id in section parameters.

Second, I changed to set self_link and delete_url to {{name}} referring to this example, because the output property name is exactly what I need for self_link and delete_url. But the acceptance test is failing.

I checked Debug log to get requests and corresponding responses:

Create dependency resource DataStore: succeeded.
Get the newly created dependency resource DataStore: succeeded.
Create target resource TargetSite: succeeded.
Get the newly created target resource TargetSite: failed (reason)
Delete the target resource TargetSite: should have this step but not because of failure in step 4.
Delete the dependency resource DataStore: succeeded.

…tion, then all 6 requests can return successfully.

modular-magician · 2024-09-10T21:57:30Z

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 ( 5 files changed, 855 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 5 files changed, 855 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 89 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 119 insertions(+))

modular-magician · 2024-09-10T21:59:55Z

Tests analytics

Total tests: 11
Passed tests: 10
Skipped tests: 0
Affected tests: 1

Click here to see the affected service packages

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

Get to know how VCR tests work

modular-magician · 2024-09-10T22:00:15Z

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccDiscoveryEngineTargetSite_discoveryengineTargetsiteBasicExample[Error message] [Debug log]

$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$

View the build log or the debug log for each test

…nly field with ignore_read.

…to fix id-missing error during ImportResourceState.

modular-magician · 2024-09-11T01:20:37Z

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 ( 5 files changed, 1077 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 5 files changed, 1077 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 89 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 119 insertions(+))

Missing test report

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

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

resource "google_discovery_engine_target_site" "primary" {
  failure_reason {
    quota_failure {
      total_required_quota = # value needed
    }
  }
  site_verification_info {
    site_verification_state = # value needed
    verify_time             = # value needed
  }
}

modular-magician · 2024-09-11T01:22:53Z

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 ( 5 files changed, 1077 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 5 files changed, 1077 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 89 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 119 insertions(+))

Missing test report

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

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

resource "google_discovery_engine_target_site" "primary" {
  failure_reason {
    quota_failure {
      total_required_quota = # value needed
    }
  }
  site_verification_info {
    site_verification_state = # value needed
    verify_time             = # value needed
  }
}

modular-magician · 2024-09-11T01:22:58Z

Tests analytics

Total tests: 11
Passed tests: 10
Skipped tests: 0
Affected tests: 1

Click here to see the affected service packages

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

Get to know how VCR tests work

modular-magician · 2024-09-11T01:23:19Z

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccDiscoveryEngineTargetSite_discoveryengineTargetsiteBasicExample[Error message] [Debug log]

$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$

View the build log or the debug log for each test

modular-magician · 2024-09-11T01:25:32Z

Tests analytics

Total tests: 11
Passed tests: 10
Skipped tests: 0
Affected tests: 1

Click here to see the affected service packages

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

Get to know how VCR tests work

modular-magician · 2024-09-11T01:25:54Z

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccDiscoveryEngineTargetSite_discoveryengineTargetsiteBasicExample[Error message] [Debug log]

$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$

View the build log or the debug log for each test

…_read_extra; Add update API.

modular-magician · 2024-09-11T04:18:31Z

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 ( 5 files changed, 1077 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 5 files changed, 1077 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 89 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 119 insertions(+))

Missing test report

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

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

resource "google_discovery_engine_target_site" "primary" {
  failure_reason {
    quota_failure {
      total_required_quota = # value needed
    }
  }
  site_verification_info {
    site_verification_state = # value needed
    verify_time             = # value needed
  }
}

modular-magician · 2024-09-11T04:21:06Z

Tests analytics

Total tests: 11
Passed tests: 10
Skipped tests: 0
Affected tests: 1

Click here to see the affected service packages

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

Get to know how VCR tests work

modular-magician · 2024-09-11T04:21:43Z

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccDiscoveryEngineTargetSite_discoveryengineTargetsiteBasicExample[Debug log]

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log or the debug log for each test

modular-magician · 2024-09-11T05:53:40Z

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 ( 6 files changed, 1200 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 6 files changed, 1200 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 89 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 119 insertions(+))

Missing test report

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

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

resource "google_discovery_engine_target_site" "primary" {
  failure_reason {
    quota_failure {
      total_required_quota = # value needed
    }
  }
  site_verification_info {
    site_verification_state = # value needed
    verify_time             = # value needed
  }
}

modular-magician · 2024-09-11T05:56:15Z

Tests analytics

Total tests: 12
Passed tests: 11
Skipped tests: 0
Affected tests: 1

Click here to see the affected service packages

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

Get to know how VCR tests work

modular-magician · 2024-09-11T05:57:08Z

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccDiscoveryEngineTargetSite_discoveryengineTargetsiteBasicExample_update[Debug log]

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log or the debug log for each test

slevenick · 2024-09-11T13:46:37Z

mmv1/products/discoveryengine/TargetSite.yaml

+      - 'project'
+custom_code: !ruby/object:Provider::Terraform::CustomCode
+  post_create: templates/terraform/post_create/set_computed_name.erb
+  post_update: templates/terraform/post_update/set_computed_name.erb


I don't think we need this post_update as well. It should work only after create because the name should never change

Does it work if you remove this?

Hi Sam, I tested locally, after removing this post_update, the update test couldn't pass because the resource name changed.

I ran cli TF_LOG=DEBUG make testacc TEST=./google/services/discoveryengine TESTARGS='-run=TestAccDiscoveryEngineTargetSite_discoveryengineTargetsiteBasicExample_update' > output.log to dump out debugging log, and got results: https://screenshot.googleplex.com/4izdznRcKyJGs9j

Double confirmed with the API owner, we plan not to support udpate operation because:

Update API is not used by UI (Pantheon);

Update API has not been fully tested, and now we expect the customer to do delete-then-recreate for an update.

Thus, for this newly supported resource TargetSite, we will support 4 APIs: get, list, create, delete
I will do the following things:

remove this post_update custom code and update test

mark this resource as immutable

…ode.

modular-magician · 2024-09-11T23:23:40Z

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 ( 5 files changed, 1077 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 5 files changed, 1077 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 89 insertions(+))
Open in Cloud Shell: Diff ( 8 files changed, 238 insertions(+))

Missing test report

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

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

resource "google_discovery_engine_target_site" "primary" {
  failure_reason {
    quota_failure {
      total_required_quota = # value needed
    }
  }
  site_verification_info {
    site_verification_state = # value needed
    verify_time             = # value needed
  }
}

modular-magician · 2024-09-11T23:26:11Z

Tests analytics

Total tests: 12
Passed tests: 10
Skipped tests: 0
Affected tests: 2

Click here to see the affected service packages

Action taken

Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

Get to know how VCR tests work

modular-magician · 2024-09-11T23:26:48Z

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccDiscoveryEngineTargetSite_discoveryengineTargetsiteAdvancedExample[Debug log]
TestAccDiscoveryEngineTargetSite_discoveryengineTargetsiteBasicExample[Debug log]

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log or the debug log for each test

slevenick

One small change required, I think just remove exactly_one_of

slevenick · 2024-09-12T19:57:57Z

mmv1/products/discoveryengine/TargetSite.yaml

+        name: 'quotaFailure'
+        description: |
+          Site verification state indicating the ownership and validity.
+        exactly_one_of:


This should just be required: true?

But the parent object is output: true, so exactly_one_of doesn't make sense here

Thanks, removed exactly_one_of .

modular-magician · 2024-09-12T21:20: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 provider: Diff ( 5 files changed, 1077 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 5 files changed, 1077 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 89 insertions(+))
Open in Cloud Shell: Diff ( 8 files changed, 238 insertions(+))

Missing test report

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

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

resource "google_discovery_engine_target_site" "primary" {
  failure_reason {
    quota_failure {
      total_required_quota = # value needed
    }
  }
  site_verification_info {
    site_verification_state = # value needed
    verify_time             = # value needed
  }
}

modular-magician · 2024-09-12T21:23:18Z

Tests analytics

Total tests: 12
Passed tests: 12
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log

modular-magician · 2024-09-12T21:41:26Z

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 ( 5 files changed, 1076 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 5 files changed, 1076 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 89 insertions(+))
Open in Cloud Shell: Diff ( 8 files changed, 238 insertions(+))

Missing test report

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

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

resource "google_discovery_engine_target_site" "primary" {
  failure_reason {
    quota_failure {
      total_required_quota = # value needed
    }
  }
  site_verification_info {
    site_verification_state = # value needed
    verify_time             = # value needed
  }
}

modular-magician · 2024-09-12T21:43:52Z

Tests analytics

Total tests: 12
Passed tests: 12
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log

Add a new resource TargetSite.

7ef1873

github-actions bot requested a review from slevenick September 9, 2024 22:23

modular-magician added the service/discoveryengine label Sep 9, 2024

Fix wrong indentation.

96d29d1

Add post_create custom code to save resource name in state after crea…

45a918f

…tion, then all 6 requests can return successfully.

jialei-chen added 4 commits September 11, 2024 00:14

Add all output (read-only) fields.

1b3b1ed

Fix non-empty plan error from provided_uri_pattern by marking input_o…

cea012c

…nly field with ignore_read.

Update import_format to match the accepted format; Add custom_import …

6265ad8

…to fix id-missing error during ImportResourceState.

Fix wrong indentation.

921e50c

Fix unequivalent ImportStateVerify attribute project by adding ignore…

d904fa7

…_read_extra; Add update API.

Add update test.

1d01f32

slevenick reviewed Sep 11, 2024

View reviewed changes

jialei-chen added 2 commits September 11, 2024 20:21

Remove update API, corresponding update test and post_update custom c…

398723a

…ode.

Add create test for advanced site search.

3142ec2

github-actions bot requested a review from slevenick September 11, 2024 23:12

jialei-chen mentioned this pull request Sep 12, 2024

Vertex AI search with a website datastore with target sites hashicorp/terraform-provider-google#17274

Closed

slevenick requested changes Sep 12, 2024

View reviewed changes

Merge branch 'GoogleCloudPlatform:main' into add-targetsite-resource

469cc67

github-actions bot requested a review from slevenick September 12, 2024 21:08

Remove exactly_one_of property for quota_failure field.

464d4fa

slevenick approved these changes Sep 13, 2024

View reviewed changes

slevenick merged commit 417f691 into GoogleCloudPlatform:main Sep 13, 2024
13 checks passed

iyabchen pushed a commit to iyabchen/magic-modules that referenced this pull request Sep 14, 2024

Add a new resource TargetSite. (GoogleCloudPlatform#11669)

1158554

abd-goog pushed a commit to abd-goog/magic-modules that referenced this pull request Sep 23, 2024

Add a new resource TargetSite. (GoogleCloudPlatform#11669)

a02b812

niharika-98 pushed a commit to niharika-98/magic-modules that referenced this pull request Oct 7, 2024

Add a new resource TargetSite. (GoogleCloudPlatform#11669)

5151a12

Philip-Jonany pushed a commit to Philip-Jonany/magic-modules that referenced this pull request Nov 4, 2024

Add a new resource TargetSite. (GoogleCloudPlatform#11669)

785f363

Add a new resource TargetSite. #11669

Add a new resource TargetSite. #11669

Conversation

jialei-chen commented Sep 9, 2024

github-actions bot commented Sep 9, 2024

modular-magician commented Sep 9, 2024

Diff report

modular-magician commented Sep 9, 2024

Tests analytics

Action taken

modular-magician commented Sep 9, 2024

modular-magician commented Sep 9, 2024

Diff report

modular-magician commented Sep 9, 2024

Tests analytics

Action taken

modular-magician commented Sep 9, 2024

jialei-chen commented Sep 9, 2024 • edited Loading

modular-magician commented Sep 10, 2024

Diff report

modular-magician commented Sep 10, 2024

Tests analytics

Action taken

modular-magician commented Sep 10, 2024

modular-magician commented Sep 11, 2024

Diff report

Missing test report

modular-magician commented Sep 11, 2024

Diff report

Missing test report

modular-magician commented Sep 11, 2024

Tests analytics

Action taken

modular-magician commented Sep 11, 2024

modular-magician commented Sep 11, 2024

Tests analytics

Action taken

modular-magician commented Sep 11, 2024

modular-magician commented Sep 11, 2024

Diff report

Missing test report

modular-magician commented Sep 11, 2024

Tests analytics

Action taken

modular-magician commented Sep 11, 2024

modular-magician commented Sep 11, 2024

Diff report

Missing test report

modular-magician commented Sep 11, 2024

Tests analytics

Action taken

modular-magician commented Sep 11, 2024

slevenick Sep 11, 2024

Choose a reason for hiding this comment

jialei-chen Sep 11, 2024

Choose a reason for hiding this comment

jialei-chen Sep 11, 2024

Choose a reason for hiding this comment

modular-magician commented Sep 11, 2024

Diff report

Missing test report

modular-magician commented Sep 11, 2024

Tests analytics

Action taken

modular-magician commented Sep 11, 2024

slevenick left a comment

Choose a reason for hiding this comment

slevenick Sep 12, 2024

Choose a reason for hiding this comment

slevenick Sep 12, 2024

Choose a reason for hiding this comment

jialei-chen Sep 12, 2024

Choose a reason for hiding this comment

modular-magician commented Sep 12, 2024

Diff report

Missing test report

modular-magician commented Sep 12, 2024

Tests analytics

modular-magician commented Sep 12, 2024

Diff report

jialei-chen commented Sep 9, 2024 •

edited

Loading