Remove deprecated cleanup() method and tests

Author: kevin-bates

jupyter_client/manager.py

@@ -7,7 +7,6 @@
 import signal
 import sys
 import typing as t
-import warnings
 from contextlib import contextmanager
 from enum import Enum
 from subprocess import Popen
@@ -19,7 +18,6 @@
 from traitlets import DottedObjectName
 from traitlets import Float
 from traitlets import Instance
-from traitlets import List
 from traitlets import observe
 from traitlets import observe_compat
 from traitlets import Type
@@ -410,15 +408,6 @@ def cleanup_resources(self, restart: bool = False) -> None:
         if self._created_context and not restart:
             self.context.destroy(linger=100)
 
-    def cleanup(self, connection_file: bool = True) -> None:
-        """Clean up resources when the kernel is shut down"""
-        warnings.warn(
-            "Method cleanup(connection_file=True) is deprecated, use cleanup_resources"
-            "(restart=False).",
-            FutureWarning,
-        )
-        self.cleanup_resources(restart=not connection_file)
-
     async def _async_shutdown_kernel(self, now: bool = False, restart: bool = False):
         """Attempts to stop the kernel process cleanly.
 
@@ -452,28 +441,7 @@ async def _async_shutdown_kernel(self, now: bool = False, restart: bool = False)
             # most 1s, checking every 0.1s.
             await ensure_async(self.finish_shutdown())
 
-        # In 6.1.5, a new method, cleanup_resources(), was introduced to address
-        # a leak issue (https://github.com/jupyter/jupyter_client/pull/548) and
-        # replaced the existing cleanup() method.  However, that method introduction
-        # breaks subclass implementations that override cleanup() since it would
-        # circumvent cleanup() functionality implemented in subclasses.
-        # By detecting if the current instance overrides cleanup(), we can determine
-        # if the deprecated path of calling cleanup() should be performed - which avoids
-        # unnecessary deprecation warnings in a majority of configurations in which
-        # subclassed KernelManager instances are not in use.
-        # Note: because subclasses may have already implemented cleanup_resources()
-        # but need to support older jupyter_clients, we should only take the deprecated
-        # path if cleanup() is overridden but cleanup_resources() is not.
-
-        overrides_cleanup = type(self).cleanup is not KernelManager.cleanup
-        overrides_cleanup_resources = (
-            type(self).cleanup_resources is not KernelManager.cleanup_resources
-        )
-
-        if overrides_cleanup and not overrides_cleanup_resources:
-            self.cleanup(connection_file=not restart)
-        else:
-            self.cleanup_resources(restart=restart)
+        self.cleanup_resources(restart=restart)
 
     shutdown_kernel = run_sync(_async_shutdown_kernel)
 

jupyter_client/multikernelmanager.py

@@ -272,10 +272,6 @@ def finish_shutdown(
         """Wait for a kernel to finish shutting down, and kill it if it doesn't"""
         self.log.info("Kernel shutdown: %s" % kernel_id)
 
-    @kernel_method
-    def cleanup(self, kernel_id: str, connection_file: bool = True) -> None:
-        """Clean up a kernel's resources"""
-
     @kernel_method
     def cleanup_resources(self, kernel_id: str, restart: bool = False) -> None:
         """Clean up a kernel's resources"""

jupyter_client/tests/test_kernelmanager.py

@@ -19,7 +19,6 @@
 from ..manager import _ShutdownStatus
 from ..manager import start_new_async_kernel
 from ..manager import start_new_kernel
-from .utils import AsyncKernelManagerWithCleanup
 from .utils import AsyncKMSubclass
 from .utils import SyncKMSubclass
 from .utils import test_env
@@ -123,7 +122,7 @@ def zmq_context():
     ctx.term()
 
 
-@pytest.fixture(params=[AsyncKernelManager, AsyncKMSubclass, AsyncKernelManagerWithCleanup])
+@pytest.fixture(params=[AsyncKernelManager, AsyncKMSubclass])
 def async_km(request, config):
     km = request.param(config=config)
     return km
@@ -467,23 +466,6 @@ async def test_get_connect_info(self, async_km):
         )
         assert keys == expected
 
-    async def test_subclass_deprecations(self, async_km):
-        await async_km.start_kernel(stdout=PIPE, stderr=PIPE)
-        is_alive = await async_km.is_alive()
-        assert is_alive
-        assert isinstance(async_km, AsyncKernelManager)
-        await async_km.shutdown_kernel(now=True)
-        is_alive = await async_km.is_alive()
-        assert is_alive is False
-        assert async_km.context.closed
-
-        if isinstance(async_km, AsyncKernelManagerWithCleanup):
-            assert async_km.which_cleanup == "cleanup"
-        elif isinstance(async_km, AsyncKMSubclass):
-            assert async_km.which_cleanup == "cleanup_resources"
-        else:
-            assert hasattr(async_km, "which_cleanup") is False
-
     @pytest.mark.timeout(10)
     @pytest.mark.skipif(sys.platform == "win32", reason="Windows doesn't support signals")
     async def test_signal_kernel_subprocesses(self, install_kernel, start_async_kernel):

jupyter_client/tests/utils.py

@@ -147,41 +147,15 @@ def cleanup_resources(self, restart=False):
 
 
 class SyncKMSubclass(KMSubclass, KernelManager):
+    """Used to test subclass hierarchies to ensure methods are called when expected."""
+
     _superclass = KernelManager
 
 
 class AsyncKMSubclass(KMSubclass, AsyncKernelManager):
-    """Used to test subclass hierarchies to ensure methods are called when expected.
-
-    This class is also used to test deprecation "routes" that are determined by superclass'
-    detection of methods.
-
-    This class represents a current subclass that overrides "interesting" methods of
-    AsyncKernelManager.
-    """
+    """Used to test subclass hierarchies to ensure methods are called when expected."""
 
     _superclass = AsyncKernelManager
-    which_cleanup = ""  # cleanup deprecation testing
-
-    @subclass_recorder
-    def cleanup(self, connection_file=True):
-        self.which_cleanup = "cleanup"
-
-    @subclass_recorder
-    def cleanup_resources(self, restart=False):
-        self.which_cleanup = "cleanup_resources"
-
-
-class AsyncKernelManagerWithCleanup(AsyncKernelManager):
-    """Used to test deprecation "routes" that are determined by superclass' detection of methods.
-
-    This class represents the older subclass that overrides cleanup().  We should find that
-    cleanup() is called on these instances via TestAsyncKernelManagerWithCleanup.
-    """
-
-    def cleanup(self, connection_file=True):
-        super().cleanup(connection_file=connection_file)
-        self.which_cleanup = "cleanup"
 
 
 class MKMSubclass(RecordCallMixin):