Skip to content

Commit 5985492

Browse files
jfrancoaclaude
andcommitted
Address PR review feedback (round 3)
- Use ASYNC_REPLICATION_CONFIG_RESET constant in parser instead of literal - Remove unused async_replication_config field from CreateCollectionDefaults and UpdateCollectionDefaults - Reject --async_replication_config 'reset' on create (no semantics for new collections) - Tighten type annotations: tuple -> Tuple[str, ...] - Drop trivially-dead `if result else None` return - Add unit tests for old-version warning path (create + update) - Add unit test for update reset path asserting Reconfigure.Replication.async_config() is invoked with no kwargs Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ec1435e commit 5985492

5 files changed

Lines changed: 109 additions & 11 deletions

File tree

test/unittests/test_managers/test_collection_manager.py

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1020,3 +1020,97 @@ def test_update_collection_async_replication_config_rejected_when_async_false(
10201020
)
10211021

10221022
mock_collections.get.return_value.config.update.assert_not_called()
1023+
1024+
1025+
def test_create_collection_async_replication_config_warns_on_old_version(
1026+
mock_client, mock_wvc_object_ttl, capsys
1027+
):
1028+
"""Warn when async_replication_config is used against a server older than v1.34.18."""
1029+
mock_collections = MagicMock()
1030+
mock_client.collections = mock_collections
1031+
mock_collections.exists.side_effect = [False, True]
1032+
mock_client.get_meta.return_value = {"version": "1.34.0"}
1033+
1034+
manager = CollectionManager(mock_client)
1035+
1036+
manager.create_collection(
1037+
collection="TestCollection",
1038+
replication_factor=3,
1039+
vector_index="hnsw",
1040+
async_enabled=True,
1041+
async_replication_config={"max_workers": 10},
1042+
)
1043+
1044+
captured = capsys.readouterr()
1045+
assert "Warning: --async_replication_config requires Weaviate >= v1.34.18" in (
1046+
captured.out + captured.err
1047+
)
1048+
mock_collections.create.assert_called_once()
1049+
1050+
1051+
def test_update_collection_async_replication_config_reset(
1052+
mock_client, mock_wvc_object_ttl
1053+
):
1054+
"""Reset (empty dict) calls Reconfigure.Replication.async_config() with no kwargs."""
1055+
mock_collections = MagicMock()
1056+
mock_client.collections = mock_collections
1057+
mock_client.collections.exists.side_effect = [True, True]
1058+
mock_client.get_meta.return_value = {"version": "1.36.0"}
1059+
1060+
mock_collection = MagicMock()
1061+
mock_client.collections.get.return_value = mock_collection
1062+
mock_collection.config.get.return_value = MagicMock(
1063+
replication_config=MagicMock(factor=3),
1064+
multi_tenancy_config=MagicMock(
1065+
enabled=False, auto_tenant_creation=False, auto_tenant_activation=False
1066+
),
1067+
)
1068+
1069+
manager = CollectionManager(mock_client)
1070+
1071+
with patch.object(
1072+
wvc.Reconfigure.Replication,
1073+
"async_config",
1074+
wraps=wvc.Reconfigure.Replication.async_config,
1075+
) as mock_async_config:
1076+
manager.update_collection(
1077+
collection="TestCollection",
1078+
async_replication_config={},
1079+
)
1080+
1081+
mock_async_config.assert_called_once_with()
1082+
mock_collection.config.update.assert_called_once()
1083+
repl_config = mock_collection.config.update.call_args.kwargs["replication_config"]
1084+
assert repl_config.asyncConfig is not None
1085+
1086+
1087+
def test_update_collection_async_replication_config_warns_on_old_version(
1088+
mock_client, mock_wvc_object_ttl, capsys
1089+
):
1090+
"""Warn when async_replication_config is used against a server older than v1.34.18."""
1091+
mock_collections = MagicMock()
1092+
mock_client.collections = mock_collections
1093+
mock_client.collections.exists.side_effect = [True, True]
1094+
mock_client.get_meta.return_value = {"version": "1.34.0"}
1095+
1096+
mock_collection = MagicMock()
1097+
mock_client.collections.get.return_value = mock_collection
1098+
mock_collection.config.get.return_value = MagicMock(
1099+
replication_config=MagicMock(factor=3),
1100+
multi_tenancy_config=MagicMock(
1101+
enabled=False, auto_tenant_creation=False, auto_tenant_activation=False
1102+
),
1103+
)
1104+
1105+
manager = CollectionManager(mock_client)
1106+
1107+
manager.update_collection(
1108+
collection="TestCollection",
1109+
async_replication_config={"max_workers": 10},
1110+
)
1111+
1112+
captured = capsys.readouterr()
1113+
assert "Warning: --async_replication_config requires Weaviate >= v1.34.18" in (
1114+
captured.out + captured.err
1115+
)
1116+
mock_collection.config.update.assert_called_once()

