Skip to content

Commit 6bcb4c5

Browse files
committed
fix: address review findings for property reindex API (#2097)
- accept a bare string for the tenants argument of update_property_index and rebuild_property_index, normalizing it to a single-element list so it cannot be exploded into a per-character csv (export API precedent) - override to_dict on _PropertyIndexes/_CollectionPropertyIndexes so the nested dataclass lists serialize to JSON-compatible dicts (_CollectionConfig precedent) - drop the dead list branch for dataType in the index status parser - cover the ReindexFailedError/ReindexCanceledError wait paths of both update and rebuild, pin the empty request body of the rebuild/cancel mocks, and prove json.dumps(...to_dict()) round-trips
1 parent 762e656 commit 6bcb4c5

6 files changed

Lines changed: 162 additions & 33 deletions

File tree

mock_tests/test_property_reindex.py

Lines changed: 121 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@
1313
PropertyIndexTaskStatus,
1414
Tokenization,
1515
)
16-
from weaviate.exceptions import WeaviateUnsupportedFeatureError
16+
from weaviate.exceptions import (
17+
ReindexCanceledError,
18+
ReindexFailedError,
19+
WeaviateUnsupportedFeatureError,
20+
)
1721

1822
COLLECTION = "TestCollection"
1923
SCHEMA_PATH = f"/v1/schema/{COLLECTION}"
@@ -128,12 +132,117 @@ def test_update_property_index_wait_for_completion(
128132
weaviate_139_mock.check_assertions()
129133

130134

135+
def test_update_property_index_bare_str_tenant(
136+
weaviate_139_mock: HTTPServer, client_139: weaviate.WeaviateClient
137+
) -> None:
138+
"""A bare string tenant is normalized to a single csv value, not exploded into characters."""
139+
weaviate_139_mock.expect_request(
140+
f"{SCHEMA_PATH}/properties/age/index/rangeFilters",
141+
method="PUT",
142+
query_string={"tenants": "tenant1"},
143+
json={},
144+
).respond_with_json({"taskId": TASK_ID, "status": "STARTED"}, status=202)
145+
146+
task = client_139.collections.use(COLLECTION).config.update_property_index(
147+
"age", "rangeFilters", tenants="tenant1"
148+
)
149+
assert task.status == PropertyIndexTaskStatus.STARTED
150+
weaviate_139_mock.check_assertions()
151+
152+
153+
@pytest.mark.parametrize(
154+
"index_status,exception",
155+
[("failed", ReindexFailedError), ("cancelled", ReindexCanceledError)],
156+
)
157+
def test_update_property_index_wait_raises(
158+
weaviate_139_mock: HTTPServer,
159+
client_139: weaviate.WeaviateClient,
160+
index_status: str,
161+
exception: type,
162+
) -> None:
163+
weaviate_139_mock.expect_request(
164+
f"{SCHEMA_PATH}/properties/name/index/searchable",
165+
method="PUT",
166+
json={"tokenization": "word"},
167+
).respond_with_json({"taskId": TASK_ID, "status": "STARTED"}, status=202)
168+
weaviate_139_mock.expect_request(f"{SCHEMA_PATH}/indexes", method="GET").respond_with_json(
169+
{
170+
"collection": COLLECTION,
171+
"properties": [
172+
{
173+
"name": "name",
174+
"dataType": "text",
175+
"indexes": [
176+
{
177+
"type": "searchable",
178+
"status": index_status,
179+
"progress": 0.42,
180+
"taskId": TASK_ID,
181+
"tokenization": "word",
182+
}
183+
],
184+
}
185+
],
186+
}
187+
)
188+
189+
with pytest.raises(exception):
190+
client_139.collections.use(COLLECTION).config.update_property_index(
191+
"name", "searchable", tokenization=Tokenization.WORD, wait_for_completion=True
192+
)
193+
weaviate_139_mock.check_assertions()
194+
195+
196+
@pytest.mark.parametrize(
197+
"index_status,exception",
198+
[("failed", ReindexFailedError), ("cancelled", ReindexCanceledError)],
199+
)
200+
def test_rebuild_property_index_wait_raises(
201+
weaviate_139_mock: HTTPServer,
202+
client_139: weaviate.WeaviateClient,
203+
index_status: str,
204+
exception: type,
205+
) -> None:
206+
weaviate_139_mock.expect_request(
207+
f"{SCHEMA_PATH}/properties/name/index/searchable/rebuild",
208+
method="POST",
209+
json={},
210+
).respond_with_json({"taskId": TASK_ID, "status": "STARTED"}, status=202)
211+
weaviate_139_mock.expect_request(f"{SCHEMA_PATH}/indexes", method="GET").respond_with_json(
212+
{
213+
"collection": COLLECTION,
214+
"properties": [
215+
{
216+
"name": "name",
217+
"dataType": "text",
218+
"indexes": [
219+
{
220+
"type": "searchable",
221+
"status": index_status,
222+
"progress": 0.42,
223+
"taskId": TASK_ID,
224+
"tokenization": "word",
225+
}
226+
],
227+
}
228+
],
229+
}
230+
)
231+
232+
with pytest.raises(exception):
233+
client_139.collections.use(COLLECTION).config.rebuild_property_index(
234+
"name", "searchable", wait_for_completion=True
235+
)
236+
weaviate_139_mock.check_assertions()
237+
238+
131239
def test_rebuild_property_index(
132240
weaviate_139_mock: HTTPServer, client_139: weaviate.WeaviateClient
133241
) -> None:
134242
weaviate_139_mock.expect_request(
135243
f"{SCHEMA_PATH}/properties/name/index/searchable/rebuild",
136244
method="POST",
245+
json={},
137246
).respond_with_json({"taskId": TASK_ID, "status": "STARTED"}, status=202)
138247

139248
task = client_139.collections.use(COLLECTION).config.rebuild_property_index(
@@ -151,6 +260,7 @@ def test_rebuild_property_index_with_tenants(
151260
f"{SCHEMA_PATH}/properties/age/index/rangeFilters/rebuild",
152261
method="POST",
153262
query_string={"tenants": "tenant1,tenant2"},
263+
json={},
154264
).respond_with_json({"taskId": TASK_ID, "status": "STARTED"}, status=202)
155265

156266
task = client_139.collections.use(COLLECTION).config.rebuild_property_index(
@@ -167,6 +277,7 @@ def test_cancel_property_index_task_cancelled(
167277
weaviate_139_mock.expect_request(
168278
f"{SCHEMA_PATH}/properties/name/index/searchable/cancel",
169279
method="POST",
280+
json={},
170281
).respond_with_json({"taskId": TASK_ID, "status": "CANCELLED"}, status=202)
171282

172283
task = client_139.collections.use(COLLECTION).config.cancel_property_index_task(
@@ -183,6 +294,7 @@ def test_cancel_property_index_task_no_op(
183294
weaviate_139_mock.expect_request(
184295
f"{SCHEMA_PATH}/properties/name/index/searchable/cancel",
185296
method="POST",
297+
json={},
186298
).respond_with_json({"status": "NO_OP"}, status=202)
187299

188300
task = client_139.collections.use(COLLECTION).config.cancel_property_index_task(
@@ -270,6 +382,14 @@ def test_get_property_indexes(
270382
assert age.indexes[0].task_id is None
271383
assert age.indexes[0].tokenization is None
272384

385+
# the nested dataclasses serialize all the way down to a JSON-compatible dict
386+
out = json.loads(json.dumps(indexes.to_dict()))
387+
assert out["collection"] == COLLECTION
388+
assert out["properties"][0]["indexes"][0]["taskId"] == TASK_ID
389+
assert out["properties"][0]["indexes"][0]["targetTokenization"] == "field"
390+
assert out["properties"][1]["dataType"] == "int"
391+
assert out["properties"][1]["indexes"][0]["status"] == "ready"
392+
273393
weaviate_139_mock.check_assertions()
274394

275395

weaviate/collections/classes/config.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2271,6 +2271,11 @@ class _PropertyIndexes(_ConfigBase):
22712271
description: Optional[str]
22722272
indexes: List[PropertyIndexStatus]
22732273

2274+
def to_dict(self) -> Dict[str, Any]:
2275+
out = super().to_dict()
2276+
out["indexes"] = [index.to_dict() for index in self.indexes]
2277+
return out
2278+
22742279

22752280
PropertyIndexes = _PropertyIndexes
22762281

@@ -2280,6 +2285,11 @@ class _CollectionPropertyIndexes(_ConfigBase):
22802285
collection: str
22812286
properties: List[PropertyIndexes]
22822287

2288+
def to_dict(self) -> Dict[str, Any]:
2289+
out = super().to_dict()
2290+
out["properties"] = [prop.to_dict() for prop in self.properties]
2291+
return out
2292+
22832293

22842294
CollectionPropertyIndexes = _CollectionPropertyIndexes
22852295

weaviate/collections/classes/config_methods.py

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -592,22 +592,17 @@ def _property_index_status_from_json(index: Dict[str, Any]) -> _PropertyIndexSta
592592

593593

594594
def _collection_property_indexes_from_json(response: Dict[str, Any]) -> _CollectionPropertyIndexes:
595-
properties: List[_PropertyIndexes] = []
596-
for prop in response.get("properties") or []:
597-
data_type = prop.get("dataType")
598-
if isinstance(data_type, list):
599-
data_type = data_type[0] if len(data_type) > 0 else ""
600-
properties.append(
595+
return _CollectionPropertyIndexes(
596+
collection=response["collection"],
597+
properties=[
601598
_PropertyIndexes(
602599
name=prop["name"],
603-
data_type=cast(str, data_type),
600+
data_type=prop["dataType"],
604601
description=prop.get("description"),
605602
indexes=[
606603
_property_index_status_from_json(index) for index in prop.get("indexes") or []
607604
],
608605
)
609-
)
610-
return _CollectionPropertyIndexes(
611-
collection=response["collection"],
612-
properties=properties,
606+
for prop in response.get("properties") or []
607+
],
613608
)

weaviate/collections/config/async_.pyi

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
from typing import Dict, List, Literal, Optional, Sequence, Union, overload
1+
from typing import Dict, List, Literal, Optional, Union, overload
22

33
from typing_extensions import deprecated
44

@@ -102,7 +102,7 @@ class _ConfigCollectionAsync(_ConfigCollectionExecutor[ConnectionAsync]):
102102
*,
103103
tokenization: Optional[Tokenization] = None,
104104
algorithm: Optional[Literal["blockmax"]] = None,
105-
tenants: Optional[Sequence[str]] = None,
105+
tenants: Union[List[str], str, None] = None,
106106
wait_for_completion: Literal[True],
107107
) -> PropertyIndexStatus: ...
108108
@overload
@@ -113,7 +113,7 @@ class _ConfigCollectionAsync(_ConfigCollectionExecutor[ConnectionAsync]):
113113
*,
114114
tokenization: Optional[Tokenization] = None,
115115
algorithm: Optional[Literal["blockmax"]] = None,
116-
tenants: Optional[Sequence[str]] = None,
116+
tenants: Union[List[str], str, None] = None,
117117
wait_for_completion: Literal[False] = False,
118118
) -> PropertyIndexTask: ...
119119
@overload
@@ -122,7 +122,7 @@ class _ConfigCollectionAsync(_ConfigCollectionExecutor[ConnectionAsync]):
122122
property_name: str,
123123
index_name: IndexName,
124124
*,
125-
tenants: Optional[Sequence[str]] = None,
125+
tenants: Union[List[str], str, None] = None,
126126
wait_for_completion: Literal[True],
127127
) -> PropertyIndexStatus: ...
128128
@overload
@@ -131,7 +131,7 @@ class _ConfigCollectionAsync(_ConfigCollectionExecutor[ConnectionAsync]):
131131
property_name: str,
132132
index_name: IndexName,
133133
*,
134-
tenants: Optional[Sequence[str]] = None,
134+
tenants: Union[List[str], str, None] = None,
135135
wait_for_completion: Literal[False] = False,
136136
) -> PropertyIndexTask: ...
137137
async def cancel_property_index_task(

weaviate/collections/config/executor.py

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -754,7 +754,7 @@ def update_property_index(
754754
*,
755755
tokenization: Optional[Tokenization] = None,
756756
algorithm: Optional[Literal["blockmax"]] = None,
757-
tenants: Optional[Sequence[str]] = None,
757+
tenants: Union[List[str], str, None] = None,
758758
wait_for_completion: Literal[True],
759759
) -> executor.Result[PropertyIndexStatus]: ...
760760

@@ -766,7 +766,7 @@ def update_property_index(
766766
*,
767767
tokenization: Optional[Tokenization] = None,
768768
algorithm: Optional[Literal["blockmax"]] = None,
769-
tenants: Optional[Sequence[str]] = None,
769+
tenants: Union[List[str], str, None] = None,
770770
wait_for_completion: Literal[False] = False,
771771
) -> executor.Result[PropertyIndexTask]: ...
772772

@@ -777,7 +777,7 @@ def update_property_index(
777777
*,
778778
tokenization: Optional[Tokenization] = None,
779779
algorithm: Optional[Literal["blockmax"]] = None,
780-
tenants: Optional[Sequence[str]] = None,
780+
tenants: Union[List[str], str, None] = None,
781781
wait_for_completion: bool = False,
782782
) -> executor.Result[Union[PropertyIndexTask, PropertyIndexStatus]]:
783783
"""Create or migrate a property index in this collection.
@@ -794,9 +794,9 @@ def update_property_index(
794794
optional as a change on an existing `searchable` or `filterable` index. Not valid for
795795
`rangeFilters`.
796796
algorithm: The search algorithm of a `searchable` index. Only `blockmax` may be requested.
797-
tenants: The tenants for which to create the index. Only valid when creating a
798-
`rangeFilters` index on a multi-tenant collection. If not provided, all tenants are
799-
affected.
797+
tenants: The tenant/list of tenants for which to create the index. Only valid when
798+
creating a `rangeFilters` index on a multi-tenant collection. If not provided, all
799+
tenants are affected.
800800
wait_for_completion: Whether to wait until the index reports `ready`. By default False.
801801
802802
Returns:
@@ -823,6 +823,8 @@ def update_property_index(
823823
)
824824
if algorithm is not None:
825825
body["algorithm"] = algorithm
826+
if isinstance(tenants, str):
827+
tenants = [tenants]
826828
params: Optional[Dict[str, Any]] = (
827829
{"tenants": ",".join(tenants)} if tenants is not None else None
828830
)
@@ -874,7 +876,7 @@ def rebuild_property_index(
874876
property_name: str,
875877
index_name: IndexName,
876878
*,
877-
tenants: Optional[Sequence[str]] = None,
879+
tenants: Union[List[str], str, None] = None,
878880
wait_for_completion: Literal[True],
879881
) -> executor.Result[PropertyIndexStatus]: ...
880882

@@ -884,7 +886,7 @@ def rebuild_property_index(
884886
property_name: str,
885887
index_name: IndexName,
886888
*,
887-
tenants: Optional[Sequence[str]] = None,
889+
tenants: Union[List[str], str, None] = None,
888890
wait_for_completion: Literal[False] = False,
889891
) -> executor.Result[PropertyIndexTask]: ...
890892

@@ -893,16 +895,16 @@ def rebuild_property_index(
893895
property_name: str,
894896
index_name: IndexName,
895897
*,
896-
tenants: Optional[Sequence[str]] = None,
898+
tenants: Union[List[str], str, None] = None,
897899
wait_for_completion: bool = False,
898900
) -> executor.Result[Union[PropertyIndexTask, PropertyIndexStatus]]:
899901
"""Rebuild an existing property index from scratch with its current configuration.
900902
901903
Args:
902904
property_name: The property whose index to rebuild.
903905
index_name: The type of the index, one of `searchable`, `filterable` or `rangeFilters`.
904-
tenants: The tenants for which to rebuild the index on a multi-tenant collection.
905-
If not provided, all tenants are affected.
906+
tenants: The tenant/list of tenants for which to rebuild the index on a multi-tenant
907+
collection. If not provided, all tenants are affected.
906908
wait_for_completion: Whether to wait until the index reports `ready`. By default False.
907909
908910
Returns:
@@ -922,6 +924,8 @@ def rebuild_property_index(
922924
_validate_input([_ValidateArgument(expected=[str], name="index_name", value=index_name)])
923925

924926
path = self.__property_index_path(property_name, index_name) + "/rebuild"
927+
if isinstance(tenants, str):
928+
tenants = [tenants]
925929
params: Optional[Dict[str, Any]] = (
926930
{"tenants": ",".join(tenants)} if tenants is not None else None
927931
)

0 commit comments

Comments
 (0)