CAMEL-23482: Upgrade Weaviate client from v5 to v6#24405
Conversation
Migrate camel-weaviate from io.weaviate:client 5.x to io.weaviate:client6 6.2.1. This changes the client API from v5 Result<T> wrappers to direct v6 types, adds gRPC connection support required by the v6 client, deprecates proxy configuration (unsupported in v6), and fixes the autowired client lifecycle to follow the Milvus/Qdrant closeClient pattern. Co-Authored-By: Otavio Rodolfo Piske <angusyoung@gmail.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
🧪 CI tested the following changed modules:
✅ POM dependency changes: targeted tests included Changed properties: weaviate-client-version Modules affected by dependency changes (1)
🔬 Scalpel shadow comparison — Scalpel: 17 tested, 25 compile-only — current: 13 all testedMaveniverse Scalpel detected 42 affected modules (current approach: 13).
|
gnodet
left a comment
There was a problem hiding this comment.
Nice work on this migration! The upgrade from Weaviate client v5 to v6 is well-executed with comprehensive documentation. A few observations:
Minor observations:
-
grpcPortfield not initialized with default value — Theschemefield follows the patternprivate String scheme = "http"with both a field initializer and@UriParam(defaultValue = "http"). ButgrpcPortonly has@UriParam(defaultValue = "50051")withoutprivate Integer grpcPort = 50051. If someone callsgetGrpcPort()on the configuration directly, they getnulleven though the documented default is 50051. The null is handled increateClient(), so this works at runtime, but the inconsistency could confuse users of the configuration API. -
Endpoint URI uses lowercase
test-collectionin tests —WeaviateContainerIT.getUri()still returnsweaviate:test-collection(lowercase) while the upgrade guide states Weaviate v6 requires PascalCase collection names. This works because all tests override via theCOLLECTION_NAMEheader withWeaviateITCollection, but it's inconsistent with the documented v6 requirement.
Questions:
- The
<classifier>all</classifier>onio.weaviate:client6produces a shaded/uber JAR. Could this cause classpath conflicts with other Camel components if any transitive dependencies (e.g., gRPC, Netty, protobuf) overlap? The comment explains it's required for gRPC's ServiceLoader, which is a valid reason, but worth verifying.
Positive highlights:
- Excellent upgrade guide with the response type change table
- Clean code improvements: extracted
resolveCollectionName(),toPrimitiveFloatArray(), proper double-checked locking withvolatile, client lifecycle withcloseClientflag - Proper deprecation handling for proxy fields
- Bug fix: replaced incorrect
QdrantAction.CREATE_COLLECTIONwithWeaviateVectorDbAction.CREATE_COLLECTIONin langchain4j embeddings IT - Test infra properly extended for gRPC port
This review does not replace specialized AI review tools (CodeRabbit, Sourcery) or static analysis (SonarCloud).
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Jiri Ondrusek
|
|
||
| @Deprecated | ||
| @Metadata(label = "producer", | ||
| description = "Proxy port to connect to weaviate through") |
There was a problem hiding this comment.
Minor consistency suggestion: The scheme field is initialized with private String scheme = "http" (matching its @UriParam(defaultValue = "http")), but grpcPort only declares the default in the annotation without a field initializer. This means getGrpcPort() returns null instead of 50051 until createClient() applies the fallback. Consider initializing the field:
| description = "Proxy port to connect to weaviate through") | |
| private Integer grpcPort = 50051; |
| <dependency> | ||
| <groupId>io.weaviate</groupId> | ||
| <artifactId>client</artifactId> | ||
| <artifactId>client6</artifactId> |
There was a problem hiding this comment.
Question: The all classifier produces a shaded/uber JAR that bundles gRPC, Netty, protobuf, etc. In a large framework like Camel where many components share these transitive dependencies, is there a risk of classpath conflicts? The comment explains why it's needed (gRPC ServiceLoader), but it would be good to confirm no version collisions occur with other components.
orpiske
left a comment
There was a problem hiding this comment.
PR Review: CAMEL-23482 — Upgrade Weaviate client from v5 to v6
This is a well-executed major dependency upgrade from io.weaviate:client 5.x to io.weaviate:client6 6.2.1. The scope is appropriate, the upgrade guide is thorough, and the test coverage is complete.
Positive Observations
- Thorough upgrade guide — Documents every breaking change with a clear migration table.
- Proper deprecation pattern — Proxy options are correctly
@Deprecatedrather than removed. - Sound concurrency — DCL in
getClient()correctly usesvolatile+ReentrantLock. ThecloseClientflag properly distinguishes autowired clients from internally-created ones. - Good DRY improvement —
resolveCollectionName()replaces 6 copies of the same header-or-fallback pattern. - Incidental bug fix —
LangChain4jEmbeddingsComponentWeaviateTargetITwas usingQdrantAction.CREATE_COLLECTIONinstead ofWeaviateVectorDbAction.CREATE_COLLECTION. classifier=alldocumented — Comment explains why the uber-jar classifier is needed.- Test-infra extended cleanly — gRPC port added across all infra service implementations.
Minor Note
The test URI path in WeaviateContainerIT is still "weaviate:test-collection" (lowercase). Since the header always overrides the collection name with "WeaviateITCollection" (PascalCase), this works. But it's inconsistent with the upgrade guide's note that v6 requires PascalCase collection names.
Overall: solid, well-documented migration. No blocking issues found.
This review does not replace specialized AI review tools (CodeRabbit, Sourcery) or static analysis (SonarCloud).
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Otavio Rodolfo Piske
|
|
||
| @Metadata(label = "producer", | ||
| description = "gRPC port for Weaviate server connection", defaultValue = "50051") | ||
| @UriParam(defaultValue = "50051") |
There was a problem hiding this comment.
Nit: The field defaults to null at the Java level but is documented as defaulting to 50051 (via @UriParam/@Metadata). The runtime fallback in createClient() handles this correctly, but initializing the field as private Integer grpcPort = 50051; would be more consistent — similar to how scheme was initialized to "http" in this same PR.
| @UriParam(defaultValue = "50051") | |
| private Integer grpcPort = 50051; |
Summary
io.weaviate:client5.x toio.weaviate:client66.2.1Test plan
WeaviateComponentConfigurationTest— 2 tests)WeaviateContainerIT— 7 tests via Testcontainers)LangChain4jEmbeddingsComponentWeaviateTargetIT— 5 tests)Claude Code on behalf of Jiri Ondrusek
🤖 Generated with Claude Code