Merge pull request #1699 from globus/function-name-code

Disallow function and function_name

changelog.d/20241101_153924_chris_function_name_code.rst

@@ -0,0 +1,6 @@
+Deprecated
+^^^^^^^^^^
+
+- Before this version, the ``function_name`` argument to ``Client.register_function``
+  was not used, so it has now been deprecated. As before, function names are
+  determined by the function's ``__name__`` and cannot be manually specified.

compute_sdk/globus_compute_sdk/sdk/client.py

@@ -587,6 +587,7 @@ def register_function(
             The function to be registered for remote execution
         function_name : str
             The entry point (function name) of the function. Default: None
+            DEPRECATED - functions' names are derived from their ``__name__`` attribute
         container_uuid : str
             Container UUID from registration with Globus Compute
         description : str
@@ -608,11 +609,14 @@ def register_function(
         function uuid : str
             UUID identifier for the registered function
         """
-        if searchable is not None:
-            warnings.warn(
-                "The 'searchable' argument is deprecated and no longer functional. "
-                "It will be removed in a future release."
-            )
+        deprecated = ["searchable", "function_name"]
+        for arg in deprecated:
+            if locals()[arg] is not None:
+                warnings.warn(
+                    f"The '{arg}' argument is deprecated and no longer functional. "
+                    "It will be removed in a future release.",
+                    DeprecationWarning,
+                )
 
         data = FunctionRegistrationData(
             function=function,

compute_sdk/globus_compute_sdk/sdk/web_client.py

@@ -55,13 +55,18 @@ def __init__(
         serializer: t.Optional[ComputeSerializer] = None,
     ):
         if function is not None:
+            if function_name is not None or function_code is not None:
+                raise ValueError(
+                    "Cannot specify 'function_name' or 'function_code'"
+                    " if 'function' is provided."
+                )
             function_name = function.__name__
             function_code = _get_packed_code(function, serializer=serializer)
 
         if function_name is None or function_code is None:
             raise ValueError(
                 "Either 'function' must be provided, or "
-                "both of 'function_name' and 'function_code'"
+                "both of 'function_name' and 'function_code'."
             )
 
         self.function_name = function_name

compute_sdk/tests/unit/test_client.py

@@ -13,6 +13,7 @@
     FunctionRegistrationData,
     FunctionRegistrationMetadata,
     WebClient,
+    _get_packed_code,
 )
 from globus_compute_sdk.serialize import ComputeSerializer
 from globus_compute_sdk.serialize.concretes import SELECTABLE_STRATEGIES
@@ -38,6 +39,10 @@ def gcc():
     yield _gcc
 
 
+def funk():
+    return "Funky"
+
+
 @pytest.mark.parametrize(
     "kwargs",
     [
@@ -302,9 +307,6 @@ def __init__(self):
 def test_register_function(gcc):
     gcc.web_client = mock.MagicMock()
 
-    def funk():
-        return "Funky"
-
     metadata = {"python_version": "3.11.3", "sdk_version": "2.3.3"}
     gcc.register_function(funk, metadata=metadata)
 
@@ -317,11 +319,20 @@ def funk():
     assert func_data.metadata.sdk_version == metadata["sdk_version"]
 
 
-def test_register_function_no_metadata(gcc):
+@pytest.mark.parametrize("dep_arg", ["searchable", "function_name"])
+def test_register_function_deprecated_args(gcc, dep_arg):
     gcc.web_client = mock.MagicMock()
 
-    def funk():
-        return "Funky"
+    with pytest.deprecated_call() as pyt_wrn:
+        gcc.register_function(funk, **{dep_arg: "foo"})
+
+    warning = pyt_wrn.pop(DeprecationWarning)
+    assert "deprecated" in str(warning).lower()
+    assert dep_arg in str(warning)
+
+
+def test_register_function_no_metadata(gcc):
+    gcc.web_client = mock.MagicMock()
 
     gcc.register_function(funk)
 
@@ -331,6 +342,63 @@ def funk():
     assert func_data.metadata is None
 
 
+def test_register_function_no_function(gcc):
+    gcc.web_client = mock.MagicMock()
+
+    with pytest.raises(ValueError) as pyt_exc:
+        gcc.register_function(None)
+
+    assert "either" in str(pyt_exc).lower()
+    assert "'function'" in str(pyt_exc).lower()
+    assert "'function_name'" in str(pyt_exc).lower()
+    assert "'function_code'" in str(pyt_exc).lower()
+
+
+def test_function_registration_data_function():
+    frd = FunctionRegistrationData(function=funk)
+
+    assert frd.function_name == "funk"
+    assert frd.function_code == _get_packed_code(funk)
+
+
+def test_function_registration_data_function_name_and_code():
+    frd = FunctionRegistrationData(
+        function=None, function_name="foo", function_code="bar"
+    )
+
+    assert frd.function_name == "foo"
+    assert frd.function_code == "bar"
+
+
+@pytest.mark.parametrize(
+    "function_name, function_code", [("foo", None), (None, "bar"), (None, None)]
+)
+def test_function_registration_data_must_have_both_function_name_and_function_code(
+    function_name, function_code
+):
+    with pytest.raises(ValueError) as pyt_exc:
+        FunctionRegistrationData(
+            function=None, function_name=function_name, function_code=function_code
+        )
+
+    assert "either" in str(pyt_exc).lower()
+    assert "'function'" in str(pyt_exc).lower()
+    assert "'function_name'" in str(pyt_exc).lower()
+    assert "'function_code'" in str(pyt_exc).lower()
+
+
+def test_function_registration_data_cant_have_both_function_and_name_code(randomstring):
+    with pytest.raises(ValueError) as pyt_exc:
+        FunctionRegistrationData(
+            function=funk, function_name=randomstring(), function_code=randomstring()
+        )
+
+    assert "cannot specify" in str(pyt_exc).lower()
+    assert "'function'" in str(pyt_exc).lower()
+    assert "'function_name'" in str(pyt_exc).lower()
+    assert "'function_code'" in str(pyt_exc).lower()
+
+
 def test_get_function(gcc):
     func_uuid_str = str(uuid.uuid4())
     gcc.web_client = mock.MagicMock()