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

Add collections operations to PineconeControlPlaneClient with integration tests #65

Merged
merged 22 commits into from
Feb 21, 2024

Conversation

austin-denoble
Copy link
Contributor

@austin-denoble austin-denoble commented Feb 14, 2024

Problem

The Java SDK is currently missing collections functionality. Now that we've added generated code for the OpenAPI spec, we can hook up collections operations in PineconeControlPlaneClient.

We'd also like to add integration tests to cover collections.

Solution

  • Update PineconeControlPlaneClient to include collections operations for create, list, delete, and describe.
  • Add new integration test files: CollectionTest and CollectionErrorTest.
  • Add new helpers to IndexManager for creating and connecting to an index by indexName, creating a collection and waiting for it to be ready, and polling until an index is ready.
  • Add new helper to BuildUpsertRequest for generating vectors by dimension.

Bonuses:

  • I added some logging configs in build.gradle for test and the integrationTest task. This was primarily to help me debug things in CI and log a bit more info to the console. We can tweak as needed, but I think having something like this will be really helpful as opposed to the generic output.
  • Set max-parallel: 1 in the integration-test job in the pr.yml workflow. Because of the way findIndexWithDimensionAndType works we cannot run integration tests in parallel without flakiness.
  • Fixed an issue in AssertRetry. We were incrementing delay at the class level which is a static variable, meaning if you re-used the function in one place it would continue increasing the delay for all subsequent calls based on the backOff.

Type of Change

  • New feature (non-breaking change which adds functionality)

Test Plan

Run integration tests to make sure CollectionTest and CollectionErrorTest pass as expected. Integration tests should also be passing in CI barring any flakiness.

@austin-denoble austin-denoble changed the title Add collections operations to PineconeControlPlaneClient and collections integration tests Add collections operations to PineconeControlPlaneClient with integration tests Feb 14, 2024
@austin-denoble austin-denoble marked this pull request as ready for review February 15, 2024 01:09
build.gradle Outdated Show resolved Hide resolved
Copy link
Contributor

@rohanshah18 rohanshah18 left a comment

Choose a reason for hiding this comment

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

Great work!! I have left some comments but one area especially I'd like to think about is the hard-coded time outs added for collection tests in indexManager's waitUntilReady methods.

@austin-denoble austin-denoble merged commit 20a38ac into main Feb 21, 2024
8 checks passed
@austin-denoble austin-denoble deleted the adenoble/collection-operations-integration-tests branch February 21, 2024 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants