diff --git a/changelog.d/20241101_153924_chris_function_name_code.rst b/changelog.d/20241101_153924_chris_function_name_code.rst new file mode 100644 index 000000000..c726ffc20 --- /dev/null +++ b/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. diff --git a/compute_sdk/globus_compute_sdk/sdk/client.py b/compute_sdk/globus_compute_sdk/sdk/client.py index 685db868a..ac53e2e25 100644 --- a/compute_sdk/globus_compute_sdk/sdk/client.py +++ b/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, diff --git a/compute_sdk/globus_compute_sdk/sdk/web_client.py b/compute_sdk/globus_compute_sdk/sdk/web_client.py index 06137af62..d5cfe03f4 100644 --- a/compute_sdk/globus_compute_sdk/sdk/web_client.py +++ b/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 diff --git a/compute_sdk/tests/unit/test_client.py b/compute_sdk/tests/unit/test_client.py index 73a621d49..ca1463665 100644 --- a/compute_sdk/tests/unit/test_client.py +++ b/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()