new: expose cluster operations#1114
Conversation
✅ Deploy Preview for poetic-froyo-8baba7 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds a new cluster_collection_update API across sync and async client layers: base classes (QdrantBase / AsyncQdrantBase), client wrappers (QdrantClient / AsyncQdrantClient), remote implementations (QdrantRemote / AsyncQdrantRemote) which call the REST distributed API, and local implementations (QdrantLocal / AsyncQdrantLocal) that raise NotImplementedError. Introduces a public type alias ClusterOperations. Consolidates several oneOf enums into canonical ShardTransferMethod and ReshardingDirection enums in HTTP models and the embed inspection cache. Adds an integration test test_cluster_collection_update exercising sharded collection operations. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
qdrant_client/local/qdrant_local.py (1)
1013-1023: Nit: tweak error message grammar.Use “updates are” and add a period for consistency with nearby messages.
) -> bool: raise NotImplementedError( - "Cluster collection updates is not supported in the local Qdrant. " - "Please use server Qdrant if you need a cluster" + "Cluster collection updates are not supported in the local Qdrant. " + "Please use server Qdrant if you need a cluster." )qdrant_client/local/async_qdrant_local.py (1)
945-954: Nit: tweak error message grammar.Match the sync local message and fix “updates is” -> “updates are”.
) -> bool: raise NotImplementedError( - "Cluster collection updates is not supported in the local Qdrant. Please use server Qdrant if you need a cluster" + "Cluster collection updates are not supported in the local Qdrant. Please use server Qdrant if you need a cluster." )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
qdrant_client/async_client_base.py(1 hunks)qdrant_client/async_qdrant_client.py(1 hunks)qdrant_client/async_qdrant_remote.py(1 hunks)qdrant_client/client_base.py(1 hunks)qdrant_client/conversions/common_types.py(1 hunks)qdrant_client/embed/_inspection_cache.py(5 hunks)qdrant_client/http/models/models.py(2 hunks)qdrant_client/local/async_qdrant_local.py(1 hunks)qdrant_client/local/qdrant_local.py(1 hunks)qdrant_client/qdrant_client.py(1 hunks)qdrant_client/qdrant_remote.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
qdrant_client/qdrant_remote.py (8)
qdrant_client/async_client_base.py (1)
cluster_collection_update(371-374)qdrant_client/async_qdrant_client.py (1)
cluster_collection_update(2290-2303)qdrant_client/async_qdrant_remote.py (2)
cluster_collection_update(2398-2409)rest(360-366)qdrant_client/client_base.py (1)
cluster_collection_update(398-404)qdrant_client/local/async_qdrant_local.py (1)
cluster_collection_update(945-954)qdrant_client/local/qdrant_local.py (1)
cluster_collection_update(1013-1023)qdrant_client/qdrant_client.py (1)
cluster_collection_update(2419-2433)qdrant_client/http/api/distributed_api.py (2)
update_collection_cluster(278-288)update_collection_cluster(355-365)
qdrant_client/async_qdrant_remote.py (5)
qdrant_client/async_client_base.py (1)
cluster_collection_update(371-374)qdrant_client/async_qdrant_client.py (1)
cluster_collection_update(2290-2303)qdrant_client/local/async_qdrant_local.py (1)
cluster_collection_update(945-954)qdrant_client/qdrant_remote.py (2)
cluster_collection_update(2584-2595)rest(411-417)qdrant_client/http/api/distributed_api.py (2)
update_collection_cluster(278-288)update_collection_cluster(355-365)
qdrant_client/async_client_base.py (7)
qdrant_client/async_qdrant_client.py (1)
cluster_collection_update(2290-2303)qdrant_client/async_qdrant_remote.py (1)
cluster_collection_update(2398-2409)qdrant_client/client_base.py (1)
cluster_collection_update(398-404)qdrant_client/local/async_qdrant_local.py (1)
cluster_collection_update(945-954)qdrant_client/local/qdrant_local.py (1)
cluster_collection_update(1013-1023)qdrant_client/qdrant_client.py (1)
cluster_collection_update(2419-2433)qdrant_client/qdrant_remote.py (1)
cluster_collection_update(2584-2595)
qdrant_client/qdrant_client.py (7)
qdrant_client/async_client_base.py (1)
cluster_collection_update(371-374)qdrant_client/async_qdrant_client.py (1)
cluster_collection_update(2290-2303)qdrant_client/async_qdrant_remote.py (1)
cluster_collection_update(2398-2409)qdrant_client/client_base.py (1)
cluster_collection_update(398-404)qdrant_client/local/async_qdrant_local.py (1)
cluster_collection_update(945-954)qdrant_client/local/qdrant_local.py (1)
cluster_collection_update(1013-1023)qdrant_client/qdrant_remote.py (1)
cluster_collection_update(2584-2595)
qdrant_client/client_base.py (7)
qdrant_client/async_client_base.py (1)
cluster_collection_update(371-374)qdrant_client/async_qdrant_client.py (1)
cluster_collection_update(2290-2303)qdrant_client/async_qdrant_remote.py (1)
cluster_collection_update(2398-2409)qdrant_client/local/async_qdrant_local.py (1)
cluster_collection_update(945-954)qdrant_client/local/qdrant_local.py (1)
cluster_collection_update(1013-1023)qdrant_client/qdrant_client.py (1)
cluster_collection_update(2419-2433)qdrant_client/qdrant_remote.py (1)
cluster_collection_update(2584-2595)
qdrant_client/local/async_qdrant_local.py (7)
qdrant_client/async_client_base.py (1)
cluster_collection_update(371-374)qdrant_client/async_qdrant_client.py (1)
cluster_collection_update(2290-2303)qdrant_client/async_qdrant_remote.py (1)
cluster_collection_update(2398-2409)qdrant_client/client_base.py (1)
cluster_collection_update(398-404)qdrant_client/local/qdrant_local.py (1)
cluster_collection_update(1013-1023)qdrant_client/qdrant_client.py (1)
cluster_collection_update(2419-2433)qdrant_client/qdrant_remote.py (1)
cluster_collection_update(2584-2595)
qdrant_client/local/qdrant_local.py (7)
qdrant_client/async_client_base.py (1)
cluster_collection_update(371-374)qdrant_client/async_qdrant_client.py (1)
cluster_collection_update(2290-2303)qdrant_client/async_qdrant_remote.py (1)
cluster_collection_update(2398-2409)qdrant_client/client_base.py (1)
cluster_collection_update(398-404)qdrant_client/local/async_qdrant_local.py (1)
cluster_collection_update(945-954)qdrant_client/qdrant_client.py (1)
cluster_collection_update(2419-2433)qdrant_client/qdrant_remote.py (1)
cluster_collection_update(2584-2595)
qdrant_client/http/models/models.py (1)
qdrant_client/grpc/collections_pb2.pyi (2)
ReshardingDirection(303-306)ShardTransferMethod(329-329)
qdrant_client/async_qdrant_client.py (7)
qdrant_client/async_client_base.py (1)
cluster_collection_update(371-374)qdrant_client/async_qdrant_remote.py (1)
cluster_collection_update(2398-2409)qdrant_client/client_base.py (1)
cluster_collection_update(398-404)qdrant_client/local/async_qdrant_local.py (1)
cluster_collection_update(945-954)qdrant_client/local/qdrant_local.py (1)
cluster_collection_update(1013-1023)qdrant_client/qdrant_client.py (1)
cluster_collection_update(2419-2433)qdrant_client/qdrant_remote.py (1)
cluster_collection_update(2584-2595)
🪛 Ruff (0.14.4)
qdrant_client/qdrant_remote.py
2589-2589: Unused method argument: kwargs
(ARG002)
qdrant_client/async_qdrant_remote.py
2403-2403: Unused method argument: kwargs
(ARG002)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Pages changed - poetic-froyo-8baba7
- GitHub Check: Python 3.11.x on ubuntu-latest test
- GitHub Check: Python 3.10.x on ubuntu-latest test
- GitHub Check: Python 3.13.x on ubuntu-latest test
- GitHub Check: Python 3.12.x on ubuntu-latest test
- GitHub Check: Python 3.9.x on ubuntu-latest test
🔇 Additional comments (9)
qdrant_client/http/models/models.py (2)
2428-2438: LGTM! Clean enum consolidation.The
ReshardingDirectionenum is well-implemented with clear documentation and follows the established pattern in this file. The consolidation from OneOf variants to a single enum improves type safety and maintainability.
2827-2838: LGTM! Well-documented enum consolidation.The
ShardTransferMethodenum is properly implemented with clear documentation explaining each transfer method. The consolidation from multiple OneOf variants to a unified enum enhances code maintainability and type safety across the codebase.qdrant_client/embed/_inspection_cache.py (2)
3153-3158: LGTM - Schema consolidation improves maintainability.The new
ShardTransferMethodenum consolidates previously fragmented oneOf variants into a single canonical definition. The enum values are well-documented and the references inMoveShard,ReplicateShard, andRestartTransfercorrectly use the centralized definition.Also applies to: 3166-3166, 4009-4009, 4024-4024
4148-4153: LGTM - Clear enum definition for resharding direction.The
ReshardingDirectionenum consolidates the direction options into a well-documented canonical definition with clear "up" (scale up/add shard) and "down" (scale down/remove shard) values. The reference inStartReshardingcorrectly points to this centralized definition.Also applies to: 4157-4157
qdrant_client/conversions/common_types.py (1)
139-139: LGTM - Type alias correctly exposes cluster operations type.The
ClusterOperationstype alias follows the established pattern in this file and properly exposesrest.ClusterOperationsfor use in the new cluster collection update API.qdrant_client/async_client_base.py (1)
371-374: LGTM - Abstract method follows established patterns.The new
cluster_collection_updatemethod correctly extends the abstract base class with appropriate signature andNotImplementedErrordefault implementation. The signature is consistent with other abstract methods in this class and matches the concrete implementations in client classes.Note: This file is autogenerated, so any future modifications should be made in the source template.
qdrant_client/async_qdrant_client.py (1)
2289-2303: Async wrapper for cluster_collection_update is consistent and correctThe method signature, kwargs assertion, and delegation to
self._client.cluster_collection_updatematch the established pattern in this class and align with the async base/remote implementations. No issues from a correctness or API‑shape standpoint.qdrant_client/client_base.py (1)
398-404: Base API extension for cluster_collection_update looks goodThe new
cluster_collection_updatemethod mirrors the pattern of other abstract operations (**kwargs+NotImplementedError) and provides a clean base hook for remote/local implementations. No functional or compatibility concerns.qdrant_client/qdrant_client.py (1)
2418-2433: Sync cluster_collection_update wrapper is correctly wiredThe method cleanly exposes the new cluster operation on
QdrantClient, enforces the usualkwargsdiscipline, and delegates to the underlying client with the expected parameters, includingtimeout. Behavior for local vs remote backends is consistent with the rest of the API surface.
| async def cluster_collection_update( | ||
| self, | ||
| collection_name: str, | ||
| cluster_operation: types.ClusterOperations, | ||
| timeout: Optional[int] = None, | ||
| **kwargs: Any, | ||
| ) -> bool: | ||
| update_result = await self.rest.distributed_api.update_collection_cluster( | ||
| collection_name=collection_name, cluster_operations=cluster_operation, timeout=timeout | ||
| ) | ||
| assert update_result is not None, "Cluster collection update returned None" | ||
| return update_result |
There was a problem hiding this comment.
🧩 Analysis chain
Return type bug: return bool, not InlineResponse200; also consume kwargs.
update_collection_cluster returns a response object with a .result bool. Current code returns the object itself, violating the annotated return type and public API contract. Also, kwargs is unused. Fix below.
async def cluster_collection_update(
self,
collection_name: str,
cluster_operation: types.ClusterOperations,
timeout: Optional[int] = None,
- **kwargs: Any,
+ **kwargs: Any,
) -> bool:
- update_result = await self.rest.distributed_api.update_collection_cluster(
+ # No extra kwargs expected at this layer; keep strict like the client wrapper.
+ assert len(kwargs) == 0, f"Unknown arguments: {list(kwargs.keys())}"
+ update_response = await self.rest.distributed_api.update_collection_cluster(
collection_name=collection_name, cluster_operations=cluster_operation, timeout=timeout
)
- assert update_result is not None, "Cluster collection update returned None"
- return update_result
+ # InlineResponse200 -> bool
+ result = getattr(update_response, "result", None)
+ assert result is not None, "Cluster collection update returned None"
+ return resultNote: This file is autogenerated; please update the generator to emit the corrected pattern. Also consider adding the timeout parameter to the abstract base signatures to match this method’s shape.
I'll verify the review comment by examining the actual code and understanding the return types involved.
---
🏁 Script executed:
```shell
#!/bin/bash
# Step 1: Check the actual code at the specified lines
cat qdrant_client/async_qdrant_remote.py | sed -n '2398,2409p'
Length of output: 518
🏁 Script executed:
#!/bin/bash
# Step 2: Search for similar methods that might use the same pattern
rg -n "distributed_api\." qdrant_client/async_qdrant_remote.py -A 5 -B 2 | head -60Length of output: 1155
🏁 Script executed:
#!/bin/bash
# Step 3: Find InlineResponse200 definition to understand its structure
rg -n "class InlineResponse200" --type python -A 10Length of output: 30
🏁 Script executed:
#!/bin/bash
# Step 4: Check if this method has similar patterns elsewhere or if there's a sync version
rg -n "def cluster_collection_update" qdrant_client/ -A 10Length of output: 6004
🏁 Script executed:
#!/bin/bash
# Step 5: Look for other methods that return bool to understand the pattern
rg -n "-> bool:" qdrant_client/async_qdrant_remote.py -B 5 -A 8 | head -80Length of output: 404
---
🏁 Script executed:
```shell
#!/bin/bash
# Find update_collection_cluster method definition to understand return type
rg -n "def update_collection_cluster" qdrant_client/ -A 15 -B 2
Length of output: 2319
🏁 Script executed:
#!/bin/bash
# Look at more examples of .result extraction in the same file
rg -n "\.result" qdrant_client/async_qdrant_remote.py -B 3 -A 1 | head -100Length of output: 3822
🏁 Script executed:
#!/bin/bash
# Check the sync version in qdrant_remote.py to see if it has the same issue
sed -n '2584,2600p' qdrant_client/qdrant_remote.pyLength of output: 506
🏁 Script executed:
#!/bin/bash
# Search for InlineResponse types to understand the response structure
rg "InlineResponse200|InlineResponse" qdrant_client/ --type-listLength of output: 389
🏁 Script executed:
#!/bin/bash
# Find what's being imported as types to understand response types
rg "from.*models|from.*types" qdrant_client/async_qdrant_remote.py | head -20Length of output: 441
Return type bug confirmed: method returns InlineResponse200 object instead of bool; also missing kwargs assertion.
The underlying API update_collection_cluster returns InlineResponse200 (verified in distributed_api.py:283), and the codebase consistently extracts the .result attribute from response objects (evidenced by numerous .result extractions throughout async_qdrant_remote.py). The method annotates -> bool but currently returns the response object directly, violating the type contract. Additionally, the wrapper classes in qdrant_client.py:2426 and async_qdrant_client.py:2297 both assert len(kwargs) == 0, but the remote implementations skip this validation for consistency.
Both async_qdrant_remote.py:2398-2409 and the sync version qdrant_remote.py:2584-2600 have this identical bug and require the same fix.
async def cluster_collection_update(
self,
collection_name: str,
cluster_operation: types.ClusterOperations,
timeout: Optional[int] = None,
- **kwargs: Any,
+ **kwargs: Any,
) -> bool:
+ assert len(kwargs) == 0, f"Unknown arguments: {list(kwargs.keys())}"
- update_result = await self.rest.distributed_api.update_collection_cluster(
+ update_response = await self.rest.distributed_api.update_collection_cluster(
collection_name=collection_name, cluster_operations=cluster_operation, timeout=timeout
)
- assert update_result is not None, "Cluster collection update returned None"
- return update_result
+ result = update_response.result
+ assert result is not None, "Cluster collection update returned None"
+ return resultNote: File is autogenerated; update generator to emit this pattern. Apply same fix to sync version at qdrant_remote.py:2584-2600.
🧰 Tools
🪛 Ruff (0.14.4)
2403-2403: Unused method argument: kwargs
(ARG002)
🤖 Prompt for AI Agents
In qdrant_client/async_qdrant_remote.py around lines 2398 to 2409, the method is
annotated to return bool but currently returns an InlineResponse200 object and
omits the kwargs validation; change it to assert len(kwargs) == 0, ensure
update_result is not None, then return update_result.result (a bool) instead of
the response object. Apply the identical fix to the sync implementation in
qdrant_remote.py around lines 2584 to 2600 so both remote wrappers match the
other clients' behavior.
| def cluster_collection_update( | ||
| self, | ||
| collection_name: str, | ||
| cluster_operation: types.ClusterOperations, | ||
| timeout: Optional[int] = None, | ||
| **kwargs: Any, | ||
| ) -> bool: | ||
| update_result = self.rest.distributed_api.update_collection_cluster( | ||
| collection_name=collection_name, cluster_operations=cluster_operation, timeout=timeout | ||
| ) | ||
| assert update_result is not None, "Cluster collection update returned None" | ||
| return update_result |
There was a problem hiding this comment.
🧩 Analysis chain
Return type bug and unused kwargs: return the boolean and validate kwargs.
Like the async variant, this currently returns the full InlineResponse200. Return its .result and assert no stray kwargs.
def cluster_collection_update(
self,
collection_name: str,
cluster_operation: types.ClusterOperations,
timeout: Optional[int] = None,
- **kwargs: Any,
+ **kwargs: Any,
) -> bool:
- update_result = self.rest.distributed_api.update_collection_cluster(
+ assert len(kwargs) == 0, f"Unknown arguments: {list(kwargs.keys())}"
+ update_response = self.rest.distributed_api.update_collection_cluster(
collection_name=collection_name, cluster_operations=cluster_operation, timeout=timeout
)
- assert update_result is not None, "Cluster collection update returned None"
- return update_result
+ result = getattr(update_response, "result", None)
+ assert result is not None, "Cluster collection update returned None"
+ return resultAlso consider adding the timeout parameter to the abstract base (QdrantBase) for signature parity with concrete implementations.
🏁 Script executed:
#!/bin/bash
# Find the cluster_collection_update method (both sync and async variants)
rg -n "def cluster_collection_update" --type=pyLength of output: 618
🏁 Script executed:
#!/bin/bash
# Search for update_collection_cluster calls and return type patterns
rg -n "update_collection_cluster" --type=py -A 2 -B 2Length of output: 2740
🏁 Script executed:
#!/bin/bash
# Look for InlineResponse200 imports and usage in the file
rg -n "InlineResponse200" --type=py -C 3Length of output: 116495
🏁 Script executed:
#!/bin/bash
# Check QdrantBase class to see base method signatures
rg -n "class QdrantBase" --type=py -A 30 | head -50Length of output: 1835
🏁 Script executed:
#!/bin/bash
# Find other similar update_ methods in qdrant_remote.py to see pattern
rg -n "def.*_update\(" qdrant_client/qdrant_remote.py --type=py -A 5 | head -60Length of output: 222
🏁 Script executed:
#!/bin/bash
# Check the full async variant implementation
cat qdrant_client/async_qdrant_remote.py | sed -n '2398,2415p'Length of output: 518
🏁 Script executed:
#!/bin/bash
# Look for cluster_collection_update in QdrantBase and AsyncQdrantBase
rg -n "def cluster_collection_update" qdrant_client/client_base.py qdrant_client/async_client_base.py -A 6Length of output: 780
🏁 Script executed:
#!/bin/bash
# Check if other similar methods validate kwargs
rg -n "assert len\(kwargs\)" qdrant_client/qdrant_remote.py -B 3 -A 1 | head -40Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if the current code at 2584-2595 already exists or needs to be created
cat qdrant_client/qdrant_remote.py | sed -n '2584,2595p'Length of output: 506
Fix return type and validate kwargs parameters.
The method currently returns the full InlineResponse200 object instead of extracting the boolean .result field, violating its -> bool signature. The unused **kwargs parameter should be validated:
def cluster_collection_update(
self,
collection_name: str,
cluster_operation: types.ClusterOperations,
timeout: Optional[int] = None,
**kwargs: Any,
) -> bool:
+ assert not kwargs, f"Unexpected keyword arguments: {list(kwargs.keys())}"
- update_result = self.rest.distributed_api.update_collection_cluster(
+ update_response = self.rest.distributed_api.update_collection_cluster(
collection_name=collection_name, cluster_operations=cluster_operation, timeout=timeout
)
- assert update_result is not None, "Cluster collection update returned None"
- return update_result
+ result = update_response.result
+ assert result is not None, "Cluster collection update returned None"
+ return resultAlso apply identical fix to the async variant at async_qdrant_remote.py:2398–2413, and consider adding timeout parameter to base class signatures (client_base.py:398 and async_client_base.py:371) for signature consistency.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def cluster_collection_update( | |
| self, | |
| collection_name: str, | |
| cluster_operation: types.ClusterOperations, | |
| timeout: Optional[int] = None, | |
| **kwargs: Any, | |
| ) -> bool: | |
| update_result = self.rest.distributed_api.update_collection_cluster( | |
| collection_name=collection_name, cluster_operations=cluster_operation, timeout=timeout | |
| ) | |
| assert update_result is not None, "Cluster collection update returned None" | |
| return update_result | |
| def cluster_collection_update( | |
| self, | |
| collection_name: str, | |
| cluster_operation: types.ClusterOperations, | |
| timeout: Optional[int] = None, | |
| **kwargs: Any, | |
| ) -> bool: | |
| assert not kwargs, f"Unexpected keyword arguments: {list(kwargs.keys())}" | |
| update_response = self.rest.distributed_api.update_collection_cluster( | |
| collection_name=collection_name, cluster_operations=cluster_operation, timeout=timeout | |
| ) | |
| result = update_response.result | |
| assert result is not None, "Cluster collection update returned None" | |
| return result |
🧰 Tools
🪛 Ruff (0.14.4)
2589-2589: Unused method argument: kwargs
(ARG002)
🤖 Prompt for AI Agents
In qdrant_client/qdrant_remote.py around lines 2584–2595, the method currently
returns the full InlineResponse200 object and ignores **kwargs; change it to
extract and return the boolean result field (e.g., return update_result.result)
to match the -> bool signature, and validate kwargs by raising a TypeError or
ValueError if any unexpected kwargs are passed (or explicitly accept and
document supported keys); apply the identical change to the async variant in
async_qdrant_remote.py around lines 2398–2413 so it returns the boolean result
and validates kwargs as well; finally, update the base class method signatures
in client_base.py (around line ~398) and async_client_base.py (around line ~371)
to include the timeout parameter for signature consistency.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
qdrant_client/async_qdrant_remote.py (1)
2398-2413: Missing kwargs validation for consistency with wrapper classes.The method accepts
**kwargsbut never validates it. The wrapper class inasync_qdrant_client.py:2297assertslen(kwargs) == 0, so this remote implementation should do the same for consistency and to prevent unexpected arguments from being silently ignored.Based on past review comments, apply this diff (note: file is autogenerated, so update the generator):
async def cluster_collection_update( self, collection_name: str, cluster_operation: types.ClusterOperations, timeout: Optional[int] = None, **kwargs: Any, ) -> bool: + assert len(kwargs) == 0, f"Unknown arguments: {list(kwargs.keys())}" update_result = ( await self.rest.distributed_api.update_collection_cluster( collection_name=collection_name, cluster_operations=cluster_operation, timeout=timeout, ) ).result assert update_result is not None, "Cluster collection update returned None" return update_resultApply the identical fix to the sync version at
qdrant_remote.py:2583-2594.qdrant_client/qdrant_remote.py (1)
2584-2595: Validate unused**kwargsparameter.The
**kwargsparameter is declared but never validated or used, allowing callers to pass invalid arguments that are silently ignored. Other client methods validate this to catch typos.Apply this diff to validate kwargs:
def cluster_collection_update( self, collection_name: str, cluster_operation: types.ClusterOperations, timeout: Optional[int] = None, **kwargs: Any, ) -> bool: + assert not kwargs, f"Unexpected keyword arguments: {list(kwargs.keys())}" update_result = self.rest.distributed_api.update_collection_cluster( collection_name=collection_name, cluster_operations=cluster_operation, timeout=timeout ).result assert update_result is not None, "Cluster collection update returned None" return update_resultAdditionally, consider adding the
timeoutparameter to the base class method signatures inclient_base.py(line 397) andasync_client_base.py(line 370) for signature consistency across the inheritance hierarchy.Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
qdrant_client/async_qdrant_remote.py(1 hunks)qdrant_client/qdrant_remote.py(1 hunks)tests/test_qdrant_client.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/test_qdrant_client.py (3)
tests/utils.py (1)
read_version(9-25)qdrant_client/qdrant_client.py (4)
create_collection(1632-1720)cluster_collection_update(2419-2433)upsert(874-953)scroll(712-780)qdrant_client/local/qdrant_local.py (4)
create_collection(772-803)cluster_collection_update(1013-1023)upsert(543-552)scroll(500-519)
qdrant_client/qdrant_remote.py (8)
qdrant_client/async_qdrant_remote.py (2)
cluster_collection_update(2398-2413)rest(360-366)qdrant_client/client_base.py (1)
cluster_collection_update(398-404)qdrant_client/async_client_base.py (1)
cluster_collection_update(371-374)qdrant_client/async_qdrant_client.py (1)
cluster_collection_update(2290-2303)qdrant_client/local/async_qdrant_local.py (1)
cluster_collection_update(945-954)qdrant_client/local/qdrant_local.py (1)
cluster_collection_update(1013-1023)qdrant_client/qdrant_client.py (1)
cluster_collection_update(2419-2433)qdrant_client/http/api/distributed_api.py (2)
update_collection_cluster(278-288)update_collection_cluster(355-365)
qdrant_client/async_qdrant_remote.py (5)
qdrant_client/qdrant_remote.py (2)
cluster_collection_update(2584-2595)rest(411-417)qdrant_client/client_base.py (1)
cluster_collection_update(398-404)qdrant_client/async_client_base.py (1)
cluster_collection_update(371-374)qdrant_client/async_qdrant_client.py (1)
cluster_collection_update(2290-2303)qdrant_client/http/api/distributed_api.py (2)
update_collection_cluster(278-288)update_collection_cluster(355-365)
🪛 Ruff (0.14.4)
qdrant_client/qdrant_remote.py
2589-2589: Unused method argument: kwargs
(ARG002)
qdrant_client/async_qdrant_remote.py
2403-2403: Unused method argument: kwargs
(ARG002)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Pages changed - poetic-froyo-8baba7
- GitHub Check: Python 3.11.x on ubuntu-latest test
- GitHub Check: Python 3.9.x on ubuntu-latest test
- GitHub Check: Python 3.13.x on ubuntu-latest test
- GitHub Check: Python 3.10.x on ubuntu-latest test
- GitHub Check: Python 3.12.x on ubuntu-latest test
| client.create_collection( | ||
| collection_name=COLLECTION_NAME, | ||
| vectors_config=models.VectorParams(size=DIM, distance=models.Distance.DOT), | ||
| timeout=TIMEOUT, | ||
| sharding_method=models.ShardingMethod.CUSTOM, | ||
| ) | ||
|
|
||
| client.cluster_collection_update( | ||
| COLLECTION_NAME, | ||
| cluster_operation=models.CreateShardingKeyOperation( | ||
| create_sharding_key=models.CreateShardingKey( | ||
| shard_key="fish", | ||
| shards_number=1, | ||
| ) | ||
| ), | ||
| ) | ||
|
|
||
| client.cluster_collection_update( | ||
| COLLECTION_NAME, | ||
| cluster_operation=models.CreateShardingKeyOperation( | ||
| create_sharding_key=models.CreateShardingKey( | ||
| shard_key="lion", | ||
| shards_number=1, | ||
| ) | ||
| ), | ||
| ) | ||
|
|
||
| client.upsert( | ||
| COLLECTION_NAME, | ||
| points=[models.PointStruct(id=1, vector={}), models.PointStruct(id=2, vector={})], | ||
| shard_key_selector="fish", | ||
| ) | ||
| client.upsert( | ||
| COLLECTION_NAME, points=[models.PointStruct(id=3, vector={})], shard_key_selector="lion" | ||
| ) |
There was a problem hiding this comment.
Vector configuration inconsistency may cause test failures.
The collection is created with VectorParams(size=DIM, distance=Distance.DOT) at line 2278, expecting vectors of size 100. However, the upserted points at lines 2305-2306 and 2309 use empty vectors (vector={}). This mismatch may cause validation errors or unexpected behavior.
If you intend to test without vectors, use vectors_config={} when creating the collection (as in test_empty_vector at line 1587). If you need vectors, provide appropriately sized vectors in the points.
Apply this diff to use empty vector config:
client.create_collection(
collection_name=COLLECTION_NAME,
- vectors_config=models.VectorParams(size=DIM, distance=models.Distance.DOT),
+ vectors_config={},
timeout=TIMEOUT,
sharding_method=models.ShardingMethod.CUSTOM,
)Or provide proper vectors:
client.upsert(
COLLECTION_NAME,
- points=[models.PointStruct(id=1, vector={}), models.PointStruct(id=2, vector={})],
+ points=[models.PointStruct(id=1, vector=np.random.rand(DIM).tolist()), models.PointStruct(id=2, vector=np.random.rand(DIM).tolist())],
shard_key_selector="fish",
)
client.upsert(
- COLLECTION_NAME, points=[models.PointStruct(id=3, vector={})], shard_key_selector="lion"
+ COLLECTION_NAME, points=[models.PointStruct(id=3, vector=np.random.rand(DIM).tolist())], shard_key_selector="lion"
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| client.create_collection( | |
| collection_name=COLLECTION_NAME, | |
| vectors_config=models.VectorParams(size=DIM, distance=models.Distance.DOT), | |
| timeout=TIMEOUT, | |
| sharding_method=models.ShardingMethod.CUSTOM, | |
| ) | |
| client.cluster_collection_update( | |
| COLLECTION_NAME, | |
| cluster_operation=models.CreateShardingKeyOperation( | |
| create_sharding_key=models.CreateShardingKey( | |
| shard_key="fish", | |
| shards_number=1, | |
| ) | |
| ), | |
| ) | |
| client.cluster_collection_update( | |
| COLLECTION_NAME, | |
| cluster_operation=models.CreateShardingKeyOperation( | |
| create_sharding_key=models.CreateShardingKey( | |
| shard_key="lion", | |
| shards_number=1, | |
| ) | |
| ), | |
| ) | |
| client.upsert( | |
| COLLECTION_NAME, | |
| points=[models.PointStruct(id=1, vector={}), models.PointStruct(id=2, vector={})], | |
| shard_key_selector="fish", | |
| ) | |
| client.upsert( | |
| COLLECTION_NAME, points=[models.PointStruct(id=3, vector={})], shard_key_selector="lion" | |
| ) | |
| client.create_collection( | |
| collection_name=COLLECTION_NAME, | |
| vectors_config={}, | |
| timeout=TIMEOUT, | |
| sharding_method=models.ShardingMethod.CUSTOM, | |
| ) | |
| client.cluster_collection_update( | |
| COLLECTION_NAME, | |
| cluster_operation=models.CreateShardingKeyOperation( | |
| create_sharding_key=models.CreateShardingKey( | |
| shard_key="fish", | |
| shards_number=1, | |
| ) | |
| ), | |
| ) | |
| client.cluster_collection_update( | |
| COLLECTION_NAME, | |
| cluster_operation=models.CreateShardingKeyOperation( | |
| create_sharding_key=models.CreateShardingKey( | |
| shard_key="lion", | |
| shards_number=1, | |
| ) | |
| ), | |
| ) | |
| client.upsert( | |
| COLLECTION_NAME, | |
| points=[models.PointStruct(id=1, vector={}), models.PointStruct(id=2, vector={})], | |
| shard_key_selector="fish", | |
| ) | |
| client.upsert( | |
| COLLECTION_NAME, points=[models.PointStruct(id=3, vector={})], shard_key_selector="lion" | |
| ) |
🤖 Prompt for AI Agents
In tests/test_qdrant_client.py around lines 2276-2310 the collection is created
with vectors_config=models.VectorParams(size=DIM, distance=models.Distance.DOT)
but the upserted points use empty vectors (vector={}), causing a size/validation
mismatch; fix by either changing the collection creation to use an empty
vectors_config (e.g., vectors_config={}) if you intend to test empty vectors, or
keep the VectorParams and replace the upserted points with vectors of length DIM
(e.g., provide lists of DIM floats for each PointStruct) and ensure
shard_key_selector usage remains correct.
* new: expose cluster operations * fix: fix cluster collection update return result, add tests * fix test --------- Co-authored-by: generall <andrey@vasnetsov.com>
2 participants
tbd: tests