-
Notifications
You must be signed in to change notification settings - Fork 24
feat(samples): add all feature values samples #981
Conversation
Warning: This pull request is touching the following templated files:
|
Here is the summary of changes. You are about to add 5 region tags.
This comment is generated by snippet-bot.
|
.setParent(EntityTypeName.of(project, location, featurestoreId, entityTypeId).toString()) | ||
.addAllRequests(createFeatureRequests).build(); | ||
|
||
OperationFuture<BatchCreateFeaturesResponse, BatchCreateFeaturesOperationMetadata> future = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name the operationFutures consistently across the files, maybe use batchCreateFeaturesOperationFuture or just future, but use same across.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, will use same across
List<CreateFeatureRequest> createFeatureRequests = new ArrayList<>(); | ||
|
||
Feature titleFeature = Feature.newBuilder().setDescription("The title of the movie") | ||
.setValueType(ValueType.valueOf("STRING")).build(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the ENUM directly here and in other places as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, will update to ENUM type everywhere as below,
.setValueType(ValueType.valueOf("STRING")).build(); | |
.setValueType(ValueType.STRING).build(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
|
||
Feature titleFeature = Feature.newBuilder().setDescription("The title of the movie") | ||
.setValueType(ValueType.valueOf("STRING")).build(); | ||
Feature genresFeature = Feature.newBuilder().setDescription("The genres of the movie") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: change to singular genre
, to me genres represents an array of genres.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using the same features as https://github.com/GoogleCloudPlatform/vertex-ai-samples/blob/main/notebooks/official/feature_store/gapic-feature-store.ipynb sample tutorial provided in SOW.
So thought it is ok, should we change it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see, let's retain it for now. It was a minor nit.
System.out.format("Operation name: %s%n", future.getInitialFuture().get().getName()); | ||
System.out.println("Waiting for operation to finish..."); | ||
BatchCreateFeaturesResponse batchCreateFeaturesResponse = | ||
future.get(timeout, TimeUnit.SECONDS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to confirm, is the behavior here to wait the 300 sec and timeout otherwise. If there is a response available, it's returned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, if the response is not received in 300 sec it will throw timeout
.setCsvReadInstances(CsvSource.newBuilder().setGcsSource(gcsSource)) | ||
.setDestination( | ||
FeatureValueDestination.newBuilder().setBigqueryDestination(bigQueryDestination)) | ||
// .addAllPassThroughFields(passThroughFieldsList) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we include this as well, if possible ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removing that as there are no passThroughFields for the movie prediction example.
List<String> ids = new ArrayList<>(); | ||
ids.add("*"); | ||
FeatureSelector featureSelector = FeatureSelector.newBuilder() | ||
.setIdMatcher(IdMatcher.newBuilder().addAllIds(ids).build()).build(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: inline something like addAllIds(ArrayList.of()...)
, or make a class variable if intended for developer to update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, will add inline as below,
.setIdMatcher(IdMatcher.newBuilder().addAllIds(ids).build()).build(); | |
.setIdMatcher(IdMatcher.newBuilder().addAllIds(Arrays.asList("title","genres","average_rating")).build()).build(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel it's better to make it one of the variables the developer has to update and pass it down here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated code to pass the feature selector ids as a list object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure sample tests pass before merging.
🤖 I have created a release *beep* *boop* --- ## [3.2.0](v3.1.0...v3.2.0) (2022-08-09) ### Features * add a DeploymentResourcePool API resource_definition ([#997](#997)) ([f3e6d4f](f3e6d4f)) * add DeploymentResourcePool in aiplatform v1beta1 deployment_resource_pool.proto ([#998](#998)) ([49fb069](49fb069)) * add DeploymentResourcePoolService in aiplatform v1beta1 deployment_resource_pool_service.proto ([49fb069](49fb069)) * add shared_resources for supported prediction_resources ([f3e6d4f](f3e6d4f)) * add SHARED_RESOURCES to DeploymentResourcesType in aiplatform v1beta1 model.proto ([49fb069](49fb069)) * added SHARED_RESOURCES enum to aiplatform v1 model.proto ([3935d8f](3935d8f)) * DeploymentResourcePool and DeployementResourcePoolService added to aiplatform v1beta1 model.proto (cl/463147866) ([3935d8f](3935d8f)) * making network arg optional in aiplatform v1 custom_job.proto ([#999](#999)) ([3935d8f](3935d8f)) * making network arg optional in aiplatform v1beta1 custom_job.proto ([3935d8f](3935d8f)) * **samples:** add all feature samples ([#980](#980)) ([a932cf8](a932cf8)) * **samples:** add all feature values samples ([#981](#981)) ([818acab](818acab)) ### Bug Fixes * declaring test-scope artifact as runtime ([#1014](#1014)) ([f90cc12](f90cc12)) ### Documentation * doc edits to aiplatform v1 dataset_service.proto, job_service.proto, model_service.proto, pipeline_service.proto, saved_query.proto, study.proto, types.proto ([3935d8f](3935d8f)) * doc edits to aiplatform v1beta1 job_service.proto, model_service.proto, pipeline_service.proto, saved_query.proto, study.proto, types.proto ([3935d8f](3935d8f)) ### Dependencies * update dependency com.google.api.grpc:proto-google-cloud-aiplatform-v1beta1 to v0.17.0 ([#1003](#1003)) ([c2b98d9](c2b98d9)) * update dependency com.google.cloud:google-cloud-bigquery to v2.14.1 ([#1006](#1006)) ([2c959b9](2c959b9)) * update dependency com.google.cloud:google-cloud-bigquery to v2.14.3 ([#1009](#1009)) ([b170504](b170504)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v3 ([#1000](#1000)) ([6fb1534](6fb1534)) * update dependency com.google.cloud:google-cloud-storage to v2.10.0 ([#1004](#1004)) ([b6e2ab4](b6e2ab4)) * update dependency com.google.cloud:google-cloud-storage to v2.11.0 ([#1005](#1005)) ([26391a3](26391a3)) * update dependency com.google.cloud:google-cloud-storage to v2.11.1 ([#1008](#1008)) ([cba42d1](cba42d1)) * update dependency com.google.cloud:google-cloud-storage to v2.11.2 ([#1010](#1010)) ([4f35eed](4f35eed)) * update dependency com.google.code.gson:gson to v2.9.1 ([#1001](#1001)) ([f12c313](f12c313)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- ## [3.2.0](googleapis/java-aiplatform@v3.1.0...v3.2.0) (2022-08-09) ### Features * add a DeploymentResourcePool API resource_definition ([#997](googleapis/java-aiplatform#997)) ([82551d8](googleapis/java-aiplatform@82551d8)) * add DeploymentResourcePool in aiplatform v1beta1 deployment_resource_pool.proto ([#998](googleapis/java-aiplatform#998)) ([76dc64f](googleapis/java-aiplatform@76dc64f)) * add DeploymentResourcePoolService in aiplatform v1beta1 deployment_resource_pool_service.proto ([76dc64f](googleapis/java-aiplatform@76dc64f)) * add shared_resources for supported prediction_resources ([82551d8](googleapis/java-aiplatform@82551d8)) * add SHARED_RESOURCES to DeploymentResourcesType in aiplatform v1beta1 model.proto ([76dc64f](googleapis/java-aiplatform@76dc64f)) * added SHARED_RESOURCES enum to aiplatform v1 model.proto ([301cfb0](googleapis/java-aiplatform@301cfb0)) * DeploymentResourcePool and DeployementResourcePoolService added to aiplatform v1beta1 model.proto (cl/463147866) ([301cfb0](googleapis/java-aiplatform@301cfb0)) * making network arg optional in aiplatform v1 custom_job.proto ([#999](googleapis/java-aiplatform#999)) ([301cfb0](googleapis/java-aiplatform@301cfb0)) * making network arg optional in aiplatform v1beta1 custom_job.proto ([301cfb0](googleapis/java-aiplatform@301cfb0)) * **samples:** add all feature samples ([#980](googleapis/java-aiplatform#980)) ([8c2a485](googleapis/java-aiplatform@8c2a485)) * **samples:** add all feature values samples ([#981](googleapis/java-aiplatform#981)) ([2d4e6fe](googleapis/java-aiplatform@2d4e6fe)) ### Bug Fixes * declaring test-scope artifact as runtime ([#1014](googleapis/java-aiplatform#1014)) ([6c47c65](googleapis/java-aiplatform@6c47c65)) ### Documentation * doc edits to aiplatform v1 dataset_service.proto, job_service.proto, model_service.proto, pipeline_service.proto, saved_query.proto, study.proto, types.proto ([301cfb0](googleapis/java-aiplatform@301cfb0)) * doc edits to aiplatform v1beta1 job_service.proto, model_service.proto, pipeline_service.proto, saved_query.proto, study.proto, types.proto ([301cfb0](googleapis/java-aiplatform@301cfb0)) ### Dependencies * update dependency com.google.api.grpc:proto-google-cloud-aiplatform-v1beta1 to v0.17.0 ([#1003](googleapis/java-aiplatform#1003)) ([b793732](googleapis/java-aiplatform@b793732)) * update dependency com.google.cloud:google-cloud-bigquery to v2.14.1 ([#1006](googleapis/java-aiplatform#1006)) ([6bb8982](googleapis/java-aiplatform@6bb8982)) * update dependency com.google.cloud:google-cloud-bigquery to v2.14.3 ([#1009](googleapis/java-aiplatform#1009)) ([8cca8b5](googleapis/java-aiplatform@8cca8b5)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v3 ([#1000](googleapis/java-aiplatform#1000)) ([c93de30](googleapis/java-aiplatform@c93de30)) * update dependency com.google.cloud:google-cloud-storage to v2.10.0 ([#1004](googleapis/java-aiplatform#1004)) ([dd52cad](googleapis/java-aiplatform@dd52cad)) * update dependency com.google.cloud:google-cloud-storage to v2.11.0 ([#1005](googleapis/java-aiplatform#1005)) ([60e2f76](googleapis/java-aiplatform@60e2f76)) * update dependency com.google.cloud:google-cloud-storage to v2.11.1 ([#1008](googleapis/java-aiplatform#1008)) ([9a2fe64](googleapis/java-aiplatform@9a2fe64)) * update dependency com.google.cloud:google-cloud-storage to v2.11.2 ([#1010](googleapis/java-aiplatform#1010)) ([3c2ac16](googleapis/java-aiplatform@3c2ac16)) * update dependency com.google.code.gson:gson to v2.9.1 ([#1001](googleapis/java-aiplatform#1001)) ([a6ffed4](googleapis/java-aiplatform@a6ffed4)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #945 ☕️