fix: cannot pass consistency level for delete

Also optinal variables default value should always be None.
And empty str is always invalid variable

See also: milvus-io#2327
pr: milvus-io#2350

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

Author: XuanYang-cn

pymilvus/client/grpc_handler.py

@@ -595,17 +595,15 @@ def delete(
         check_pass_param(collection_name=collection_name, timeout=timeout)
         try:
             req = Prepare.delete_request(
-                collection_name,
-                partition_name,
-                expression,
-                consistency_level=kwargs.get("consistency_level", 0),
-                param_name=kwargs.pop("param_name", None),
+                collection_name=collection_name,
+                expression=expression,
+                partition_name=partition_name,
+                consistency_level=kwargs.pop("consistency_level", 0),
                 **kwargs,
             )
             future = self._stub.Delete.future(req, timeout=timeout)
-
             if kwargs.get("_async", False):
-                cb = kwargs.get("_callback")
+                cb = kwargs.pop("_callback")
                 f = MutationFuture(future, cb, timeout=timeout, **kwargs)
                 f.add_callback(ts_utils.update_ts_on_mutation(collection_name))
                 return f

pymilvus/client/prepare.py

@@ -660,29 +660,31 @@ def batch_upsert_param(
     def delete_request(
         cls,
         collection_name: str,
+        expression: str,
         partition_name: str,
-        expr: str,
-        consistency_level: Optional[Union[int, str]],
+        consistency_level: Optional[Union[int, str]] = None,
         **kwargs,
     ):
-        def check_str(instr: str, prefix: str):
-            if instr is None:
-                raise ParamError(message=f"{prefix} cannot be None")
-            if not isinstance(instr, str):
-                raise ParamError(message=f"{prefix} value {instr} is illegal")
-            if len(instr) == 0:
-                raise ParamError(message=f"{prefix} cannot be empty")
-
-        check_str(collection_name, "collection_name")
-        if partition_name is not None and partition_name != "":
-            check_str(partition_name, "partition_name")
-        param_name = kwargs.get("param_name", "expr")
-        check_str(expr, param_name)
+        def valid_str(var: Any) -> True:
+            return isinstance(var, str) and len(var) > 0
+
+        if not valid_str(collection_name):
+            raise ParamError(
+                message=f"collection_name {collection_name} is illegal, expect none empty str"
+            )
+
+        if not valid_str(expression):
+            raise ParamError(message=f"expression {expression} is illegal, expect none empty str")
+
+        if partition_name is not None and not valid_str(partition_name):
+            raise ParamError(
+                message=f"partition_name {partition_name} is illegal, expect none empty str"
+            )
 
         return milvus_types.DeleteRequest(
             collection_name=collection_name,
             partition_name=partition_name,
-            expr=expr,
+            expr=expression,
             consistency_level=get_consistency_level(consistency_level),
             expr_template_values=cls.prepare_expression_template(kwargs.get("expr_params", {})),
         )

pymilvus/milvus_client/milvus_client.py

@@ -543,8 +543,8 @@ def delete(
         collection_name: str,
         ids: Optional[Union[list, str, int]] = None,
         timeout: Optional[float] = None,
-        filter: Optional[str] = "",
-        partition_name: Optional[str] = "",
+        filter: Optional[str] = None,
+        partition_name: Optional[str] = None,
         **kwargs,
     ) -> Dict:
         """Delete entries in the collection by their pk or by filter.
@@ -616,7 +616,6 @@ def delete(
                 expr,
                 partition_name,
                 timeout=timeout,
-                param_name="filter or ids",
                 expr_params=kwargs.pop("filter_params", {}),
                 **kwargs,
             )

tests/test_prepare.py

@@ -8,6 +8,17 @@
 
 
 class TestPrepare:
+    @pytest.mark.parametrize("coll_name", [None, "", -1, 1.1, []])
+    def test_delete_request_wrong_coll_name(self, coll_name: str):
+        with pytest.raises(MilvusException):
+            Prepare.delete_request(coll_name, "id>1", None, 0)
+
+    @pytest.mark.parametrize("part_name", ["", -1, 1.1, []])
+    def test_delete_request_wrong_part_name(self, part_name):
+        with pytest.raises(MilvusException):
+            Prepare.delete_request("coll", "id>1", part_name, 0)
+
+
     def test_search_requests_with_expr_offset(self):
         fields = [
             FieldSchema("pk", DataType.INT64, is_primary=True),
@@ -42,7 +53,7 @@ def test_search_requests_with_expr_offset(self):
                 params = json.loads(p.value)
                 if PAGE_RETAIN_ORDER_FIELD in params:
                     page_retain_order_exists = True
-                    assert  params[PAGE_RETAIN_ORDER_FIELD] == True
+                    assert  params[PAGE_RETAIN_ORDER_FIELD] is True
 
         assert offset_exists is True
         assert page_retain_order_exists is True
@@ -112,7 +123,7 @@ def test_get_schema_from_collection_schema(self):
 
         c_schema = Prepare.get_schema_from_collection_schema("random", schema)
 
-        assert c_schema.enable_dynamic_field == False
+        assert c_schema.enable_dynamic_field is False
         assert c_schema.name == "random"
         assert len(c_schema.fields) == 2
         assert c_schema.fields[0].name == "field_vector"