fix: apply collection configuration during Qdrant upsert#5978
fix: apply collection configuration during Qdrant upsert#5978majiayu000 wants to merge 3 commits intoFlowiseAI:mainfrom
Conversation
The Additional Collection Configuration (shard_number, replication_factor, etc.) was only applied in the init() method but ignored in upsert(). When upserting to a non-existent collection, it was created with default settings instead of user-specified ones. Fixes FlowiseAI#5030 Signed-off-by: majiayu000 <1835304752@qq.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug in the Qdrant vector store integration where custom collection configurations, such as Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly identifies and addresses the issue of qdrantCollectionConfiguration being ignored during upsert. The approach of mirroring the logic from the init method is sound for consistency. However, this change introduces a critical vulnerability: providing an invalid JSON string as configuration will cause an unhandled exception and crash the process. My review includes a critical comment with a suggested fix to move the new logic inside the existing try...catch block. Additionally, I've provided feedback on the new tests. While they cover the happy paths, they use an inaccurate mock for parseJsonBody that masks the aforementioned crash and are tightly coupled to the implementation, which could lead to maintenance issues. Addressing these points will make the fix more robust and the tests more reliable.
Ensures invalid JSON in collection configuration is caught by the existing try/catch instead of crashing the process. Signed-off-by: majiayu000 <1835304752@qq.com>
- Fix parseJsonBody mock to throw on invalid JSON, matching real behavior - Add type annotations to fix TS compilation errors in tests - Add test case for invalid JSON to verify try/catch error handling Signed-off-by: majiayu000 <1835304752@qq.com>
Summary
Fixes #5030
The "Additional Collection Configuration" parameter (
qdrantCollectionConfiguration) was properly applied in theinit()method but completely ignored in theupsert()method. When upserting documents to a non-existent collection, the collection was created with default Qdrant settings (e.g.,replication_factor: 1) instead of user-specified values (e.g.,replication_factor: 3,shard_number: 4).This fix extracts and merges
qdrantCollectionConfigurationintodbConfig.collectionConfigin theupsert()method, using the same pattern already used ininit().Changes
packages/components/nodes/vectorstores/Qdrant/Qdrant.ts: ExtractqdrantCollectionConfigurationinupsert()and merge it intodbConfig.collectionConfigpackages/components/nodes/vectorstores/Qdrant/Qdrant.test.ts: Add 4 tests covering JSON string config, object config, no config (defaults), and vectors mergingTest plan
shard_numberandreplication_factorare included incollectionConfigsizeanddistancesettings are preserved alongside collection config