feat: make it possible to deifine a feature in Vertex AI FeatureStore #6568

shotarok · 2022-09-18T22:46:37Z

Part of hashicorp/terraform-provider-google#9298
Part of hashicorp/terraform-provider-google#12307

Add vertex_ai_featurestore_entitytype_feature to enable the google-beta provider to define a feature in Vertex AI FeatureStore. #6565 is a PR to promote the other featurestore resources to GA from beta. If we merge PR#6565 first, it'd be nice to update this PR to use GA instead of beta too.

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).
Generated Terraform, and ran make test and make lint to ensure it passes unit and linter tests.
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).
Ran relevant acceptance tests (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)

vertex_ai_featurestore_entitytype_feature

modular-magician · 2022-09-18T22:47:38Z

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

I've detected that you're a community contributor. @ScottSuarez, 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.

shotarok · 2022-09-18T22:53:10Z

mmv1/products/vertexai/api.yaml

+    base_url: 'projects/{{project}}/locations/{{region}}/featurestores/{{featurestore}}/entityTypes/{{entityType}}/features'
+    create_url: 'projects/{{project}}/locations/{{region}}/featurestores/{{featurestore}}/entityTypes/{{entityType}}/features?featureId={{name}}'
+    self_link: 'projects/{{project}}/locations/{{region}}/featurestores/{{featurestore}}/entityTypes/{{entityType}}/features/{{name}}'


[MEMO] I intentionally used the literal paths instead of include_project: true because I had the following error when running the google-beta provider unit tests.

# github.com/hashicorp/terraform-provider-google-beta/google-beta [github.com/hashicorp/terraform-provider-google-beta/google-beta.test] google-beta/resource_vertex_ai_featurestore_entitytype_feature.go:28:5: project redeclared in this block google-beta/resource_vertex_ai_featurestore_entitytype.go:28:5: other declaration of project FAIL github.com/hashicorp/terraform-provider-google-beta/google-beta [build failed] FAIL

That's because the project variable is defined as a global package variable. Therefore, include_project can't be used twice currently.

magic-modules/mmv1/templates/terraform/resource.erb

Lines 66 to 70 in be60743

<% if object.async&.is_a? Api::OpAsync -%>

<% if object.async.include_project -%>

var project string

<% end -%>

<% end -%>

I've fixed the issue in d41b2d9.

modular-magician · 2022-09-18T22:58:00Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 147 insertions(+))
Terraform Beta: Diff ( 4 files changed, 724 insertions(+), 2 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-09-19T00:03:48Z

Tests analytics

Total tests: 2175
Passed tests 1934
Skipped tests: 238
Failed tests: 3

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFirebaserulesRelease_BasicRelease|TestAccComputeInstance_soleTenantNodeAffinities|TestAccVertexAIFeaturestoreEntitytypeFeature_vertexAiFeaturestoreEntitytypeFeatureExample

modular-magician · 2022-09-19T00:05:27Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]

Tests failed during RECORDING mode:
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]
TestAccVertexAIFeaturestoreEntitytypeFeature_vertexAiFeaturestoreEntitytypeFeatureExample[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

google_vertex_ai_featurestore_entitytype has already used entitytype instead of entityType. google_vertex_ai_featurestore_entitytype_feature also should use entitytype instead of entity_type as an attribute for the compatibility.

modular-magician · 2022-09-19T01:17:58Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 147 insertions(+))
Terraform Beta: Diff ( 4 files changed, 724 insertions(+), 2 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-09-19T02:19:25Z

Tests analytics

Total tests: 2175
Passed tests 1935
Skipped tests: 238
Failed tests: 2

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccComputeInstance_soleTenantNodeAffinities|TestAccVertexAIFeaturestoreEntitytypeFeature_vertexAiFeaturestoreEntitytypeFeatureExample

modular-magician · 2022-09-19T02:21:11Z

Tests failed during RECORDING mode:
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]
TestAccVertexAIFeaturestoreEntitytypeFeature_vertexAiFeaturestoreEntitytypeFeatureExample[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

modular-magician · 2022-09-19T04:44:16Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 233 files changed, 784 insertions(+))
Terraform Beta: Diff ( 264 files changed, 1420 insertions(+), 4 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-09-19T05:50:50Z

Tests analytics

Total tests: 2175
Passed tests 1932
Skipped tests: 238
Failed tests: 5

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccVertexAIFeaturestoreEntitytypeFeature_vertexAiFeaturestoreEntitytypeFeatureExample|TestAccFirebaserulesRelease_BasicRelease|TestAccComputeInstance_soleTenantNodeAffinities|TestAccComputeForwardingRule_update|TestAccCGCSnippet_eventarcWorkflowsExample

modular-magician · 2022-09-19T05:52:38Z

Tests passed during RECORDING mode:
TestAccVertexAIFeaturestoreEntitytypeFeature_vertexAiFeaturestoreEntitytypeFeatureExample[Debug log]
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccComputeForwardingRule_update[Debug log]

Tests failed during RECORDING mode:
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]
TestAccCGCSnippet_eventarcWorkflowsExample[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

shotarok · 2022-09-26T07:46:13Z

Hi @ScottSuarez, could you please review this PR?

ScottSuarez · 2022-09-27T22:17:28Z

Hey @shotarok, sure ! Thanks for bringing this back into my awareness. I'll take a look

ScottSuarez · 2022-09-27T23:47:37Z

There is a side effect of whitespace removal on your change being against a lot of files. It's making if difficult to tell the diff for your changes resource.erb. Could you either: revert the changes to the templating file, make this templating change in a new pull request, or change it so the whitespace difference is gone and I can verify there is no practical difference to the generated code.

Terraform Beta: Diff ( 264 files changed, 1420 insertions(+), 4 deletions(-))

ScottSuarez

Comments in turn

mmv1/products/vertexai/api.yaml

shotarok · 2022-09-28T00:08:02Z

Thank you for your comments! I didn't realize the large number of diffs. I'll update the template file to minimize the diffs. Once I update the code, I'll get back to you.

modular-magician · 2022-09-28T11:02:09Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 182 files changed, 320 insertions(+))
Terraform Beta: Diff ( 211 files changed, 895 insertions(+), 4 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-09-28T11:54:18Z

Tests analytics

Total tests: 2184
Passed tests 1942
Skipped tests: 239
Failed tests: 3

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFirebaserulesRelease_BasicRelease|TestAccComputeInstance_soleTenantNodeAffinities|TestAccCGCSnippet_eventarcWorkflowsExample

modular-magician · 2022-09-28T12:00:05Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]

Tests failed during RECORDING mode:
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]
TestAccCGCSnippet_eventarcWorkflowsExample[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

mmv1/templates/terraform/resource.erb

modular-magician · 2022-09-28T12:30:26Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 139 insertions(+))
Terraform Beta: Diff ( 5 files changed, 687 insertions(+), 4 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-09-28T13:23:06Z

Tests analytics

Total tests: 2184
Passed tests 1942
Skipped tests: 239
Failed tests: 3

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFirebaserulesRelease_BasicRelease|TestAccComputeInstance_soleTenantNodeAffinities|TestAccCGCSnippet_eventarcWorkflowsExample

modular-magician · 2022-09-28T13:30:17Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]

