-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
community[minor], azure-cosmosdb[major]: add new integration for Azure CosmosDB for NoSQL #6133
Conversation
@jacoblee93 I've updated the PR and answered all your comments, let me know if it's good for you? |
this.indexOptions = dbConfig.indexOptions ?? {}; | ||
|
||
// Deferring initialization to the first call to `initialize` | ||
this.initialize = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would prefer this as a static method like this:
Since I'm not sure this will show up well in API refs, but won't block on it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you be ok with a middle ground approach?
- make the current
initialize
method I made internal - add a new static
initialize
method the calls constructor + init/connect the db connection - keep the "automatic" init when calling any VS method in case it wasn't done before (for compatibility / simplicity)
- explain how the init works in the docs
My reasoning: I'd like to make the migration from the previous implementation painless (ie keep the auto init) and also make it work for folks who skim quickly through docs and just call the constructor without using the initialize
method
Thanks! It's failing lint + format but I can have a look. Just FYI - we're also doing a bunch of work right now to standardize our integration docs/make our API refs more usable and maintainable: https://js.langchain.com/v0.2/docs/integrations/vectorstores/pinecone |
Thank you! |
Thanks! I'll look into updating the docs to the new format in the next PR! |
Changes
@langchain/community
@langchain/azure-cosmosdb
AzureCosmosDBMongoDBvCoreVectorStore
instead ofAzureCosmosDBVectorStore
docs