-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
support connect milvus with token, fix auto id #2345
support connect milvus with token, fix auto id #2345
Conversation
Signed-off-by: nameczz <[email protected]>
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -632,7 +632,7 @@ | |||
"@typescript-eslint/parser": "^5.58.0", |
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.
Great work on the PR! I've noticed a change in the "@zilliz/milvus2-sdk-node" dependency version, which seems to be a hard dependency change. This comment is to flag the change for maintainers to review and ensure it aligns with the project's requirements.
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.
I only update package.json in this pr, not sure about yarn.lock
langchain/src/vectorstores/milvus.ts
Outdated
@@ -116,10 +119,26 @@ export class Milvus extends VectorStore { |
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.
This comment is flagging the change in the PR for maintainers to review as it accesses an environment variable indirectly through the getEnvironmentVariable
function.
@@ -421,7 +440,7 @@ export class Milvus extends VectorStore { | |||
if (field.is_primary_key) { | |||
this.primaryField = field.name; | |||
} | |||
const dtype = DataTypeMap[field.data_type.toLowerCase()]; | |||
const dtype = DataTypeMap[field.data_type]; |
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.
In new sdk, data_type is keyof typeof DataType. So we don't need to lowerCase here, otherwise ts will throw error.
…update-milvus-with-new-sdk
Looks good! Appreciate the backwards compatibility shim! |
Thank you! |
Milvus node sdk support connect with token after v2.2.11 .
User need to use token if want to connect ZILLIZ cloud serverless cluster.
Also i find the hard code auto id = true in LangChain, so i optimized it.
Please help review this pr , thanks.