Tests failed during RECORDING mode:
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]
TestAccCGCSnippet_eventarcWorkflowsExample[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

shotarok · 2022-09-28T18:14:45Z

Hi @ScottSuarez, I have updated code baed on your feedback. When you have time, could you review the changes? Thanks

ScottSuarez · 2022-09-29T18:54:14Z

Thanks for the work !

feat: make it possible to deifine a feature in Vertex AI FeatureStore

ae817af

modular-magician requested a review from ScottSuarez September 18, 2022 22:47

shotarok commented Sep 18, 2022

View reviewed changes

shotarok marked this pull request as draft September 19, 2022 03:10

shotarok added 2 commits September 18, 2022 21:31

fix: make it possible to use include_project more than once

d41b2d9

fix: fix the urls for the vertex ai features

a6cf1d8

shotarok marked this pull request as ready for review September 19, 2022 04:35

ScottSuarez reviewed Sep 28, 2022

View reviewed changes

mmv1/products/vertexai/api.yaml Outdated Show resolved Hide resolved

fix: minimize the unrelated diffs by white lines

f750be6

modular-magician requested a review from ScottSuarez September 28, 2022 10:45

refactor: use string instead of enum for maintainability

34219f2

shotarok commented Sep 28, 2022

View reviewed changes

mmv1/templates/terraform/resource.erb Outdated Show resolved Hide resolved

fix: minimize the unrelated diffs by a white line (take 2)

bbd4acc

ScottSuarez approved these changes Sep 29, 2022

View reviewed changes

ScottSuarez merged commit 05d5f2e into GoogleCloudPlatform:main Sep 29, 2022

This was referenced Sep 29, 2022

feat: make it possible to deifine a feature in Vertex AI FeatureStore hashicorp/terraform-provider-google-beta#4736

Merged

feat: make it possible to deifine a feature in Vertex AI FeatureStore hashicorp/terraform-provider-google#12684

Merged

shotarok deleted the vertex-ai-featurestore-entity-feature branch September 29, 2022 19:43

shotarok mentioned this pull request Sep 30, 2022

Promote Vertex AI FeatureStore resources (GA only) #6565

Merged

5 tasks

shotarok mentioned this pull request Oct 16, 2022

Add support google_vertex_ai_featurestore_entity_feature hashicorp/terraform-provider-google#12307

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: make it possible to deifine a feature in Vertex AI FeatureStore #6568

feat: make it possible to deifine a feature in Vertex AI FeatureStore #6568

shotarok commented Sep 18, 2022 •

edited

Loading

modular-magician commented Sep 18, 2022

shotarok Sep 18, 2022 •

edited

Loading

shotarok Sep 19, 2022

modular-magician commented Sep 18, 2022

modular-magician commented Sep 19, 2022

modular-magician commented Sep 19, 2022

modular-magician commented Sep 19, 2022

modular-magician commented Sep 19, 2022

modular-magician commented Sep 19, 2022

modular-magician commented Sep 19, 2022

modular-magician commented Sep 19, 2022

modular-magician commented Sep 19, 2022

shotarok commented Sep 26, 2022

ScottSuarez commented Sep 27, 2022

ScottSuarez commented Sep 27, 2022 •

edited

Loading

ScottSuarez left a comment

shotarok commented Sep 28, 2022

modular-magician commented Sep 28, 2022

modular-magician commented Sep 28, 2022

modular-magician commented Sep 28, 2022

modular-magician commented Sep 28, 2022

modular-magician commented Sep 28, 2022

modular-magician commented Sep 28, 2022

shotarok commented Sep 28, 2022 •

edited

Loading

ScottSuarez commented Sep 29, 2022

	<% if object.async&.is_a? Api::OpAsync -%>
	<% if object.async.include_project -%>
	var project string
	<% end -%>
	<% end -%>

feat: make it possible to deifine a feature in Vertex AI FeatureStore #6568

feat: make it possible to deifine a feature in Vertex AI FeatureStore #6568

Conversation

shotarok commented Sep 18, 2022 • edited Loading

modular-magician commented Sep 18, 2022

shotarok Sep 18, 2022 • edited Loading

Choose a reason for hiding this comment

shotarok Sep 19, 2022

Choose a reason for hiding this comment

modular-magician commented Sep 18, 2022

Diff report:

modular-magician commented Sep 19, 2022

Tests analytics

Action taken

modular-magician commented Sep 19, 2022

modular-magician commented Sep 19, 2022

Diff report:

modular-magician commented Sep 19, 2022

Tests analytics

Action taken

modular-magician commented Sep 19, 2022

modular-magician commented Sep 19, 2022

Diff report:

modular-magician commented Sep 19, 2022

Tests analytics

Action taken

modular-magician commented Sep 19, 2022

shotarok commented Sep 26, 2022

ScottSuarez commented Sep 27, 2022

ScottSuarez commented Sep 27, 2022 • edited Loading

ScottSuarez left a comment

Choose a reason for hiding this comment

shotarok commented Sep 28, 2022

modular-magician commented Sep 28, 2022

Diff report:

modular-magician commented Sep 28, 2022

Tests analytics

Action taken

modular-magician commented Sep 28, 2022

modular-magician commented Sep 28, 2022

Diff report:

modular-magician commented Sep 28, 2022

Tests analytics

Action taken

modular-magician commented Sep 28, 2022

shotarok commented Sep 28, 2022 • edited Loading

ScottSuarez commented Sep 29, 2022

shotarok commented Sep 18, 2022 •

edited

Loading

shotarok Sep 18, 2022 •

edited

Loading

ScottSuarez commented Sep 27, 2022 •

edited

Loading

shotarok commented Sep 28, 2022 •

edited

Loading