fix: serialize WeaviateDocumentStore grpc_port and grpc_secure#3453
Conversation
WeaviateDocumentStore.to_dict() serialized url, collection_settings, auth, headers and the embedded/additional config, but omitted grpc_port and grpc_secure. Both are real constructor parameters used to build the connection (connect_to_custom / connect_to_local), so they were silently lost on a to_dict/from_dict round-trip (e.g. a pipeline dumped to YAML and reloaded), and a reloaded store fell back to the defaults (50051 / False). Add both to the to_dict() payload (from_dict already forwards init params via default_from_dict) plus a round-trip regression test; the existing to_dict assertion is updated with the two default values.
github-actions
Bot
added
integration:weaviate
type:documentation
|
Thanks for the PR @ly-wang19! The unit tests are failing currently, could you make sure to fix them? |
The bm25/embedding/hybrid retriever serialization tests embed the WeaviateDocumentStore.to_dict() output inside their own expected dict. Now that to_dict() serializes grpc_port and grpc_secure, the embedded document_store init_parameters in those fixtures must include them too, otherwise the retriever to_dict assertions fail (the Python 3.10/3.14 CI failures reported on this PR).
|
Thanks @bogdankostic — fixed in 0b83a18. The failures came from the retriever serialization tests ( |
Coverage report (weaviate)
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
bogdankostic
left a comment
bogdankostic
left a comment
There was a problem hiding this comment.
Looks good, thanks @ly-wang19!
2 participants
Related Issues
None — found while reviewing
to_dict/from_dictparity across the document stores.Proposed Changes:
WeaviateDocumentStore.to_dict()omittedgrpc_portandgrpc_secure, so they were lost on ato_dict/from_dictround-trip and a reloaded store fell back to the defaults (50051/False). Added both toto_dict();from_dictalready forwards them viadefault_from_dict.How did you test it?
Added a round-trip regression test with non-default values (
grpc_port=50052,grpc_secure=True) — fails before the change, passes after. Updated the existingtest_to_dictassertion. Non-integration tests +ruffpass.Notes for the reviewer
One-line change in
to_dict; nofrom_dictchange needed.Checklist
fix: