-
Notifications
You must be signed in to change notification settings - Fork 177
new: update grpc #1113
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
new: update grpc #1113
Conversation
✅ Deploy Preview for poetic-froyo-8baba7 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR extracts many point-identification, filtering, and geospatial message types into a new common.proto (and generated common_pb2 / common_pb2.pyi / common_pb2_grpc), updates points.proto to import common.proto and use those types, and regenerates related gRPC Python artifacts. collections.proto is updated to add a ReplicatePoints message and to include an optional replicate_points operation in UpdateCollectionClusterSetupRequest. VectorParams in collections.proto gains several optional fields (hnsw_config, quantization_config, on_disk, datatype, multivector_config). Generated .py and .pyi files and qdrant_client/grpc/init.py exports are updated accordingly. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🪛 Ruff (0.14.4)qdrant_client/grpc/__init__.py3-3: (F403) ⏰ 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)
🔇 Additional comments (1)
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 |
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
qdrant_client/grpc/points_pb2.pyi (1)
1-8: Verify backward compatibility for moved types (PointId,Filter,Condition,GeoPoint)Type references to
points_pb2.PointId,points_pb2.Filter,points_pb2.Condition, andpoints_pb2.GeoPointwill break. These types are no longer exposed at thepoints_pb2module level—they've moved tocommon_pb2without re-exports. Code usingfrom qdrant_client.grpc.points_pb2 import PointIdwill fail.Confirm whether this breaking change is intentional. If backward compatibility is required, add module-level re-exports (e.g.,
PointId = common_pb2.PointId) to the runtime.pyfile and corresponding type aliases to this stub.
🧹 Nitpick comments (3)
qdrant_client/proto/common.proto (1)
1-142: Common filter/geo schema is consistent; only Buf package-directory rule may need attentionThe shared
PointId, filter/condition tree, geo types, ranges, and value-count messages are well-structured and internally consistent: field numbers are clean, recursive references (Filter↔Condition,NestedCondition) are valid, and everything needed by other protos (e.g.,Filter,GeoPoint,GeoPolygon,ValuesCount) is defined here.Static analysis (Buf) complains about
package qdrantnot matching the directory (qdrant_client/proto). That pattern matches your existing protos, so it’s not a new semantic problem, but if Buf is part of CI you may want to either adjust itsPACKAGE_DIRECTORY_MATCHconfig or align paths/packages to avoid new CI noise from this file.qdrant_client/proto/collections.proto (1)
23-27: VectorParams extensions and MultiVectorConfig look coherent
- Extending
VectorParamswithhnsw_config,quantization_config,on_disk,datatype, andmultivector_configis consistent with how similar settings are already modeled elsewhere (e.g., collection-level configs), and field numbers 3–7 avoid conflicts.- Adding
on_disktoVectorParamsDiff(tag 3) enables toggling on-disk vector storage at the per-vector-config level, which mirrors the base message.- The new
MultiVectorComparatorenum andMultiVectorConfigmessage cleanly model multi-vector search behavior and are wired viaVectorParams.multivector_config.One potential follow-up: if you intend
datatypeandmultivector_configto be mutable via diffs as well, consider extendingVectorParamsDiffin a future change to include them; otherwise the current schema is internally consistent.Also applies to: 31-33, 72-78
qdrant_client/grpc/common_pb2.py (1)
23-25: Ruff E712 on_USE_C_DESCRIPTORSshould be handled via lint config, not by editing generated codeThe
if _descriptor._USE_C_DESCRIPTORS == False:pattern is produced by the protobuf generator. I’d avoid touching this file manually and instead configure Ruff to ignore_pb2.pyfiles (or E712 for this path) so future regenerations don’t reintroduce the warning.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
qdrant_client/grpc/collections_pb2.py(2 hunks)qdrant_client/grpc/collections_pb2.pyi(6 hunks)qdrant_client/grpc/common_pb2.py(1 hunks)qdrant_client/grpc/common_pb2.pyi(1 hunks)qdrant_client/grpc/common_pb2_grpc.py(1 hunks)qdrant_client/grpc/points_pb2.pyi(51 hunks)qdrant_client/proto/collections.proto(4 hunks)qdrant_client/proto/common.proto(1 hunks)qdrant_client/proto/points.proto(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
qdrant_client/grpc/collections_pb2.pyi (3)
qdrant_client/http/models/models.py (2)
ReplicatePoints(2400-2403)Filter(914-924)qdrant_client/grpc/common_pb2.pyi (3)
filter(127-127)filter(216-217)Filter(59-88)qdrant_client/grpc/points_pb2.pyi (61)
filter(1479-1480)filter(1635-1636)filter(1780-1781)filter(1895-1896)filter(2043-2044)filter(2217-2218)filter(2327-2328)filter(2879-2880)filter(2951-2952)filter(3092-3093)filter(3193-3194)filter(3296-3297)filter(4520-4520)HasField(244-244)HasField(282-282)HasField(323-323)HasField(344-344)HasField(366-366)HasField(387-387)HasField(440-440)HasField(487-487)HasField(584-584)HasField(611-611)HasField(656-656)HasField(699-699)HasField(750-750)HasField(798-798)HasField(828-828)HasField(868-868)ClearField(225-225)ClearField(245-245)ClearField(261-261)ClearField(283-283)ClearField(302-302)ClearField(324-324)ClearField(345-345)ClearField(367-367)ClearField(388-388)ClearField(441-441)ClearField(488-488)ClearField(509-509)ClearField(528-528)ClearField(543-543)ClearField(585-585)ClearField(612-612)WhichOneof(246-246)WhichOneof(443-443)WhichOneof(445-445)WhichOneof(447-447)WhichOneof(490-490)WhichOneof(492-492)WhichOneof(494-494)WhichOneof(586-586)WhichOneof(613-613)WhichOneof(659-659)WhichOneof(661-661)WhichOneof(663-663)WhichOneof(665-665)WhichOneof(702-702)WhichOneof(704-704)WhichOneof(706-706)
qdrant_client/grpc/points_pb2.pyi (1)
qdrant_client/grpc/common_pb2.pyi (6)
PointId(21-38)Filter(59-88)filter(127-127)filter(216-217)Condition(110-147)GeoPoint(42-55)
qdrant_client/grpc/common_pb2.pyi (2)
qdrant_client/grpc/collections_pb2.pyi (2)
collections(723-723)filter(2801-2802)qdrant_client/grpc/points_pb2.pyi (22)
filter(1479-1480)filter(1635-1636)filter(1780-1781)filter(1895-1896)filter(2043-2044)filter(2217-2218)filter(2327-2328)filter(2879-2880)filter(2951-2952)filter(3092-3093)filter(3193-3194)filter(3296-3297)filter(4520-4520)points(636-636)points(682-683)points(777-778)points(1011-1012)points(3426-3426)points(3584-3585)points(3638-3639)points(3659-3660)points(4518-4518)
🪛 Buf (1.59.0)
qdrant_client/proto/common.proto
2-2: Files with package "qdrant" must be within a directory "qdrant" relative to root but were in directory "qdrant_client/proto".
(PACKAGE_DIRECTORY_MATCH)
🪛 Ruff (0.14.4)
qdrant_client/grpc/common_pb2.py
23-23: Avoid equality comparisons to False; use not _descriptor._USE_C_DESCRIPTORS: for false checks
Replace with not _descriptor._USE_C_DESCRIPTORS
(E712)
⏰ 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). (3)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Pages changed - poetic-froyo-8baba7
🔇 Additional comments (14)
qdrant_client/grpc/common_pb2.pyi (1)
1-561: Stubs correctly mirror the common proto schemaThe type stubs line up with the new
common.protomessages (fields, oneofs, and container types), including presence semantics foroptionalfields and the geo/filter tree. I don’t see mismatches or typing issues here.qdrant_client/grpc/common_pb2_grpc.py (1)
1-4: Minimal _grpc module is appropriate for a proto with no servicesGiven that
common.protodefines only messages and no services, this minimal generated module is expected and safe; it just ensures imports ofqdrant_client.grpc.common_pb2_grpcwork.qdrant_client/grpc/collections_pb2.py (2)
15-16: New common_pb2 import correctly wires in shared typesAdding
from . import common_pb2 as common__pb2is necessary so that thecollectionsdescriptors can reference shared messages (e.g.,Filter) fromcommon.proto. This aligns with the new proto imports and looks correct.
19-268: Regenerated descriptors align with updated collections.protoThe serialized
DESCRIPTORblob now importscommon.protoand includes the extendedVectorParams/VectorParamsDiff,SparseIndexConfig.datatype, the newReplicatePointsmessage, and thereplicate_pointsbranch ofUpdateCollectionClusterSetupRequest.operation. The_globals[...]_serialized_start/_endupdates are consistent with those structural changes. No manual fixes needed here.qdrant_client/proto/collections.proto (3)
7-7: Importing common.proto to use shared Filter/Point primitives is appropriateThe new
import "common.proto";and the added section header comments make sense: collections now rely on shared query primitives (e.g.,Filter) defined incommon.proto, and the comments help delineate the collection config section without changing behavior.Also applies to: 16-18
217-230: SparseIndexConfig.datatype is consistent with the new Datatype enumAdding
optional Datatype datatype = 3;toSparseIndexConfigreuses the sameDatatypeenum introduced for vectors, which keeps type choices coherent across dense and sparse indexes. Tag 3 is unused previously, so compatibility is preserved.
732-736: ReplicatePoints and new cluster operation integrate cleanly
ReplicatePointswithfrom_shard_key,to_shard_key, and optionalFilter filterprovides a clear, targeted way to replicate only matching points between shard keys.- The new
ReplicatePoints replicate_points = 10;branch inUpdateCollectionClusterSetupRequest.operationuses a fresh tag within the oneof and does not collide with existing operations.Filterhere comes fromcommon.proto(imported at the top), matching the shared filter schema used by other APIs.The semantics and wiring into the cluster setup request look sound.
Also applies to: 772-773
qdrant_client/grpc/collections_pb2.pyi (3)
7-7: Newcommon_pb2import is appropriate for shared filter typesBringing in
common_pb2here is consistent with existing imports likejson_with_int_pb2and is required for the newReplicatePoints.filter: common_pb2.Filtertyping below. No issues from a typing or packaging perspective.
342-346: VectorParams header comment is a safe, non‑behavioral improvementThe added comment block around
VectorParamssimply clarifies that this section pertains to collection config; it doesn’t affect the generated typing surface and is fine to keep.
2888-2942: ReplicatePoints message and cluster setup extension look consistent and type‑sound
- New
ReplicatePointsmirrors the HTTP model (from_shard_key,to_shard_key, optionalfilter) and reuses the existingShardKeytype plus the sharedcommon_pb2.Filter, which keeps HTTP and gRPC models aligned.- The corresponding
global___ReplicatePointsalias matches the pattern used throughout this stub.UpdateCollectionClusterSetupRequest:
- Adds
REPLICATE_POINTS_FIELD_NUMBERand areplicate_pointsproperty/ctor parameter of typeglobal___ReplicatePoints.- Extends
HasField,ClearField, and the"operation"oneof to include"replicate_points", so this new operation participates in the same mutually exclusive semantics as the existing cluster operations.Overall this is a clean extension of the API surface and should integrate smoothly with existing code that dispatches on the
"operation"oneof.qdrant_client/proto/points.proto (1)
6-8: Importingcommon.protocorrectly centralizes shared primitivesAdding
import "common.proto";is necessary now thatPointId,Filter,Condition,GeoPoint, and related messages are defined in the shared proto. Since both files usepackage qdrant;, all existing references (e.g.,Filter filter = 3;,PointId id = 1;) continue to resolve to the same fully qualified types, preserving wire compatibility while avoiding duplication.qdrant_client/grpc/points_pb2.pyi (3)
5-8: Importingcommon_pb2is correct and consistent with existing generated importsThe new
import common_pb2follows the same pattern ascollections_pb2/json_with_int_pb2and cleanly supports the subsequent type references.
547-583: PointId usage correctly migrated tocommon_pb2.PointIdacross request/response shapesAll updated PointId-bearing fields (
VectorInput.id,PointVectors.id,GetPoints.ids,ScrollPoints.offset,Recommend*/SearchMatrixPair/SearchMatrixOffsets,ScrollResponse.next_page_offset,RetrievedPoint.id,PointsIdsList.ids,PointStruct.id,ScoredPoint.id) consistently referencecommon_pb2.PointIdin both property types and constructor parameters, matching the centralized definition incommon_pb2.pyi. I don’t see any remaining references to a localPointId/global___PointIdin this stub.Also applies to: 811-827, 710-749, 1768-1817, 3366-3379, 3402-3411, 4161-4173, 4200-4245, 4533-4543, 4548-4588, 3827-3883
623-657: Filter/Condition/GeoPoint centralization tocommon_pb2looks consistentAll
Filter-typed fields and constructor params (update operations, search/recommend/query/scroll/facet/matrix selectors,PointsSelector.filter) now consistently usecommon_pb2.Filter. Likewise,Expression.conditionnow usescommon_pb2.ConditionandGeoDistance.originusescommon_pb2.GeoPoint. These line up with the corresponding message definitions incommon_pb2.pyi, and I don’t see any leftover references to the old local types in this stub.Also applies to: 763-799, 1462-1525, 1613-1688, 1768-1819, 1865-1956, 2011-2111, 2191-2263, 2315-2358, 2862-2904, 2922-2999, 3061-3147, 3177-3232, 3282-3325, 3416-3441, 3577-3600, 4507-4528, 4533-4543, 2486-2582, 2589-2601
* new: update grpc * fix: make common_pb2 available under grpc
No description provided.