From 8b02320c3008c9f36809c18db2e2404cb8ccb3b9 Mon Sep 17 00:00:00 2001 From: Sainikhil Juluri Date: Sun, 16 Aug 2026 16:19:46 -0700 Subject: [PATCH] fix: report partition_names in validation errors instead of the internal key check_pass_param was called with the internal key partition_name_array, and _raise_param_error formats the registry key straight into the message, so an invalid value reported a parameter name that appears nowhere in the public API: `partition_name_array` value 1 is illegal Register the validator under partition_names and pass that name from the call sites. The old key stays registered so anything still passing it keeps being validated. Fixes #2589 Co-Authored-By: Claude Opus 5 Signed-off-by: Sainikhil Juluri --- pymilvus/client/async_grpc_handler.py | 6 +++--- pymilvus/client/check.py | 2 ++ pymilvus/client/grpc_handler.py | 6 +++--- pymilvus/client/prepare.py | 8 ++++---- tests/unit/prepare/test_collection.py | 18 ++++++++++++++++++ tests/unit/test_client_validators.py | 20 ++++++++++++++++++++ 6 files changed, 50 insertions(+), 10 deletions(-) diff --git a/pymilvus/client/async_grpc_handler.py b/pymilvus/client/async_grpc_handler.py index e9ff31dd8..ce44006a9 100644 --- a/pymilvus/client/async_grpc_handler.py +++ b/pymilvus/client/async_grpc_handler.py @@ -1037,7 +1037,7 @@ async def search( limit=limit, round_decimal=round_decimal, anns_field=anns_field, - partition_name_array=partition_names, + partition_names=partition_names, output_fields=output_fields, guarantee_timestamp=kwargs.get("guarantee_timestamp"), timeout=timeout, @@ -1103,7 +1103,7 @@ async def hybrid_search( check_pass_param( limit=limit, round_decimal=round_decimal, - partition_name_array=partition_names, + partition_names=partition_names, output_fields=output_fields, guarantee_timestamp=kwargs.get("guarantee_timestamp"), timeout=timeout, @@ -1390,7 +1390,7 @@ async def release_partitions( **kwargs, ): check_pass_param( - collection_name=collection_name, partition_name_array=partition_names, timeout=timeout + collection_name=collection_name, partition_names=partition_names, timeout=timeout ) request = Prepare.release_partitions("", collection_name, partition_names) response = await self._async_stub.ReleasePartitions( diff --git a/pymilvus/client/check.py b/pymilvus/client/check.py index f171a8512..cfe815f82 100644 --- a/pymilvus/client/check.py +++ b/pymilvus/client/check.py @@ -353,6 +353,8 @@ def __init__(self) -> None: "nlist": is_legal_nlist, "cmd": is_legal_cmd, "partition_name": is_legal_partition_name, + "partition_names": is_legal_partition_name_array, + # Kept so callers still passing the internal name keep validating. "partition_name_array": is_legal_partition_name_array, "limit": is_legal_limit, "anns_field": is_legal_anns_field, diff --git a/pymilvus/client/grpc_handler.py b/pymilvus/client/grpc_handler.py index a28910b33..3b1d1014c 100644 --- a/pymilvus/client/grpc_handler.py +++ b/pymilvus/client/grpc_handler.py @@ -1420,7 +1420,7 @@ def search( limit=limit, round_decimal=round_decimal, anns_field=anns_field, - partition_name_array=partition_names, + partition_names=partition_names, output_fields=output_fields, guarantee_timestamp=kwargs.get("guarantee_timestamp"), timeout=timeout, @@ -1481,7 +1481,7 @@ def hybrid_search( check_pass_param( limit=limit, round_decimal=round_decimal, - partition_name_array=partition_names, + partition_names=partition_names, output_fields=output_fields, guarantee_timestamp=kwargs.get("guarantee_timestamp"), timeout=timeout, @@ -2177,7 +2177,7 @@ def release_partitions( **kwargs, ): check_pass_param( - collection_name=collection_name, partition_name_array=partition_names, timeout=timeout + collection_name=collection_name, partition_names=partition_names, timeout=timeout ) request = Prepare.release_partitions("", collection_name, partition_names) response = self._stub.ReleasePartitions( diff --git a/pymilvus/client/prepare.py b/pymilvus/client/prepare.py index ace1f4411..170a0b15e 100644 --- a/pymilvus/client/prepare.py +++ b/pymilvus/client/prepare.py @@ -618,7 +618,7 @@ def show_partitions_request( partition_names: Optional[List[str]] = None, type_in_memory: bool = False, ): - check_pass_param(collection_name=collection_name, partition_name_array=partition_names) + check_pass_param(collection_name=collection_name, partition_names=partition_names) req = milvus_types.ShowPartitionsRequest(collection_name=collection_name) if partition_names: if not isinstance(partition_names, (list,)): @@ -637,7 +637,7 @@ def show_partitions_request( def get_loading_progress( cls, collection_name: str, partition_names: Optional[List[str]] = None ): - check_pass_param(collection_name=collection_name, partition_name_array=partition_names) + check_pass_param(collection_name=collection_name, partition_names=partition_names) req = milvus_types.GetLoadingProgressRequest(collection_name=collection_name) if partition_names: req.partition_names.extend(partition_names) @@ -645,7 +645,7 @@ def get_loading_progress( @classmethod def get_load_state(cls, collection_name: str, partition_names: Optional[List[str]] = None): - check_pass_param(collection_name=collection_name, partition_name_array=partition_names) + check_pass_param(collection_name=collection_name, partition_names=partition_names) req = milvus_types.GetLoadStateRequest(collection_name=collection_name) if partition_names: req.partition_names.extend(partition_names) @@ -2250,7 +2250,7 @@ def load_partitions( ) if partition_names: - check_pass_param(partition_name_array=partition_names) + check_pass_param(partition_names=partition_names) req.partition_names.extend(partition_names) if replica_number: diff --git a/tests/unit/prepare/test_collection.py b/tests/unit/prepare/test_collection.py index 2bb7b3985..3531bec97 100644 --- a/tests/unit/prepare/test_collection.py +++ b/tests/unit/prepare/test_collection.py @@ -555,3 +555,21 @@ def test_error_on_empty_drop_identifier(self, kwargs): def test_error_on_multiple_drop_identifiers(self, kwargs): with pytest.raises(ParamError, match="exactly one valid Drop identifier"): Prepare.alter_collection_schema_request(collection_name="coll", **kwargs) + + +class TestLoadPartitionsRequest: + """Tests for load_partitions.""" + + def test_rejects_invalid_partition_names_by_their_public_name(self): + """The error must name `partition_names`, the argument the caller actually passed. + + The validator was registered under the internal key `partition_name_array`, and the + message is built from that registry key, so callers were told a parameter name that + appears nowhere in the public API (#2589). + """ + with pytest.raises(ParamError, match=r"`partition_names` value \['p1', 1\] is illegal"): + Prepare.load_partitions("coll", ["p1", 1]) + + def test_accepts_valid_partition_names(self): + req = Prepare.load_partitions("coll", ["p1", "p2"]) + assert list(req.partition_names) == ["p1", "p2"] diff --git a/tests/unit/test_client_validators.py b/tests/unit/test_client_validators.py index 313e66dbd..ad4f937d4 100644 --- a/tests/unit/test_client_validators.py +++ b/tests/unit/test_client_validators.py @@ -261,6 +261,26 @@ def test_not_list(self): assert is_legal_partition_name_array("p1") is False +class TestPartitionNamesErrorMessage: + def test_reports_the_public_parameter_name(self): + """The message must name `partition_names`, the argument callers actually pass. + + The checker was registered under the internal key `partition_name_array`, and + `_raise_param_error` formats the registry key straight into the message, so an + invalid value reported a name that appears nowhere in the public API (#2589). + """ + with pytest.raises(ParamError, match=r"`partition_names` value 1 is illegal"): + check_pass_param(partition_names=1) + + def test_valid_value_passes(self): + check_pass_param(partition_names=["p1", "p2"]) + + def test_internal_key_still_validates(self): + """Back-compat: anything still passing the old key keeps being checked.""" + with pytest.raises(ParamError, match=r"`partition_name_array` value 1 is illegal"): + check_pass_param(partition_name_array=1) + + class TestIsLegalDropRatio: def test_valid(self): assert is_legal_drop_ratio(0.5) is True