Skip to content

fix: parse asyncConfig from replication config GET response#1996

Merged
jfrancoa merged 5 commits intomainfrom
jose/fix-async-repl-config-get
Mar 27, 2026
Merged

fix: parse asyncConfig from replication config GET response#1996
jfrancoa merged 5 commits intomainfrom
jose/fix-async-repl-config-get

Conversation

@jfrancoa
Copy link
Copy Markdown
Contributor

Summary

  • The server returns asyncConfig inside replicationConfig on collection GET, but the Python client was silently dropping it — replication_config.async_config was always missing.
  • Add _AsyncReplicationConfig dataclass (14 optional int fields matching the server schema) and wire it into _collection_config_from_json.
  • Export AsyncReplicationConfig from weaviate.outputs.config so users can type-check against it.

Test plan

  • Unit test: parsing replication config without asyncConfig (backwards compat)
  • Unit test: parsing replication config with full asyncConfig
  • Unit test: parsing replication config with partial asyncConfig (missing fields → None)
  • Unit test: to_dict() round-trip includes asyncConfig when present
  • Unit test: to_dict() omits asyncConfig when None
  • Pre-commit hooks pass (ruff, pyright, stubs-autogen, docstrings)

🤖 Generated with Claude Code

The server returns asyncConfig inside replicationConfig on collection
GET, but the Python client was not parsing it. Add _AsyncReplicationConfig
dataclass, wire it into _collection_config_from_json, and export
AsyncReplicationConfig from the public API surface.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jfrancoa jfrancoa requested a review from Copilot March 26, 2026 13:43
Copy link
Copy Markdown

@orca-security-eu orca-security-eu bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed SAST high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

@jfrancoa jfrancoa requested review from bevzzz and tsmith023 March 26, 2026 13:45
Existing callers construct _ReplicationConfig without async_config.
Adding a default prevents a breaking positional argument error.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes the Python client’s parsing of collection replicationConfig so the server-provided asyncConfig is preserved (instead of being silently dropped), and exposes the parsed type for user type-checking.

Changes:

  • Add _AsyncReplicationConfig / AsyncReplicationConfig output dataclass and attach it to ReplicationConfig as async_config.
  • Parse replicationConfig.asyncConfig in _collection_config_from_json.
  • Add unit tests covering presence/absence/partial asyncConfig and to_dict() behavior; export AsyncReplicationConfig via weaviate.outputs.config.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
weaviate/outputs/config.py Re-exports AsyncReplicationConfig for public typing.
weaviate/collections/classes/config_methods.py Parses replicationConfig.asyncConfig into _AsyncReplicationConfig during collection GET config parsing.
weaviate/collections/classes/config.py Introduces the _AsyncReplicationConfig dataclass and adds async_config to _ReplicationConfig.
test/collection/test_config.py Adds tests validating parsing and to_dict() serialization for asyncConfig.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Default async_config to None for backwards compatibility (copilot)
- Use `is not None` instead of truthiness check for asyncConfig
  parsing so empty dict is not silently dropped (copilot)
- Replace unit tests calling _collection_config_from_json with
  integration round-trip tests (dirkkul)
- Add integration test for creating with async_config then removing it

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jfrancoa jfrancoa force-pushed the jose/fix-async-repl-config-get branch 2 times, most recently from 4bff06b to e5ec450 Compare March 26, 2026 15:37
Override merge_with_existing on _ReplicationConfigUpdate to:
- Replace entire asyncConfig when provided (not merge), so omitted
  fields revert to server defaults. This follows the generative/reranker
  clear+replace pattern.
- Pop asyncConfig from schema when asyncEnabled=False, so disabling
  async replication also clears the config.

Add unit tests for all merge behaviors (replace, clear, preserve, reset)
and integration tests for unsetting individual fields, resetting all
fields via empty async_config(), and disabling async replication.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jfrancoa jfrancoa force-pushed the jose/fix-async-repl-config-get branch from e5ec450 to 7050230 Compare March 26, 2026 16:54
@jfrancoa jfrancoa requested a review from Copilot March 26, 2026 16:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Revert to truthiness check when parsing asyncConfig from server
response. An empty dict {} means no custom config and should be
treated as absent (None), not as an _AsyncReplicationConfig with
all None fields.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.26027% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.95%. Comparing base (ccf4f61) to head (53c9687).
⚠️ Report is 68 commits behind head on main.

Files with missing lines Patch % Lines
integration/test_collection_config.py 94.20% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1996      +/-   ##
==========================================
+ Coverage   87.84%   87.95%   +0.11%     
==========================================
  Files         280      280              
  Lines       21347    21652     +305     
==========================================
+ Hits        18752    19045     +293     
- Misses       2595     2607      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jfrancoa jfrancoa merged commit bced996 into main Mar 27, 2026
216 of 242 checks passed
@jfrancoa jfrancoa deleted the jose/fix-async-repl-config-get branch March 27, 2026 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants