Search Engine TF Provider #9822

Pratster95 · 2024-01-15T22:32:54Z

Release Note Template for Downstream PRs (will be copied)

`google_discovery_engine_search_engine`

modular-magician · 2024-01-15T22:34:21Z

Hello! I am a robot. It looks like you are a: ~~Community Contributor~~ Googler ~~Core Contributor~~. Tests will run automatically.

@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-01-15T22:43: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.

Terraform GA: Diff ( 6 files changed, 1127 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1127 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 168 insertions(+))
TF OiCS: Diff ( 4 files changed, 117 insertions(+))

modular-magician · 2024-01-15T22:45:59Z

Tests analytics

Total tests: 4
Passed tests 2
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-01-15T22:46:14Z

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

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

modular-magician · 2024-01-16T03:14: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.

Terraform GA: Diff ( 6 files changed, 1127 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1127 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 168 insertions(+))
TF OiCS: Diff ( 4 files changed, 117 insertions(+))

modular-magician · 2024-01-16T03:16:32Z

Tests analytics

Total tests: 4
Passed tests 2
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-01-16T03:16:48Z

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

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

Pratster95 · 2024-01-16T04:12:22Z

I ran make fmt in my local and don't see any gofmt errors after that but after pushing the code I am seeing gofmt error for terraform-provider-google-beta-build-and-unit-tests and terraform-provider-google-build-and-unit-tests

shuyama1 · 2024-01-16T16:47:14Z

Can you try gofmt -w mmv1/third_party/terraform/services/discoveryengine/resource_discovery_engine_search_engine_test.go, if you're running it in the upstream repo magic-modules

modular-magician · 2024-01-16T18:45: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.

Terraform GA: Diff ( 6 files changed, 1122 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1122 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 168 insertions(+))
TF OiCS: Diff ( 4 files changed, 117 insertions(+))

modular-magician · 2024-01-16T18:47:42Z

Tests analytics

Total tests: 4
Passed tests 2
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-01-16T18:47:56Z

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

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

Pratster95 · 2024-01-16T18:53:27Z

Thank You @shuyama1 I was able to fix that issue. For the VCR test I would have to merge the test of datastore and app in one test file and refer to the datastore created using the Datastore TF Provider. I used a Datastore ID from my test project and the acceptance tests passed locally

modular-magician · 2024-01-16T20:03:52Z

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.

Terraform GA: Diff ( 6 files changed, 1140 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1140 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 168 insertions(+))
TF OiCS: Diff ( 4 files changed, 117 insertions(+))

modular-magician · 2024-01-16T20:06:17Z

Tests analytics

Total tests: 4
Passed tests 2
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-01-16T20:07:27Z

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

Rerun these tests in REPLAYING mode to catch issues

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

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

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

modular-magician · 2024-01-16T21:12: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.

Terraform GA: Diff ( 6 files changed, 1158 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1158 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 168 insertions(+))
TF OiCS: Diff ( 4 files changed, 126 insertions(+))

modular-magician · 2024-01-16T21:15:26Z

Tests analytics

Total tests: 4
Passed tests 3
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-01-16T21:16:16Z

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

Rerun these tests in REPLAYING mode to catch issues

$\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

mmv1/products/discoveryengine/SearchEngine.yaml

Co-authored-by: Ray <[email protected]>

modular-magician · 2024-01-17T17:13:41Z

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.

Terraform GA: Diff ( 6 files changed, 1158 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1158 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 168 insertions(+))
TF OiCS: Diff ( 4 files changed, 126 insertions(+))

modular-magician · 2024-01-17T17:16:09Z

Tests analytics

Total tests: 4
Passed tests 4
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

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

Pratster95 · 2024-01-17T17:25:37Z

@5fff Could you verify your CLA. It is causing CLA test to fail

shuyama1

Would you mind also adding create + update test coverage on fields common_config and search_engine_config detected in the Missing test report #9822 (comment)? Thank you!

shuyama1 · 2024-01-29T23:50:57Z

mmv1/products/discoveryengine/SearchEngine.yaml

+        values:
+          - :SEARCH_TIER_STANDARD
+          - :SEARCH_TIER_ENTERPRISE
+        default_from_api: true


Suggested change

default_from_api: true

default_value: SEARCH_TIER_STANDARD

Then we may want to set default value if the default is fixed

mmv1/products/discoveryengine/SearchEngine.yaml

modular-magician · 2024-01-30T04:10: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.

Terraform GA: Diff ( 6 files changed, 1192 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1192 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 164 insertions(+))
TF OiCS: Diff ( 4 files changed, 118 insertions(+))

modular-magician · 2024-01-30T04:13:20Z

Tests analytics

Total tests: 4
Passed tests 3
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-01-30T04:14:33Z

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

Rerun these tests in REPLAYING mode to catch issues

$\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-01-30T21:02:55Z

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.

Terraform GA: Diff ( 6 files changed, 1192 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1192 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 164 insertions(+))
TF OiCS: Diff ( 4 files changed, 118 insertions(+))

modular-magician · 2024-01-30T21:05:30Z

Tests analytics

Total tests: 4
Passed tests 4
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

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

mmv1/products/discoveryengine/SearchEngine.yaml

chenlei1216 · 2024-01-30T21:30:16Z

mmv1/products/discoveryengine/SearchEngine.yaml

+    immutable: false
+    properties:
+      - !ruby/object:Api::Type::Enum
+        name: 'searchTier'


Thanks! SGTM to make it required as a start to ask customers always set the tier explicitly. We can resolve the comment

modular-magician · 2024-01-31T01:03: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.

Terraform GA: Diff ( 6 files changed, 1199 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1199 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 164 insertions(+))
TF OiCS: Diff ( 4 files changed, 122 insertions(+))

modular-magician · 2024-01-31T01:06:42Z

Tests analytics

Total tests: 6
Passed tests 5
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-01-31T01:07:35Z

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

Rerun these tests in REPLAYING mode to catch issues

$\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

shuyama1

Mostly LGTM! Thanks! Some small comments only

shuyama1 · 2024-01-31T01:01:43Z

mmv1/products/discoveryengine/SearchEngine.yaml

+
+parameters:
+  - !ruby/object:Api::Type::String
+    name: 'engine_id'


Suggested change

name: 'engine_id'

name: 'engineId'

Sorry for missing it. Should not affect generated code downstream though

shuyama1 · 2024-01-31T01:01:46Z

mmv1/products/discoveryengine/SearchEngine.yaml

+    immutable: true
+    url_param_only: true
+  - !ruby/object:Api::Type::String
+    name: 'collection_id'


Suggested change

name: 'collection_id'

name: 'collectionId'

shuyama1 · 2024-01-31T01:10:36Z

mmv1/products/discoveryengine/SearchEngine.yaml

+      characters.
+    output: true
+  - !ruby/object:Api::Type::Enum
+    name: 'industryVertical'


We may need to set default value for this field since the default is fixed. Does the API return the default value for this field if it's not explicitly set? I don't see a test config without this field, so I'm not sure the API behavior. We may consider removing all optional fields from the basic example to ensure we have required fields set correctly and we handle fields that have default values returned from the API.

SG. Let's add "default_value: :GENERIC". This is the default server behavior.

I tested the provider in a TF directory without the industry_vertical field and it takes the default as generic. I will also add its default_value under properties

Also for the Search and Chat Engine the industry_field should be derived from datastore's industry_vertical so it has a dependency that's why in the example also I refer it to the Datastore's industry_vertical while creating the Search Engine resource,instead of leaving it out.
@shuyama1 @chenlei1216 Let me know your thoughts

shuyama1 · 2024-01-31T01:13:45Z

mmv1/products/discoveryengine/SearchEngine.yaml

+    name: 'searchEngineConfig'
+    description: |
+      Configurations for a Search Engine.
+    # default_from_api: true


Suggested change

# default_from_api: true

modular-magician · 2024-01-31T03:41:11Z

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.

Terraform GA: Diff ( 6 files changed, 1195 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1195 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 164 insertions(+))
TF OiCS: Diff ( 4 files changed, 119 insertions(+))

modular-magician · 2024-01-31T03:43:43Z

Tests analytics

Total tests: 6
Passed tests 5
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-01-31T03:44:34Z

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

Rerun these tests in REPLAYING mode to catch issues

$\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

shuyama1

Thank you!

* Search Engine TF Provider * Discovery Engine Search Engine * Formatted Test File * Updated Test File * Added Examples to SearchEngine Config and added DS to basic test example * Update mmv1/products/discoveryengine/SearchEngine.yaml Co-authored-by: Ray <[email protected]> * Updated SearchEngine.yaml file * Updated SearchEngine.yaml * Updated Search TF Files as per PR Review * Updated files as per PR Review * Set searchEngineConfig as required field in SearchEngine.yaml * Removed default_from_api from SearchEngineConfig * Added Changes as per PR review --------- Co-authored-by: Ray <[email protected]>

Search Engine TF Provider

e2cf440

modular-magician requested a review from shuyama1 January 15, 2024 22:34

Discovery Engine Search Engine

0065b63

Formatted Test File

a524791

Updated Test File

8731af8

Added Examples to SearchEngine Config and added DS to basic test example

cfa3e89

5fff reviewed Jan 17, 2024

View reviewed changes

mmv1/products/discoveryengine/SearchEngine.yaml Outdated Show resolved Hide resolved

Update mmv1/products/discoveryengine/SearchEngine.yaml

ae04b0d

Co-authored-by: Ray <[email protected]>

shuyama1 reviewed Jan 30, 2024

View reviewed changes

Updated files as per PR Review

777642c

modular-magician requested a review from shuyama1 January 30, 2024 04:01

Set searchEngineConfig as required field in SearchEngine.yaml

4543913

chenlei1216 approved these changes Jan 30, 2024

View reviewed changes

Removed default_from_api from SearchEngineConfig

54f3751

shuyama1 reviewed Jan 31, 2024

View reviewed changes

Added Changes as per PR review

2ba3dd2

modular-magician requested a review from shuyama1 January 31, 2024 03:31

shuyama1 approved these changes Jan 31, 2024

View reviewed changes

shuyama1 merged commit 69eb35b into GoogleCloudPlatform:main Jan 31, 2024
12 checks passed

Search Engine TF Provider #9822

Search Engine TF Provider #9822

Conversation

Pratster95 commented Jan 15, 2024 • edited by shuyama1 Loading

modular-magician commented Jan 15, 2024

modular-magician commented Jan 15, 2024

Diff report

modular-magician commented Jan 15, 2024

Tests analytics

Action taken

modular-magician commented Jan 15, 2024

modular-magician commented Jan 16, 2024

Diff report

modular-magician commented Jan 16, 2024

Tests analytics

Action taken

modular-magician commented Jan 16, 2024

Pratster95 commented Jan 16, 2024 • edited Loading

shuyama1 commented Jan 16, 2024

modular-magician commented Jan 16, 2024

Diff report

modular-magician commented Jan 16, 2024

Tests analytics

Action taken

modular-magician commented Jan 16, 2024

Pratster95 commented Jan 16, 2024

modular-magician commented Jan 16, 2024

Diff report

modular-magician commented Jan 16, 2024

Tests analytics

Action taken

modular-magician commented Jan 16, 2024

Rerun these tests in REPLAYING mode to catch issues

modular-magician commented Jan 16, 2024

Diff report

modular-magician commented Jan 16, 2024

Tests analytics

Action taken

modular-magician commented Jan 16, 2024

Rerun these tests in REPLAYING mode to catch issues

modular-magician commented Jan 17, 2024

Diff report

modular-magician commented Jan 17, 2024

Tests analytics

Pratster95 commented Jan 17, 2024

shuyama1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

modular-magician commented Jan 30, 2024

Diff report

modular-magician commented Jan 30, 2024

Tests analytics

Action taken

modular-magician commented Jan 30, 2024

Rerun these tests in REPLAYING mode to catch issues

modular-magician commented Jan 30, 2024

Diff report

modular-magician commented Jan 30, 2024

Tests analytics

Choose a reason for hiding this comment

modular-magician commented Jan 31, 2024

Diff report

modular-magician commented Jan 31, 2024

Tests analytics

Action taken

modular-magician commented Jan 31, 2024

Rerun these tests in REPLAYING mode to catch issues

shuyama1 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

Pratster95 Jan 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

modular-magician commented Jan 31, 2024

Diff report

modular-magician commented Jan 31, 2024

Pratster95 commented Jan 15, 2024 •

edited by shuyama1

Loading

Pratster95 commented Jan 16, 2024 •

edited

Loading

Pratster95 Jan 31, 2024 •

edited

Loading