Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promote Vertex AI FeatureStore resources (GA only) #6565

Merged
merged 13 commits into from
Oct 14, 2022
Merged

Promote Vertex AI FeatureStore resources (GA only) #6565

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
626404e
feat: promote vertex ai feature store resources (ga only)
shotarok Sep 3, 2022
89d6672
feat: deprecate the field: monitoring_interval
shotarok Sep 27, 2022
3e76e59
Revert "feat: deprecate the field: monitoring_interval"
shotarok Sep 28, 2022
b600ab9
feat: update the example to use only the GA fields
shotarok Sep 28, 2022
a540bf2
Merge remote-tracking branch 'gcp/main' into promote-vertex-ai-featur…
shotarok Sep 30, 2022
c37fbcd
fix: diable monitoring_config.snapshot_analysis in the example
shotarok Sep 30, 2022
bc1954a
feat: promote the featurestore entity feature resource to GA
shotarok Sep 30, 2022
d3b5090
fix: add default_value to fix the unintentional diff
shotarok Oct 1, 2022
2b0982d
feat: update the link to the api doc
shotarok Oct 1, 2022
acc9fb1
Revert "fix: add default_value to fix the unintentional diff"
shotarok Oct 8, 2022
c87fbf6
refactor: use deprecation_message to clarify the deprecated field
shotarok Oct 8, 2022
9662dd4
feat: add a new example to test beta fields
shotarok Oct 12, 2022
6d8c548
fix: use a different resource name to run tests in parallel
shotarok Oct 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions mmv1/products/vertexai/api.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,13 @@ objects:
base_url: projects/{{project}}/locations/{{region}}/featurestores
create_url: projects/{{project}}/locations/{{region}}/featurestores?featurestoreId={{name}}
self_link: 'projects/{{project}}/locations/{{region}}/featurestores/{{name}}'
min_version: beta
update_verb: :PATCH
update_mask: true
references: !ruby/object:Api::Resource::ReferenceLinks
guides:
'Official Documentation':
'https://cloud.google.com/vertex-ai/docs'
api: 'https://cloud.google.com/vertex-ai/docs/reference/rest/v1beta1/projects.locations.featurestores'
api: 'https://cloud.google.com/vertex-ai/docs/reference/rest/v1/projects.locations.featurestores'
async: !ruby/object:Api::OpAsync
operation: !ruby/object:Api::OpAsync::Operation
path: 'name'
Expand Down Expand Up @@ -194,14 +193,13 @@ objects:
base_url: '{{featurestore}}/entityTypes'
create_url: '{{featurestore}}/entityTypes?entityTypeId={{name}}'
self_link: '{{featurestore}}/entityTypes/{{name}}'
min_version: beta
update_verb: :PATCH
update_mask: true
references: !ruby/object:Api::Resource::ReferenceLinks
guides:
'Official Documentation':
'https://cloud.google.com/vertex-ai/docs'
api: 'https://cloud.google.com/vertex-ai/docs/reference/rest/v1beta1/projects.locations.featurestores.entityTypes'
api: 'https://cloud.google.com/vertex-ai/docs/reference/rest/v1/projects.locations.featurestores.entityTypes'
async: !ruby/object:Api::OpAsync
operation: !ruby/object:Api::OpAsync::Operation
path: 'name'
Expand Down Expand Up @@ -273,7 +271,11 @@ objects:
The monitoring schedule for snapshot analysis. For EntityType-level config: unset / disabled = true indicates disabled by default for Features under it; otherwise by default enable snapshot analysis monitoring with monitoringInterval for Features under it.
- !ruby/object:Api::Type::String
name: 'monitoringInterval'
default_value: "0s"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't add a default value in a minor version of the provider. It's technically considered a breaking change even if it was the implicit default value before

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slevenick Got it! I'll revert the change. BTW, could you give me an example to test a beta field? I'd like to test monitoring_interval only in the google-beta.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, can we define an additional google_vertex_ai_featurestore_entitytype only in the test of the google-beta provider?

resource "google_vertex_ai_featurestore_entitytype" "entity" {
  name     = "<%= ctx[:vars]['name'] %>"
  labels = {
    foo = "bar"
  }
  featurestore = google_vertex_ai_featurestore.featurestore.id
  monitoring_config {
    snapshot_analysis {
      disabled = true
    }
  }
}


<%- if is_beta %>
resource "google_vertex_ai_featurestore_entitytype" "entity_for_beta" {
  provider = google-beta
  name     = "<%= ctx[:vars]['name'] + '_for_beta' %>"
  labels = {
    foo = "bar"
  }
  featurestore = google_vertex_ai_featurestore.featurestore.id
  monitoring_config {
    snapshot_analysis {
      disabled = false
      monitoring_interval = "86400s"
    }
  }
}
<%- end %>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you can find examples of the version tag needed to make some code only exist in the beta provider. I think its something like <%- unless version == "ga" %>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slevenick it turns out there is no version information in the bindings to render the example. I'm wondering if there is another way to test a beta field. Or should I pass the version to render the example?

