[spruce] Bump @pinecone-database version to support global control plane - #4
Merged
Merged
Conversation
austin-denoble
commented
Dec 20, 2023
| import { chunkedUpsert } from "./utils/chunkedUpsert.ts"; | ||
|
|
||
| dotenv.config(); | ||
| validateEnvironmentVariables(); |
Contributor
Author
There was a problem hiding this comment.
Added validation here.
austin-denoble
commented
Dec 20, 2023
| const indexList = await pinecone.listIndexes(); | ||
| if (indexList.indexOf({ name: indexName }) === -1) { | ||
| await pinecone.createIndex({ name: indexName, dimension: 384, waitUntilReady: true }) | ||
| if (!indexList.indexes?.some((index) => index.name === indexName)) { |
Contributor
Author
There was a problem hiding this comment.
Update to handle listIndexes() response and the createIndex() call below now includes spec.
austin-denoble
commented
Dec 20, 2023
| const description = await pinecone.describeIndex(indexName); | ||
| if (!description.status?.ready) { | ||
| throw `Index not ready, description was ${JSON.stringify(description)}` | ||
| throw new Error( |
Contributor
Author
There was a problem hiding this comment.
Lint was complaining about not throwing an Error here.
austin-denoble
commented
Dec 20, 2023
| @@ -1,34 +1,29 @@ | |||
| import type { Index, PineconeRecord } from '@pinecone-database/pinecone'; | |||
| import type { Index, PineconeRecord } from "@pinecone-database/pinecone"; | |||
| import { sliceIntoChunks } from "./util.ts"; | |||
Contributor
Author
There was a problem hiding this comment.
The sliceIntoChunks function below was a duplicate of what we have in utils.
austin-denoble
commented
Dec 20, 2023
| await Promise.allSettled( | ||
| chunks.map(async (chunk) => { | ||
| try { | ||
| await index.namespace(namespace).upsert(chunk); |
Contributor
Author
There was a problem hiding this comment.
Previously we were upserting vectors instead of the chunk itself.
…File in index.ts to use 100000
rschwabco
requested changes
Dec 20, 2023
rschwabco
left a comment
rschwabco
left a comment
Contributor
There was a problem hiding this comment.
Other than the hard coded region/cloud - lgtm
Contributor
Author
|
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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've been working to support the new global control plane service in the Pinecone clients. We need to update our sample apps to migrate to the new client versions, and update associated code paths as needed.
Solution
README.md,.env.example, andvalidateEnvironmentVariables()to removePINECONE_ENVIRONMENT.index.tsto handle the newlistIndexes()response shape, and passing the proper config tocreateIndex(). Update relevant snippets inREADME.md.I also took a pass at some minor cleanup not directly related to the upgrade:
validateEnvironmentVariables()relevant code is run (index.ts, andrecommend.ts). This is minor asgetEnv()will throw in the same way, but the validation util wasn't used at all.sliceIntoChunks()function.chunkedUpsert()wherechunks.map(async (chunk) => ...)was upsertingvectorsand not the specificchunk.For now, I've created a spruce branch to merge this into as we probably don't want to release sample apps that aren't aligned with what's available publicly.
Type of Change
Test Plan
There are no unit tests associated with this project, so testing manually is required. I tested both the embedding and upsert path along with the recommendations path, and they seem to work as we'd expect. We should add some level of testing and validation to this app at some point.
Validate embedding and upsert
npm run indexValidate recommendations
npm run recommend -- --query="Xbox" --section="Games  "