-
Notifications
You must be signed in to change notification settings - Fork 354
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.
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
feat: code snippets for feature store control plane #709
Conversation
Here is the summary of changes. You are about to add 4 region tags.
This comment is generated by snippet-bot.
|
5ce50af
to
cfc2ff9
Compare
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 for the samples, left a few comments, also, the delete sample need a testing too.
samples/snippets/feature_store_service/create_entity_type_sample.py
Outdated
Show resolved
Hide resolved
samples/snippets/feature_store_service/delete_featurestore_sample.py
Outdated
Show resolved
Hide resolved
samples/snippets/feature_store_service/create_featurestore_sample_test.py
Show resolved
Hide resolved
samples/snippets/feature_store_service/create_entity_type_sample_test.py
Show resolved
Hide resolved
samples/snippets/feature_store_service/create_feature_sample_test.py
Outdated
Show resolved
Hide resolved
For delete sample, I see that dataset_service only has the sample file, and no test for delete sample was provided: I wonder if this is intentional as the teardown procedure already has the deletion snippets. |
678e868
to
0c84871
Compare
Anyways, I've added delete featurestore test |
0c84871
to
27a15d5
Compare
* Add resource creation and deletion including featurestore, entity type and feature.
27a15d5
to
4d8e39d
Compare
morgandu@ is OOO and lclc@ has addressed all requested changes, lifting blocking review.
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.
LGTM - @morgandu's requested changes have been addressed, tests pass.
No description provided.