Merge pull request #643 from kevin-bates/remove-deprecations

[Release 7.0] Remove deprecations in kernel manager

Author: blink1073

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
@@ -129,27 +127,6 @@ def kernel_spec(self) -> t.Optional[kernelspec.KernelSpec]:
             self._kernel_spec = self.kernel_spec_manager.get_kernel_spec(self.kernel_name)
         return self._kernel_spec
 
-    kernel_cmd: List = List(
-        Unicode(),
-        config=True,
-        help="""DEPRECATED: Use kernel_name instead.
-
-        The Popen Command to launch the kernel.
-        Override this if you have a custom kernel.
-        If kernel_cmd is specified in a configuration file,
-        Jupyter does not pass any arguments to the kernel,
-        because it cannot make any assumptions about the
-        arguments that the kernel understands. In particular,
-        this means that the kernel does not receive the
-        option --debug if it given on the Jupyter command line.
-        """,
-    )
-
-    def _kernel_cmd_changed(self, name, old, new):
-        warnings.warn(
-            "Setting kernel_cmd is deprecated, use kernel_spec to " "start different kernels."
-        )
-
     cache_ports: Bool = Bool(
         help="True if the MultiKernelManager should cache ports for this KernelManager instance"
     )
@@ -226,11 +203,8 @@ def client(self, **kwargs) -> KernelClient:
     def format_kernel_cmd(self, extra_arguments: t.Optional[t.List[str]] = None) -> t.List[str]:
         """replace templated args (e.g. {connection_file})"""
         extra_arguments = extra_arguments or []
-        if self.kernel_cmd:
-            cmd = self.kernel_cmd + extra_arguments
-        else:
-            assert self.kernel_spec is not None
-            cmd = self.kernel_spec.argv + extra_arguments
+        assert self.kernel_spec is not None
+        cmd = self.kernel_spec.argv + extra_arguments
 
         if cmd and cmd[0] in {
             "python",
@@ -324,11 +298,10 @@ def pre_start_kernel(self, **kw) -> t.Tuple[t.List[str], t.Dict[str, t.Any]]:
         # Don't allow PYTHONEXECUTABLE to be passed to kernel process.
         # If set, it can bork all the things.
         env.pop("PYTHONEXECUTABLE", None)
-        if not self.kernel_cmd:
-            # If kernel_cmd has been set manually, don't refer to a kernel spec.
-            # Environment variables from kernel spec are added to os.environ.
-            assert self.kernel_spec is not None
-            env.update(self._get_env_substitutions(self.kernel_spec.env, env))
+
+        # Environment variables from kernel spec are added to os.environ.
+        assert self.kernel_spec is not None
+        env.update(self._get_env_substitutions(self.kernel_spec.env, env))
 
         kw["env"] = env
         return kernel_cmd, kw
@@ -435,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.
 
@@ -477,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):