weaviate_cli/commands/create.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import sys
22
import click
3-
from typing import Optional
3+
from typing import Optional, Tuple
44
import json
55

66
from weaviate import WeaviateClient
@@ -256,7 +256,7 @@ def create_collection_cli(
256256
object_ttl_time: Optional[int],
257257
object_ttl_filter_expired: bool,
258258
object_ttl_property_name: Optional[str],
259-
async_replication_config: tuple,
259+
async_replication_config: Tuple[str, ...],
260260
) -> None:
261261
"""Create a collection in Weaviate."""
262262

@@ -272,6 +272,11 @@ def create_collection_cli(
272272
sys.exit(1)
273273
client = None
274274
try:
275+
parsed_async_config = parse_async_replication_config(async_replication_config)
276+
if parsed_async_config == {}:
277+
raise click.UsageError(
278+
"--async_replication_config 'reset' is only supported on update, not create."
279+
)
275280
client = get_client_from_context(ctx)
276281
# Call the function from create_collection.py passing both general and specific arguments
277282
collection_man = CollectionManager(client)
@@ -302,9 +307,7 @@ def create_collection_cli(
302307
object_ttl_time=object_ttl_time,
303308
object_ttl_filter_expired=object_ttl_filter_expired,
304309
object_ttl_property_name=object_ttl_property_name,
305-
async_replication_config=parse_async_replication_config(
306-
async_replication_config
307-
),
310+
async_replication_config=parsed_async_config,
308311
)
309312
except Exception as e:
310313
click.echo(f"Error: {e}")

weaviate_cli/commands/update.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import sys
22
import click
33
import json
4-
from typing import Optional, Union
4+
from typing import Optional, Tuple, Union
55

66
from weaviate_cli.completion.complete import collection_name_complete
77
from weaviate_cli.managers.alias_manager import AliasManager
@@ -136,7 +136,7 @@ def update_collection_cli(
136136
object_ttl_time: Optional[int],
137137
object_ttl_filter_expired: bool,
138138
object_ttl_property_name: Optional[str],
139-
async_replication_config: tuple,
139+
async_replication_config: Tuple[str, ...],
140140
) -> None:
141141
"""Update a collection in Weaviate."""
142142

weaviate_cli/defaults.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ class CreateCollectionDefaults:
8787
object_ttl_time: Optional[int] = None
8888
object_ttl_filter_expired: Optional[bool] = None
8989
object_ttl_property_name: str = "releaseDate"
90-
async_replication_config: Optional[tuple] = None
9190

9291

9392
@dataclass
@@ -264,7 +263,6 @@ class UpdateCollectionDefaults:
264263
object_ttl_time: Optional[int] = None
265264
object_ttl_filter_expired: Optional[bool] = None
266265
object_ttl_property_name: str = "releaseDate"
267-
async_replication_config: Optional[tuple] = None
268266

269267

270268
@dataclass

weaviate_cli/utils.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,10 @@ def parse_async_replication_config(
164164
if not config_tuples:
165165
return None
166166

167-
if len(config_tuples) == 1 and config_tuples[0].strip().lower() == "reset":
167+
if (
168+
len(config_tuples) == 1
169+
and config_tuples[0].strip().lower() == ASYNC_REPLICATION_CONFIG_RESET
170+
):
168171
return {}
169172

170173
result = {}
@@ -190,7 +193,7 @@ def parse_async_replication_config(
190193
f"Invalid value for '{key}': '{value}'. Must be an integer."
191194
)
192195

193-
return result if result else None
196+
return result
194197

195198

196199
def parse_permission(perm: str) -> PermissionsCreateType:

0 commit comments

Comments
 (0)