Merge release-candidate/2024-10 branch to main
#85
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… regenerate for `2024-10` (#76) ## Problem The upstream `/apis` submodule has undergone some changes since we last regenerated the client core. Our `/codegen/build-clients.sh` script no longer handles the new module naming or structure properly. Additionally, we want to regenerate off the upcoming API version so we can pull in changes for things like Rerank. This PR is **merging to a release candidate (RC) branch: `release-candidate/2024-10`**. We will track upcoming major version changes in this branch and use it for releasing dev builds. ## Solution - Refactor `codegen/build-clients.sh` script to support the newer structure for the upcoming API version, specifically `inference` is its own module, and `db_control`/`db_data` as separate modules. - Update imports and exports for `control` -> `db_control`, and `control` -> `inference` for the `Embed()` method on `InferenceService`. We made the decision to have `InferenceService` as its own module inside `Client` because of the split in the API spec, so this was fairly easy to refactor around here. ## 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 - [X] Infrastructure change (CI configs, etc) - [ ] Non-code change (docs, etc) - [ ] None of the above: (explain here) ## Test Plan The code changes under `/internal/gen/*` are all from running the new `/codegen/build-clients.sh` script. I changed files in the submodule for `apis` manually for generating, but it aligns with this PR: pinecone-io/apis#138 Make sure CI build + tests pass for the new core stuff here.
## Problem We are releasing a new version of the API this month: `2024-10`. There are 3 primary new features that are included in this release: - Import - Inference - Embed - Rerank This PR implements the operations to support import. Sorry about the size, but you can basically ignore all of the generated code under `internal/gen` unless you're curious about the new structure of the generated core files. Follow the `codgen/build-clients.sh` script for those details. ## Solution Since the import operations are technically part of the data plane but only supported via REST, they are represented in the OpenAPI spec and not our protos file. Because of this, we need to change a few things to support these operations `Client` and `IndexConnection` structs to support these operations because traditionally the code `IndexConnection` wraps was targeting gRPC-only db data operations. We now need to generate rest code for the data plane as well so we can interact with imports. - Update the `codegen/build-clients.sh` script to handle building new modules for both `internal/gen/db_data/grpc` and `internal/gen/db_data/rest`. - Update `Client` struct and move `NewClientBaseParams` into a field that can be shared more easily when constructing the `IndexConnection`. - Add `buildDataClientBaseOptions` to handle constructing the necessary rest client options for the underlying `dbDataClient`. - Add an `ensureHostHasHttps` helper as we need to make sure this is present for the index `Host` that's passed, which was not necessary for grpc. - Update `Index` method to handle calling `buildDataClientBaseOptions` and passes the new client into `newIndexConnection`. - Update `IndexConnection` to support both REST and gRPC interfaces under the hood (`restClient`, `grpcClient`). - Update `newIndexConnection` to support attaching the new `restClient` to the `IndexConnection` struct. - Update `IndexConnection` to support all import operations: `StartImport`, `ListImports`, `DescribeImport`, `CancelImport`. - Add end-to-end integration test for validating the import flow against serverless indexes. - Some nitpicky code cleanup, renaming of things around the new rest vs. grpc paradigm, etc. ## Type of Change - [ ] Bug fix (non-breaking change which fixes an issue) - [X] 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 `just test` - make sure CI passes To see examples of how to use the new methods, check the doc comments. --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1208325183834377 - https://app.asana.com/0/0/1208541827330963
## Problem The `InferenceService` and `Embed` operation were implemented in a previous PR: #67 As a part of the `2024-10` release we are also adding `Rerank` to the client, which will live under `InferenceService` with `Embed`. ## Solution This work builds on top of external contributions from @Stosan (#73) Thank you for you diligence in getting this started! - Update `InferenceService` to expose the new `Rerank` method. - Add various request and response types to represent the interface. - Add doc comments and an integration test for calling the reranker. ## Type of Change - [ ] Bug fix (non-breaking change which fixes an issue) - [X] 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 `just test` If you want to use the code yourself, here's an example: ```go ctx := context.Background() clientParams := pinecone.NewClientParams{ ApiKey: "YOUR_API_KEY", } pc, err := pinecone.NewClient(clientParams) if err != nil { log.Fatalf("Failed to create Client: %v", err) } rerankModel := "bge-reranker-v2-m3" topN := 2 retunDocuments := true documents := []pinecone.Document{ { "id": "vec1", "text": "Apple is a popular fruit known for its sweetness and crisp texture."}, { "id": "vec2", "text": "Many people enjoy eating apples as a healthy snack."}, { "id": "vec3", "text": "Apple Inc. has revolutionized the tech industry with its sleek designs and user-friendly interfaces."}, { "id": "vec4", "text": "An apple a day keeps the doctor away, as the saying goes."}, } ranking, err := pc.Inference.Rerank(ctx, &pinecone.RerankRequest{ Model: rerankModel, Query: "i love cats", ReturnDocuments: &retunDocuments, TopN: &topN, RankFields: &[]string{"text"}, Documents: documents, }) if err != nil { log.Fatalf("Failed to rerank: %v", err) } fmt.Printf("Rerank result: %+v\n", ranking) ``` --------- Co-authored-by: Sam Ayo <[email protected]>
## Problem In anticipation of the upcoming major release, we need to update the Go README to incorporate sections for `rerank` and `import`. ## 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) - [x] Non-code change (docs, etc) - [ ] None of the above: (explain here)
## Problem There have been a few small PRs that have gone into the `apis` repo that we need to regenerate code for: - pinecone-io/apis#175 - pinecone-io/apis#171 We're also returning the generated `EmbeddingsList` type for the `Embed` method rather than a custom type like we're doing elsewhere. ## Solution - Add new `EmbedResponse`, and `Embedding` types and return from `Embed`. Add doc comments. - Regenerate core SDK code from `2024-10` ## Type of Change - [ ] Bug fix (non-breaking change which fixes an issue) - [X] 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 Make sure CI is successful.
## Problem We can actually hyperlink to all of the contents of the `Pinecone` package in our doc comments, which is nice. There's also a few final tweaks I want to get in before the release. ## Solution - Clean up doc comments and add links for various namespaces. - Add an example of passing errorMode for import in integration test. All of the changes here are to code comments, there aren't any actual changes to code included in this PR. ## 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) - [X] Non-code change (docs, etc) - [ ] None of the above: (explain here) ## Test Plan `just docs` locally to serve the webpage including all of the client documentation
## Problem PR was merged today making some slight adjustments to the db control spec to better align with the server impl: pinecone-io/apis#181 ## Solution Slightly adjust the `PodSpec` type and related code to account for the new shape. ## Type of Change - [ ] Bug fix (non-breaking change which fixes an issue) - [X] 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 `just test` - CI
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
We have been developing the client against the upcoming
2024-10Pinecone API release in a release candidate branch:release-candidate/2024-10We need to merge this branch into
mainso we can cut thev2.0.0release.Solution
Merge
release-candidate/2024-10intomain. All of the commits on the branch have been validated through prior code reviews. The new features for2024-10includeEmbed,Rerank, andImport:/codegen/build-clients.shto support newapisstructure, regenerate for2024-10#762024-10API #79Type of Change
Test Plan
just testCI and the testing suite should pass against this PR. Additionally, we've done validation against each individual commit that comprises the merge.