community[minor]: Add Upstash Embeddings Support #5150
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
dosubot
bot
added
size:L
![dosubot[bot]](https://avatars.githubusercontent.com/in/324583?s=60&v=4){kind=small}
| @@ -203,7 +272,7 @@ export class UpstashVectorStore extends VectorStore { | |||
| static async fromTexts( | |||
| texts: string[], | |||
| metadatas: UpstashMetadata | UpstashMetadata[], | |||
| embeddings: EmbeddingsInterface, | |||
| embeddings: EmbeddingsInterface | "UpstashEmbeddings", | |||
There was a problem hiding this comment.
We've been using FakeEmbeddings like this - could we do the same here?
There was a problem hiding this comment.
Oh, I mean instead of passing a string here, pass FakeEmbeddings
There was a problem hiding this comment.
The thing is, passing something explicit like "UpstashEmbeddings" makes it easier to understand you have the option to either get the embeddings from Upstash, or passing a embedding client to create the embeddings your self. In the Vectara case, it seems that there's no option to pass an embedding, so all the embeddings will be created directly by it self. With Upstash, this is optional. Sorry for the delay here.
|
Sorry for the delay! |
| @@ -203,7 +272,7 @@ export class UpstashVectorStore extends VectorStore { | |||
| static async fromTexts( | |||
| texts: string[], | |||
| metadatas: UpstashMetadata | UpstashMetadata[], | |||
| embeddings: EmbeddingsInterface, | |||
| embeddings: EmbeddingsInterface | "UpstashEmbeddings", | |||
There was a problem hiding this comment.
Oh, I mean instead of passing a string here, pass FakeEmbeddings
|
Hey @jacoblee93 any chance for review again? |
Hey, sorry for the delay! I left a note about using |
Hey, it's ready as you asked. |
| @@ -78,10 +96,14 @@ export class UpstashVectorStore extends VectorStore { | |||
| */ | |||
| async addDocuments( | |||
| documents: DocumentInterface[], | |||
| options?: { ids?: string[] } | |||
| options?: { ids?: string[]; UpstashEmbeddings?: boolean } | |||
There was a problem hiding this comment.
Small style nit - we generally use camel case for properties. I can push a rename
jacoblee93
changed the title
| @@ -45,22 +50,35 @@ const CONCURRENT_UPSERT_LIMIT = 1000; | |||
| export class UpstashVectorStore extends VectorStore { | |||
| declare FilterType: string; | |||
|
|
|||
| declare embeddings: EmbeddingsInterface; | |||
There was a problem hiding this comment.
Should make this an optional prop
| index: UpstashIndex; | ||
|
|
||
| caller: AsyncCaller; | ||
|
|
||
| embeddings: EmbeddingsInterface; | ||
| upstashEmbeddingsConfig?: boolean; |
There was a problem hiding this comment.
Renaming to useUpstashEmbeddings instead
| * @param options Optional object containing the array of ids for the documents. | ||
| * @returns Promise that resolves with the ids of the provided documents when the upsert operation is done. | ||
| */ | ||
| async addData(documents: DocumentInterface[], options?: { ids?: string[] }) { |
There was a problem hiding this comment.
Going to make this protected since I think redundant vs. just calling addDocuments
…feature_upstash
|
Thank you! I renamed a couple of things but re-ran the integration tests and they passed. Will get this released today. |
This PR adds Upstash Embeddings support, which allows users to send their data to Upstash Vector Database without creating the embeddings. Upstash Embeddings will handle the embedding operation for them.
More context on Upstash announcement