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.

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(vectorsearch): add create/list/delete index samples #9880

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ericgribkoff
Copy link

Description

Part of b/378713849

Checklist

  • I have followed Sample Format Guide
  • pom.xml parent set to latest shared-configuration
  • Appropriate changes to README are included in PR
  • These samples need a new API enabled in testing projects to pass (let us know which ones)
  • These samples need a new/updated env vars in testing projects set to pass (let us know which ones)
  • Tests pass: mvn clean verify required
  • Lint passes: mvn -P lint checkstyle:check required
  • Static Analysis: mvn -P lint clean compile pmd:cpd-check spotbugs:check advisory only
  • This sample adds a new sample directory, and I updated the CODEOWNERS file with the codeowners for this sample
  • This sample adds a new Product API, and I updated the Blunderbuss issue/PR auto-assigner with the codeowners for this sample
  • Please merge this PR for me once it is approved

@ericgribkoff ericgribkoff requested review from yoshi-approver and a team as code owners December 16, 2024 19:39
Copy link

snippet-bot bot commented Dec 16, 2024

Here is the summary of changes.

You are about to add 4 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

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

Hi @ericgribkoff and reviewers,

Gemini is currently reviewing this pull request and will post a full review in a few minutes. In the meantime, here's a quick summary to help everyone get up to speed:

This PR adds sample code for creating, listing, and deleting vector indexes in Vertex AI. The intent is to provide clear, concise examples for developers using the Java client library. The changes include:

  • Adding four new Java files: CreateIndexSample.java, CreateStreamingIndexSample.java, DeleteIndexSample.java, and ListIndexesSample.java to the aiplatform/src/main/java/aiplatform/vectorsearch directory. These files contain the sample code for the respective operations.
  • Updating the aiplatform/pom.xml file to include necessary dependencies for the Vertex AI Vector Search API. Specifically, the following dependencies were added: grpc-google-cloud-aiplatform-v1, grpc-testing.
  • Adding unit tests for the new sample code in CreateIndexSampleTest.java, CreateStreamingIndexSampleTest.java, DeleteIndexSampleTest.java, and ListIndexesSampleTest.java.

All changes are well-documented and follow the sample format guide. The tests are comprehensive and cover all aspects of the functionality. I'll provide a more detailed review shortly.


And now, a little haiku to brighten your day:

Code flows like a stream,
Tests run green, a joyful sight,
Vectors find their place.

Thanks,
Gemini

@product-auto-label product-auto-label bot added samples Issues that are directly related to samples. api: aiplatform Issues related to the AI Platform API. labels Dec 16, 2024
Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

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

This pull request adds samples for creating, listing, and deleting vector search indexes. The implementation looks good overall, and the provided tests seem comprehensive. However, there are a few minor suggestions for improvement and some missing tests:

  • The contentsDeltaUri variable is declared in the main methods but not used in the corresponding sample methods (createIndexSample and createStreamingIndexSample). It's currently only used in the string formatting for metadataJson. Consider passing it directly to the sample methods and using it to set the contentsDeltaUri field in the metadata. Also, clarify the purpose of contentsDeltaUri in the sample code and ensure it's used correctly.
  • The metadataJson string is hardcoded within the main methods. Consider making it a parameter of the sample methods or loading it from a configuration file to make the samples more flexible.
  • Add more comprehensive Javadoc documentation for the sample methods, explaining the purpose of each parameter and the expected behavior. Include examples of how to use the methods in the documentation.
  • Add a section to the README with examples of how to use these new samples.
  • Add integration tests that interact with the actual Vertex AI service to verify the end-to-end functionality of the samples.

@ericgribkoff ericgribkoff changed the title chore(vectorsearch): add create/list/delete index samples feat(vectorsearch): add create/list/delete index samples Dec 16, 2024
@ericgribkoff
Copy link
Author

  • Add more comprehensive Javadoc documentation for the sample methods, explaining the purpose of each parameter and the expected behavior. Include examples of how to use the methods in the documentation.

The sample methods are themselves examples of using the API and the method names are self-explanatory; I don't think additional Javadoc provides any value, and this matches with the other samples in the repository.

  • Add a section to the README with examples of how to use these new samples.

There is no README, nor do I see a need for one - these samples will exist at https://cloud.google.com/vertex-ai/docs/vector-search/create-manage-index

  • Add integration tests that interact with the actual Vertex AI service to verify the end-to-end functionality of the samples.

I intentionally did not do this; these are samples using the GAPIC API, and the unit tests use stubs for the gRPC vertex API service so that we can validate that the intended API calls are sent by the GAPIC API. E.g., the GAPIC API codegen is fully exercised by the tests in this sample. Validating that the API itself "works" is far beyond the scope of a sample, requires muc greater testing that lives elsewhere, and some of these operations (e.g., create index) are costly and time consuming to run on every commit for sample code.

@iennae iennae assigned polong-lin and unassigned grayside Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: aiplatform Issues related to the AI Platform API. samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants