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 global control plane code #59

Merged
merged 5 commits into from
Jan 29, 2024
Merged

Add global control plane code #59

merged 5 commits into from
Jan 29, 2024

Conversation

rohanshah18
Copy link
Contributor

@rohanshah18 rohanshah18 commented Jan 24, 2024

Problem

Global control plane support needs to be added to the Java SDK.

Solution

Add open api generated code for the global control plane.

  1. Files under org.openapitools.client are auto generated and can be ignored during the review
  2. Updated gradle build to gradle clean build for the github actions
  3. Rearranged the test folders for adding collection tests
  4. Updated build.gradle to add dependencies for the open api generated code

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Infrastructure change (CI configs, etc)
  • Non-code change (docs, etc)
  • None of the above: (explain here)

Test Plan

Ran integration tests for the existing code base but future PR's will be adding unit and integration tests for each operations.

@rohanshah18 rohanshah18 marked this pull request as ready for review January 29, 2024 19:47
Copy link
Contributor

@austin-denoble austin-denoble left a comment

Choose a reason for hiding this comment

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

Overall code looks good to me, we'll need to integrate all of the generated control plane logic into PineconeIndexOperationClient to really test things out, but the passing builds should give us some confidence here.

It seems one of the integration test runs is failing. Specifically Java 17 with Gradle 7.3.1. Looks like the ConfigureIndexTest failed a bunch of different times on setUp(). The other failure is UpdateAndQueryTest where it looks like there was an assertion failure, which may be due to flakiness or freshness. Not sure if you want to try and address these before merging.

@rohanshah18 rohanshah18 merged commit 12a2ee5 into main Jan 29, 2024
7 of 8 checks passed
@rohanshah18 rohanshah18 deleted the rshah/addgCPS branch January 29, 2024 22:07
@rohanshah18
Copy link
Contributor Author

@austin-denoble That's correct! As we discussed internally, this PR is meant to add openAPI changes only and the future index operations tickets are meant to add integration tests for index operations.
Also you're currently working on the collections, those tickets will help us test the open api generated code for collections.

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