Improve error message for errant func registration

The observed behavior in the wild was that a double registration would complain
(correctly), but reference `None` as the function id.  Clearly not helpful.
Improve the situation by referencing the cached value, but also relax the
double-registration constraint slightly: if the calling code attempts to
re-register the function with the _same_ id, then that's odd -- so warn about
it -- but let it go.  Still raise the exception for other errant registration
attempts.

[sc-36241]

changelog.d/20241008_092953_kevin_improve_double_registration_error_message.rst

@@ -0,0 +1,4 @@
+Bug Fixes
+^^^^^^^^^
+
+- Fix ``function_id`` in error message that previously referenced ``None``

compute_sdk/globus_compute_sdk/sdk/executor.py

@@ -453,15 +453,24 @@ def register_function(
             the ``Client.register_function()``.
         :returns: the function's ``function_id`` string, as returned by
             registration upstream
+        :raises ValueError: raised if a function has already been registered with
+            this Executor
         """
         if self._stopped:
             raise RuntimeError(f"{self!r} is shutdown; refusing to register function")
 
         fn_cache_key = self._fn_cache_key(fn)
         if fn_cache_key in self._function_registry:
-            msg = f"Function already registered as function id: {function_id}"
+            cached_fn_id = self._function_registry[fn_cache_key]
+            msg = f"Function already registered as function id: {cached_fn_id}"
+            if function_id:
+                if function_id == cached_fn_id:
+                    log.warning(msg)
+                    return cached_fn_id
+
+                msg += f" (attempted id: {function_id})"
+
             log.error(msg)
-            self.shutdown(wait=False, cancel_futures=True)
             raise ValueError(msg)
 
         if function_id:

compute_sdk/tests/unit/test_executor.py

@@ -379,33 +379,48 @@ def some_func(*a, **k):
     gce._task_submitter.is_alive.return_value = False  # allow test cleanup
 
 
-def test_multiple_register_function_fails(gc_executor):
+def test_multiple_register_function_fails(gc_executor, mock_log):
     gcc, gce = gc_executor
 
     gcc.register_function.return_value = "abc"
 
-    gce.register_function(noop)
+    exp_f_id = gce.register_function(noop)
 
-    with pytest.raises(ValueError):
-        gce.register_function(noop)
+    f_id = gce.register_function(noop, function_id=exp_f_id)
+    assert f_id == exp_f_id, "Explicitly same func_id re-registered allowed ..."
+    assert mock_log.warning.called, "... but is odd, so warn about it"
 
-    try_assert(lambda: gce._stopped)
+    a, k = mock_log.warning.call_args
+    assert "Function already registered" in a[0]
+    assert f_id in a[0]
 
-    with pytest.raises(RuntimeError):
+    with pytest.raises(ValueError) as pyt_e:
         gce.register_function(noop)
 
+    e_str = str(pyt_e.value)
+    assert "Function already registered" in e_str
+    assert f_id in e_str
+    assert "attempted" not in e_str
+    assert not gce._stopped
+
 
 def test_shortcut_register_function(gc_executor):
     gcc, gce = gc_executor
 
     fn_id = str(uuid.uuid4())
+    other_id = str(uuid.uuid4())
     gce.register_function(noop, function_id=fn_id)
 
-    with pytest.raises(ValueError) as pyt_exc:
-        gce.register_function(noop, function_id=fn_id)
+    with pytest.raises(ValueError) as pyt_e:
+        gce.register_function(noop, function_id=other_id)
 
-    assert "Function already registered" in str(pyt_exc.value)
-    gcc.register_function.assert_not_called()
+    e_str = str(pyt_e.value)
+    assert "Function already registered" in e_str
+    assert fn_id in e_str
+    assert f"attempted id: {other_id}" in e_str
+    assert not gce._stopped
+
+    assert not gcc.register_function.called
 
 
 def test_failed_registration_shuts_down_executor(gc_executor, randomstring):
@@ -420,6 +435,13 @@ def test_failed_registration_shuts_down_executor(gc_executor, randomstring):
     assert pyt_exc.value is exc, "Expected raw exception raised"
     try_assert(lambda: gce._stopped)
 
+    with pytest.raises(RuntimeError) as pyt_e:
+        gce.register_function(noop)
+
+    e_str = str(pyt_e.value)
+    assert "is shutdown" in e_str
+    assert "refusing to register function" in e_str
+
 
 @pytest.mark.parametrize("container_id", (None, uuid.uuid4(), str(uuid.uuid4())))
 def test_container_id_as_id(gc_executor, container_id):