fix: Raise ParamError if index params wrong (#2718)

See also: milvus-io/milvus#40564

Signed-off-by: yangxuan <[email protected]>

---------

Signed-off-by: yangxuan <[email protected]>

Author: XuanYang-cn

examples/simple_async.py

@@ -122,4 +122,4 @@ async def other_async_task(collection_name):
 
 results = loop.run_until_complete(other_async_task(collection_name))
 for r in results:
-    print(r)
+    print(r)

pymilvus/milvus_client/async_milvus_client.py

@@ -21,7 +21,8 @@
 from pymilvus.orm.connections import connections
 from pymilvus.orm.types import DataType
 
-from .index import IndexParams
+from .check import validate_param
+from .index import IndexParam, IndexParams
 
 logger = logging.getLogger(__name__)
 logger.setLevel(logging.DEBUG)
@@ -124,7 +125,7 @@ async def _fast_create_collection(
             raise ex from ex
 
         index_params = IndexParams()
-        index_params.add_index(vector_field_name, "", "", metric_type=metric_type)
+        index_params.add_index(vector_field_name, index_type="AUTOINDEX", metric_type=metric_type)
         await self.create_index(collection_name, index_params, timeout=timeout)
         await self.load_collection(collection_name, timeout=timeout)
 
@@ -186,24 +187,29 @@ async def create_index(
         timeout: Optional[float] = None,
         **kwargs,
     ):
+        validate_param("collection_name", collection_name, str)
+        validate_param("index_params", index_params, IndexParams)
+        if len(index_params) == 0:
+            raise ParamError(message="IndexParams is empty, no index can be created")
+
         for index_param in index_params:
             await self._create_index(collection_name, index_param, timeout=timeout, **kwargs)
 
     async def _create_index(
-        self, collection_name: str, index_param: Dict, timeout: Optional[float] = None, **kwargs
+        self,
+        collection_name: str,
+        index_param: IndexParam,
+        timeout: Optional[float] = None,
+        **kwargs,
     ):
         conn = self._get_connection()
         try:
-            params = index_param.pop("params", {})
-            field_name = index_param.pop("field_name", "")
-            index_name = index_param.pop("index_name", "")
-            params.update(index_param)
             await conn.create_index(
                 collection_name,
-                field_name,
-                params,
+                index_param.field_name,
+                index_param.get_index_configs(),
                 timeout=timeout,
-                index_name=index_name,
+                index_name=index_param.index_name,
                 **kwargs,
             )
             logger.debug("Successfully created an index on collection: %s", collection_name)
@@ -571,6 +577,13 @@ def create_schema(cls, **kwargs):
         kwargs["check_fields"] = False  # do not check fields for now
         return CollectionSchema([], **kwargs)
 
+    @classmethod
+    def prepare_index_params(cls, field_name: str = "", **kwargs) -> IndexParams:
+        index_params = IndexParams()
+        if field_name and validate_param("field_name", field_name, str):
+            index_params.add_index(field_name, **kwargs)
+        return index_params
+
     async def close(self):
         await connections.async_disconnect(self._using)
 

pymilvus/milvus_client/check.py

@@ -1,11 +1,32 @@
-from typing import Any
+from typing import Any, Dict, Tuple, Union
 
+from pymilvus.exceptions import ParamError
+
+
+def validate_params(params: Dict[str, Any], expected_type: Union[type, Tuple[type, ...]]):
+    validate_param("params", params, Dict)
+    for param_name, param in params.items():
+        validate_param(param_name, param, expected_type)
+
+
+def validate_param(param_name: str, param: Any, expected_type: Union[type, Tuple[type, ...]]):
+    if param is None:
+        msg = f"missing required argument: [{param_name}]"
+        raise ParamError(message=msg)
 
-def check_param_type(param_name: str, param: Any, expected_type: Any, ignore_none: bool = True):
-    if ignore_none and param is None:
-        return
     if not isinstance(param, expected_type):
-        msg = f"wrong type of arugment '{param_name}', "
-        msg += f"expected '{expected_type.__name__}', "
-        msg += f"got '{type(param).__name__}'"
-        raise TypeError(msg)
+        msg = (
+            f"wrong type of argument [{param_name}], "
+            f"expected type: [{expected_type.__name__}], "
+            f"got type: [{type(param).__name__}]"
+        )
+        raise ParamError(message=msg)
+
+
+def validate_noneable_param(
+    param_name: str, param: Any, expected_type: Union[type, Tuple[type, ...]]
+):
+    if param is None:
+        return
+
+    validate_param(param_name, param, expected_type)

pymilvus/milvus_client/index.py

@@ -1,9 +1,30 @@
+from typing import Dict
+
+
 class IndexParam:
     def __init__(self, field_name: str, index_type: str, index_name: str, **kwargs):
+        """
+        Examples:
+
+            >>> IndexParam(
+            >>>     field_name="embeddings",
+            >>>     index_type="HNSW",
+            >>>     index_name="hnsw_index",
+            >>>     metric_type="COSINE",
+            >>>     M=64,
+            >>>     efConstruction=100,
+            >>> )
+        """
         self._field_name = field_name
         self._index_type = index_type
         self._index_name = index_name
-        self._kwargs = kwargs
+
+        # index configs are unique to each index,
+        # if params={} is passed in, it will be flattened and merged
+        # with other configs.
+        self._configs = {}
+        self._configs.update(kwargs.pop("params", {}))
+        self._configs.update(kwargs)
 
     @property
     def field_name(self):
@@ -17,12 +38,31 @@ def index_name(self):
     def index_type(self):
         return self._index_type
 
-    def __iter__(self):
-        yield "field_name", self.field_name
-        if self.index_type:
-            yield "index_type", self.index_type
-        yield "index_name", self.index_name
-        yield from self._kwargs.items()
+    def get_index_configs(self) -> Dict:
+        """return index_type and index configs in a dict
+
+        Examples:
+
+            {
+                "index_type": "HNSW",
+                "metrics_type": "COSINE",
+                "M": 64,
+                "efConstruction": 100,
+            }
+        """
+        return {
+            "index_type": self.index_type,
+            **self._configs,
+        }
+
+    def to_dict(self) -> Dict:
+        """All params"""
+        return {
+            "field_name": self.field_name,
+            "index_type": self.index_type,
+            "index_name": self.index_name,
+            **self._configs,
+        }
 
     def __str__(self):
         return str(dict(self))
@@ -36,19 +76,15 @@ def __eq__(self, other: None):
         return False
 
 
-class IndexParams:
-    def __init__(self, field_name: str = "", **kwargs):
-        self._indexes = []
-        if field_name:
-            self.add_index(field_name, **kwargs)
+class IndexParams(list):
+    """List of indexs of a collection"""
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
 
     def add_index(self, field_name: str, index_type: str = "", index_name: str = "", **kwargs):
         index_param = IndexParam(field_name, index_type, index_name, **kwargs)
-        self._indexes.append(index_param)
-
-    def __iter__(self):
-        for v in self._indexes:
-            yield dict(v)
+        super().append(index_param)
 
     def __str__(self):
         return str(list(self))

pymilvus/milvus_client/milvus_client.py

@@ -1,5 +1,3 @@
-"""MilvusClient for dealing with simple workflows."""
-
 import logging
 from typing import Dict, List, Optional, Union
 from uuid import uuid4
@@ -32,7 +30,8 @@
 from pymilvus.orm.iterator import QueryIterator, SearchIterator
 from pymilvus.orm.types import DataType
 
-from .index import IndexParams
+from .check import validate_param
+from .index import IndexParam, IndexParams
 
 logger = logging.getLogger(__name__)
 
@@ -112,9 +111,8 @@ def _fast_create_collection(
         timeout: Optional[float] = None,
         **kwargs,
     ):
-        if dimension is None:
-            msg = "missing requried argument: 'dimension'"
-            raise TypeError(msg)
+        validate_param("dimension", dimension, int)
+
         if "enable_dynamic_field" not in kwargs:
             kwargs["enable_dynamic_field"] = True
 
@@ -132,8 +130,7 @@ def _fast_create_collection(
             pk_args["max_length"] = kwargs["max_length"]
 
         schema.add_field(primary_field_name, pk_data_type, is_primary=True, **pk_args)
-        vector_type = DataType.FLOAT_VECTOR
-        schema.add_field(vector_field_name, vector_type, dim=dimension)
+        schema.add_field(vector_field_name, DataType.FLOAT_VECTOR, dim=dimension)
         schema.verify()
 
         conn = self._get_connection()
@@ -147,7 +144,7 @@ def _fast_create_collection(
             raise ex from ex
 
         index_params = IndexParams()
-        index_params.add_index(vector_field_name, "", "", metric_type=metric_type)
+        index_params.add_index(vector_field_name, index_type="AUTOINDEX", metric_type=metric_type)
         self.create_index(collection_name, index_params, timeout=timeout)
         self.load_collection(collection_name, timeout=timeout)
 
@@ -158,24 +155,29 @@ def create_index(
         timeout: Optional[float] = None,
         **kwargs,
     ):
+        validate_param("collection_name", collection_name, str)
+        validate_param("index_params", index_params, IndexParams)
+        if len(index_params) == 0:
+            raise ParamError(message="IndexParams is empty, no index can be created")
+
         for index_param in index_params:
             self._create_index(collection_name, index_param, timeout=timeout, **kwargs)
 
     def _create_index(
-        self, collection_name: str, index_param: Dict, timeout: Optional[float] = None, **kwargs
+        self,
+        collection_name: str,
+        index_param: IndexParam,
+        timeout: Optional[float] = None,
+        **kwargs,
     ):
         conn = self._get_connection()
         try:
-            params = index_param.pop("params", {})
-            field_name = index_param.pop("field_name", "")
-            index_name = index_param.pop("index_name", "")
-            params.update(index_param)
             conn.create_index(
                 collection_name,
-                field_name,
-                params,
+                index_param.field_name,
+                index_param.get_index_configs(),
                 timeout=timeout,
-                index_name=index_name,
+                index_name=index_param.index_name,
                 **kwargs,
             )
             logger.debug("Successfully created an index on collection: %s", collection_name)
@@ -872,8 +874,11 @@ def create_schema(cls, **kwargs):
         return CollectionSchema([], **kwargs)
 
     @classmethod
-    def prepare_index_params(cls, field_name: str = "", **kwargs):
-        return IndexParams(field_name, **kwargs)
+    def prepare_index_params(cls, field_name: str = "", **kwargs) -> IndexParams:
+        index_params = IndexParams()
+        if field_name and validate_param("field_name", field_name, str):
+            index_params.add_index(field_name, **kwargs)
+        return index_params
 
     def _create_collection_with_schema(
         self,

tests/conftest.py

@@ -4,6 +4,8 @@
 
 # https://github.com/grpc/grpc/blob/5918f98ecbf5ace77f30fa97f7fc3e8bdac08e04/src/python/grpcio_tests/tests/testing/_client_test.py
 from grpc.framework.foundation import logging_pool
+import logging
+logging.getLogger("faker").setLevel(logging.WARNING)
 
 from pymilvus.grpc_gen import milvus_pb2
 

tests/test_milvus_client.py

@@ -0,0 +1,24 @@
+from unittest.mock import patch
+
+import pytest
+
+from pymilvus.exceptions import ParamError
+from pymilvus.milvus_client.index import IndexParams
+from pymilvus.milvus_client.milvus_client import MilvusClient
+
+
+class TestMilvusClient:
+    @pytest.mark.parametrize("index_params", [None, {}, "str", MilvusClient.prepare_index_params()])
+    def test_create_index_invalid_params(self, index_params):
+        with patch("pymilvus.orm.utility.get_server_type", return_value="milvus"), patch('pymilvus.milvus_client.milvus_client.MilvusClient._create_connection', return_value="test"):
+            client = MilvusClient()
+
+            if isinstance(index_params, IndexParams):
+                with pytest.raises(ParamError, match="IndexParams is empty, no index can be created"):
+                    client.create_index("test_collection", index_params)
+            elif index_params is None:
+                with pytest.raises(ParamError, match="missing required argument:.*"):
+                    client.create_index("test_collection", index_params)
+            else:
+                with pytest.raises(ParamError, match="wrong type of argument .*"):
+                    client.create_index("test_collection", index_params)