fix: remote function argument validation

Author: Namrata Madan

src/sagemaker/remote_function/client.py

@@ -457,30 +457,28 @@ def _validate_submit_args(func, *args, **kwargs):
             )
 
         if len(args) < minimum_num_expected_positional_args:
-            missing_args_count = minimum_num_expected_positional_args - len(args)
-            if missing_args_count == 1:
-                missing_args = f"'{full_arg_spec.args[minimum_num_expected_positional_args - 1]}'"
-            else:
-                missing_args = (
-                    ", ".join(
-                        map(
-                            lambda x: f"'{x}'",
-                            full_arg_spec.args[
-                                len(args) : minimum_num_expected_positional_args - 1
-                            ],
-                        )
-                    )
-                    + f", and '{full_arg_spec.args[minimum_num_expected_positional_args - 1]}'"
+            missing_positional_args = full_arg_spec.args[
+                len(args) : minimum_num_expected_positional_args
+            ]
+            missing_args = list(filter(lambda arg: arg not in kwargs, missing_positional_args))
+            if missing_args:
+                missing_args_str = (
+                    ", ".join(map(lambda x: f"'{x}'", missing_args[:-1]))
+                    + f", and '{missing_args[-1]}'"
+                    if len(missing_args) > 1
+                    else f"'{missing_args[0]}'"
+                )
+                raise TypeError(
+                    f"{func.__name__}() missing {len(missing_args)} required positional "
+                    + f"{'arguments' if len(missing_args) > 1 else 'argument'}: {missing_args_str}"
                 )
-            raise TypeError(
-                f"{func.__name__}() missing {missing_args_count} required positional "
-                + f"{'arguments' if missing_args_count > 1 else 'argument'}: {missing_args}"
-            )
 
         # kwargs related validations
 
         for k in kwargs:
-            if k not in full_arg_spec.kwonlyargs:
+            if k in full_arg_spec.args and len(args) > full_arg_spec.args.index(k):
+                raise TypeError(f"{func.__name__}() got multiple values for argument '{k}'")
+            if k not in full_arg_spec.kwonlyargs and k not in full_arg_spec.args:
                 raise TypeError(f"{func.__name__}() got an unexpected keyword argument '{k}'")
 
         missing_kwargs = [
@@ -489,13 +487,12 @@ def _validate_submit_args(func, *args, **kwargs):
             if k not in full_arg_spec.kwonlydefaults and k not in kwargs
         ]
         if missing_kwargs:
-            if len(missing_kwargs) == 1:
-                missing_kwargs_string = f"'{missing_kwargs[0]}'"
-            else:
-                missing_kwargs_string = (
-                    ", ".join(map(lambda x: f"'{x}'", missing_kwargs[:-1]))
-                    + f", and '{missing_kwargs[-1]}'"
-                )
+            missing_kwargs_string = (
+                ", ".join(map(lambda x: f"'{x}'", missing_kwargs[:-1]))
+                + f", and '{missing_kwargs[-1]}'"
+                if len(missing_kwargs) > 1
+                else f"'{missing_kwargs[0]}'"
+            )
 
             raise TypeError(
                 f"{func.__name__}() missing {len(missing_kwargs)} required keyword-only "

tests/unit/sagemaker/remote_function/test_client.py

@@ -297,29 +297,57 @@ def square(x):
 @pytest.mark.parametrize(
     "args, kwargs, error_message",
     [
-        ((1, 2), {}, "decorated_function() missing 1 required keyword-only argument: 'c'"),
         (
-            (1, 2),
-            {"c": 3, "d": 4, "e": "extra_arg"},
-            "decorated_function() got an unexpected keyword argument 'e'",
+            [1, 2, 3],
+            {},
+            "decorated_function() missing 2 required keyword-only arguments: 'd', and 'e'",
         ),
-        ((), {"c": 3, "d": 4}, "decorated_function() missing 1 required positional argument: 'a'"),
+        ([1, 2, 3], {"d": 4}, "decorated_function() missing 1 required keyword-only argument: 'e'"),
         (
-            (1, 2, "extra_Arg"),
+            [1, 2, 3],
+            {"d": 3, "e": 4, "g": "extra_arg"},
+            "decorated_function() got an unexpected keyword argument 'g'",
+        ),
+        (
+            [],
             {"c": 3, "d": 4},
-            "decorated_function() takes 2 positional arguments but 3 were given.",
+            "decorated_function() missing 2 required positional arguments: 'a', and 'b'",
+        ),
+        ([1], {"c": 3, "d": 4}, "decorated_function() missing 1 required positional argument: 'b'"),
+        (
+            [1, 2, 3, "extra_arg"],
+            {"d": 3, "e": 4},
+            "decorated_function() takes 3 positional arguments but 4 were given.",
+        ),
+        ([], {"a": 1, "b": 2, "d": 3, "e": 2}, None),
+        (
+            (1, 2),
+            {"a": 1, "c": 3, "d": 2},
+            "decorated_function() got multiple values for argument 'a'",
+        ),
+        (
+            (1, 2),
+            {"b": 1, "c": 3, "d": 2},
+            "decorated_function() got multiple values for argument 'b'",
         ),
     ],
 )
-def test_decorator_invalid_function_args(args, kwargs, error_message):
+@patch("sagemaker.remote_function.client._JobSettings")
+@patch("sagemaker.remote_function.client._Job.start")
+def test_decorator_invalid_function_args(job_start, job_settings, args, kwargs, error_message):
     @remote(image_uri=IMAGE, s3_root_uri=S3_URI)
-    def decorated_function(a, b=1, *, c, d=3):
-        return a * b * c * d
-
-    with pytest.raises(TypeError) as e:
-        decorated_function(*args, **kwargs)
-
-    assert error_message in str(e.value)
+    def decorated_function(a, b, c=1, *, d, e, f=3):
+        return a * b * c * d * e * f
+
+    if error_message:
+        with pytest.raises(TypeError) as e:
+            decorated_function(*args, **kwargs)
+        assert error_message in str(e.value)
+    else:
+        try:
+            decorated_function(*args, **kwargs)
+        except Exception as ex:
+            pytest.fail("Unexpected Exception: " + str(ex))
 
 
 def test_executor_invalid_arguments():