magic-modules/mmv1/provider/terraform/examples.rb

Lines 259 to 267 in 3132afe

body = lines(compile_file(
{
vars: rand_vars.merge(overrides),
test_env_vars: test_env_vars.map { |k, _| [k, "%{#{k}}"] }.to_h,
primary_resource_id: primary_resource_id,
primary_resource_type: primary_resource_type
},
pwd + '/' + config_path
))

Copy link
Contributor Author

@shotarok shotarok Oct 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understood we could define multiple examples for a resource, and we can define min_version of an example object. I'll add two example erb files.

Copy link
Contributor Author

@shotarok shotarok Oct 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could define an example only for the beta provider by min_version of an example object. However, I couldn't define an example only for the ga provider only with the min_version.

magic-modules/mmv1/templates/terraform/examples/base_configs/test_file.go.erb

Lines 29 to 32 in 13f6fa6

object.examples
.reject(&:skip_test)
.reject { |e| @api.version_obj_or_closest(version) < @api.version_obj_or_closest(e.min_version) }
.each do |example|

Copy link
Contributor Author

@shotarok shotarok Oct 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created two examples. One is without monitoring_interval, which works for both ga and beta. Another example is with monitoring_interval, which only works for beta.

min_version: beta
description: |
This item is deprecated!
shotarok marked this conversation as resolved.
Show resolved Hide resolved

Configuration of the snapshot analysis based monitoring pipeline running interval. The value is rolled up to full day.

A duration in seconds with up to nine fractional digits, terminated by 's'. Example: "3.5s".
Expand All @@ -284,14 +286,13 @@ objects:
base_url: '{{entitytype}}/features'
create_url: '{{entitytype}}/features?featureId={{name}}'
self_link: '{{entitytype}}/features/{{name}}'
min_version: beta
update_verb: :PATCH
update_mask: true
references: !ruby/object:Api::Resource::ReferenceLinks
guides:
'Official Documentation':
'https://cloud.google.com/vertex-ai/docs'
api: 'https://cloud.google.com/vertex-ai/docs/reference/rest/v1beta1/projects.locations.featurestores.entityTypes.features'
api: 'https://cloud.google.com/vertex-ai/docs/reference/rest/v1/projects.locations.featurestores.entityTypes.features'
async: !ruby/object:Api::OpAsync
operation: !ruby/object:Api::OpAsync::Operation
path: 'name'
Expand Down
1 change: 0 additions & 1 deletion mmv1/templates/terraform/examples/vertex_ai_featurestore.tf.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
resource "google_vertex_ai_featurestore" "featurestore" {
provider = google-beta
name = "<%= ctx[:vars]['name'] %>"
labels = {
foo = "bar"
Expand Down
5 changes: 1 addition & 4 deletions mmv1/templates/terraform/examples/vertex_ai_featurestore_entitytype.tf.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
resource "google_vertex_ai_featurestore" "featurestore" {
provider = google-beta
name = "<%= ctx[:vars]['name'] %>"
labels = {
foo = "bar"
Expand All @@ -14,16 +13,14 @@ resource "google_vertex_ai_featurestore" "featurestore" {
}

resource "google_vertex_ai_featurestore_entitytype" "entity" {
provider = google-beta
name = "<%= ctx[:vars]['name'] %>"
labels = {
foo = "bar"
}
featurestore = google_vertex_ai_featurestore.featurestore.id
monitoring_config {
snapshot_analysis {
disabled = false
monitoring_interval = "86400s"
disabled = true
}
}
}
6 changes: 1 addition & 5 deletions mmv1/templates/terraform/examples/vertex_ai_featurestore_entitytype_feature.tf.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
resource "google_vertex_ai_featurestore" "featurestore" {
provider = google-beta
name = "<%= ctx[:vars]['name'] %>"
labels = {
foo = "bar"
Expand All @@ -11,22 +10,19 @@ resource "google_vertex_ai_featurestore" "featurestore" {
}

resource "google_vertex_ai_featurestore_entitytype" "entity" {
provider = google-beta
name = "<%= ctx[:vars]['name'] %>"
labels = {
foo = "bar"
}
featurestore = google_vertex_ai_featurestore.featurestore.id
monitoring_config {
snapshot_analysis {
disabled = false
monitoring_interval = "86400s"
disabled = true
}
}
}

resource "google_vertex_ai_featurestore_entitytype_feature" "feature" {
provider = google-beta
name = "<%= ctx[:vars]['name'] %>"
labels = {
foo = "bar"
Expand Down