feat(vertexai): Add fields to Vertex AI FeatureStore EntityType for Feature Value Monitoring #6699

shotarok · 2022-10-16T04:57:46Z

part of hashicorp/terraform-provider-google#9298

Add fields to Vertex AI FeatureStore EntityType for Feature Value Monitoring. This is the GA API reference: https://cloud.google.com/vertex-ai/docs/reference/rest/v1/projects.locations.featurestores.entityTypes#FeaturestoreMonitoringConfig.

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)

vertexai: add fields to `vertex_ai_featurestore_entitytype` to support feature value monitoring

modular-magician · 2022-10-16T04:58:50Z

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.

mmv1/products/vertexai/terraform.yaml

modular-magician · 2022-10-16T05:11:49Z

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

Diff report:

Terraform GA: Diff ( 3 files changed, 384 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 3 files changed, 385 insertions(+), 2 deletions(-))
TF Validator: Diff ( 3 files changed, 126 insertions(+), 3 deletions(-))

modular-magician · 2022-10-16T05:51:42Z

Tests analytics

Total tests: 2191
Passed tests 1948
Skipped tests: 239
Failed tests: 4

Action taken

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

TestAccDataSourceGoogleServiceAccountIdToken_basic|TestAccVertexAIFeaturestoreEntitytype_vertexAiFeaturestoreEntitytypeExample|TestAccVertexAIFeaturestoreEntitytypeFeature_vertexAiFeaturestoreEntitytypeFeatureWithBetaFieldsExample|TestAccVertexAIFeaturestoreEntitytype_vertexAiFeaturestoreEntitytypeWithBetaFieldsExample

modular-magician · 2022-10-16T05:53:22Z

Tests passed during RECORDING mode:
TestAccDataSourceGoogleServiceAccountIdToken_basic[Debug log]
TestAccVertexAIFeaturestoreEntitytype_vertexAiFeaturestoreEntitytypeExample[Debug log]

Tests failed during RECORDING mode:
TestAccVertexAIFeaturestoreEntitytypeFeature_vertexAiFeaturestoreEntitytypeFeatureWithBetaFieldsExample[Error message] [Debug log]
TestAccVertexAIFeaturestoreEntitytype_vertexAiFeaturestoreEntitytypeWithBetaFieldsExample[Error message] [Debug log]

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

shotarok · 2022-10-16T06:44:55Z

[MEMO] Based on the test error logs I reproduced locally, I updated the code to deal with the fixed default values in 7141575.

modular-magician · 2022-10-16T06:57:26Z

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

Diff report:

Terraform GA: Diff ( 4 files changed, 403 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 5 files changed, 420 insertions(+), 2 deletions(-))
TF Validator: Diff ( 3 files changed, 126 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 1 file changed, 8 insertions(+))

modular-magician · 2022-10-16T07:35:53Z

Tests analytics

Total tests: 2191
Passed tests 1950
Skipped tests: 239
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

TestAccVertexAIFeaturestoreEntitytype_vertexAiFeaturestoreEntitytypeWithBetaFieldsExample|TestAccVertexAIFeaturestoreEntitytypeFeature_vertexAiFeaturestoreEntitytypeFeatureWithBetaFieldsExample

modular-magician · 2022-10-16T07:37:38Z

Tests passed during RECORDING mode:
TestAccVertexAIFeaturestoreEntitytype_vertexAiFeaturestoreEntitytypeWithBetaFieldsExample[Debug log]
TestAccVertexAIFeaturestoreEntitytypeFeature_vertexAiFeaturestoreEntitytypeFeatureWithBetaFieldsExample[Debug log]

All tests passed
View the build log or the debug log for each test

shotarok · 2022-10-16T13:53:58Z

@megan07 All tests passed. Could you please review this PR when you have time? Thanks

shotarok · 2022-10-24T17:50:26Z

Hi @megan07, please let me ping you based on the step 10. 🙏

Hello @slevenick, we discussed the previous PR: #6565 (review). I'd appreciate it if I could get some advice on #6699 (comment). Thanks

megan07

Looks good, I have a couple of minor comments. Thanks!

mmv1/products/vertexai/api.yaml

megan07 · 2022-11-07T17:35:59Z

mmv1/products/vertexai/api.yaml

+                name: 'value'
+                description: |
+                  Specify a threshold value that can trigger the alert. For numerical feature, the distribution distance is calculated by Jensen–Shannon divergence. Each feature must have a non-zero threshold if they need to be monitored. Otherwise no alert will be triggered for that feature. The default value is 0.3.
+                default_value: 0.3


I think we want to make this required rather than having a default, as we don't want numerical_threshold_config to be an empty block. Same with categorical_threshold_config below. What are your thoughts?

If I remember correctly, the API returned 0.3 when not specifying the fields. That said, I agree. we don't want them to be empty fields. I'll make them required. Thanks!

mmv1/products/vertexai/terraform.yaml

modular-magician · 2022-11-08T20:54:33Z

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 ( 4 files changed, 397 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 5 files changed, 414 insertions(+), 2 deletions(-))
TF Validator: Diff ( 3 files changed, 126 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 1 file changed, 8 insertions(+))

modular-magician · 2022-11-08T21:34:21Z

Tests analytics

Total tests: 2230
Passed tests 1987
Skipped tests: 241
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

TestAccComputeForwardingRule_update|TestAccBigQueryDataTable_bigtable

modular-magician · 2022-11-08T21:35:39Z

Tests passed during RECORDING mode:
TestAccComputeForwardingRule_update[Debug log]
TestAccBigQueryDataTable_bigtable[Debug log]

All tests passed
View the build log or the debug log for each test

shotarok · 2022-11-08T21:36:51Z

Hi @megan07, I updated the code, and all tests passed. Could you please review this PR again? Thanks!

megan07

LGTM! Thanks!

…eature Value Monitoring (GoogleCloudPlatform#6699) * feat: add fields for feature monitoring * fix: use default_value for the fixed default values * fix: update code based on feedback

feat: add fields for feature monitoring

abb55c1

modular-magician requested a review from megan07 October 16, 2022 04:58

shotarok commented Oct 16, 2022

View reviewed changes

mmv1/products/vertexai/terraform.yaml Show resolved Hide resolved

shotarok commented Oct 16, 2022

View reviewed changes

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

fix: use default_value for the fixed default values

7141575

shotarok changed the title ~~Add fields to Vertex AI FeatureStore EntityType for Feature Value Monitoring~~ feat(vertexai): Add fields to Vertex AI FeatureStore EntityType for Feature Value Monitoring Oct 24, 2022

megan07 suggested changes Nov 7, 2022

View reviewed changes

fix: update code based on feedback

d689f84

modular-magician requested a review from megan07 November 8, 2022 20:40

megan07 approved these changes Nov 8, 2022

View reviewed changes

megan07 merged commit 2192aa3 into GoogleCloudPlatform:main Nov 8, 2022

shotarok deleted the vertex-ai-featurestore-monitoring-config branch November 8, 2022 22:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(vertexai): Add fields to Vertex AI FeatureStore EntityType for Feature Value Monitoring #6699

feat(vertexai): Add fields to Vertex AI FeatureStore EntityType for Feature Value Monitoring #6699

shotarok commented Oct 16, 2022 •

edited

Loading

modular-magician commented Oct 16, 2022

modular-magician commented Oct 16, 2022

modular-magician commented Oct 16, 2022

modular-magician commented Oct 16, 2022

shotarok commented Oct 16, 2022 •

edited

Loading

modular-magician commented Oct 16, 2022

modular-magician commented Oct 16, 2022

modular-magician commented Oct 16, 2022

shotarok commented Oct 16, 2022

shotarok commented Oct 24, 2022 •

edited

Loading

megan07 left a comment

megan07 Nov 7, 2022

shotarok Nov 8, 2022

modular-magician commented Nov 8, 2022

modular-magician commented Nov 8, 2022

modular-magician commented Nov 8, 2022

shotarok commented Nov 8, 2022

megan07 left a comment

feat(vertexai): Add fields to Vertex AI FeatureStore EntityType for Feature Value Monitoring #6699

feat(vertexai): Add fields to Vertex AI FeatureStore EntityType for Feature Value Monitoring #6699

Conversation

shotarok commented Oct 16, 2022 • edited Loading

modular-magician commented Oct 16, 2022

modular-magician commented Oct 16, 2022

Diff report:

modular-magician commented Oct 16, 2022

Tests analytics

Action taken

modular-magician commented Oct 16, 2022

shotarok commented Oct 16, 2022 • edited Loading

modular-magician commented Oct 16, 2022

Diff report:

modular-magician commented Oct 16, 2022

Tests analytics

Action taken

modular-magician commented Oct 16, 2022

shotarok commented Oct 16, 2022

shotarok commented Oct 24, 2022 • edited Loading

megan07 left a comment

Choose a reason for hiding this comment

megan07 Nov 7, 2022

Choose a reason for hiding this comment

shotarok Nov 8, 2022

Choose a reason for hiding this comment

modular-magician commented Nov 8, 2022

Diff report

modular-magician commented Nov 8, 2022

Tests analytics

Action taken

modular-magician commented Nov 8, 2022

shotarok commented Nov 8, 2022

megan07 left a comment

Choose a reason for hiding this comment

shotarok commented Oct 16, 2022 •

edited

Loading

shotarok commented Oct 16, 2022 •

edited

Loading

shotarok commented Oct 24, 2022 •

edited

Loading