Repository: jupyter/jupyter_client
Pull request #622 (2021-03-07 23:22:31)

Remove dependency on ipython_genutils.
IPython Genutils seem to start to be troublesome to package for some Linux
Ditributions because of Python2/3 compat. Get rid of it.

It was not in the dependencies anyway...

davidbrochart: LGTM
Carreau: Thanks @davidbrochart 

I'm going to get that in soon and do a release unless there are objections then.
mcepl: > I agree that the changes look good. My only question is what is the intention of this moving forward? Is the rest of the Jupyter stack supposed to remove what `ipython_genutils` references they can, then reply on `jupyter_client` for these "utils" methods? If so, these strike me as better-suited for `jupyter_core.paths` and, perhaps a new `jupyter_core.import_string` module. That is, let `jupyter_core` be the library rather than `jupyter_client`. I'm curious why client was picked over core - unless the intention is to let each package duplicate their own versions of the utilities.

See https://github.com/ipython/ipython_genutils/issues/17 and https://github.com/ipython/ipython/pull/12840 ... aside from python-ipykernel I think we at least see the light in the end of the tunnel of converting whole ipython/jupyter universe from their own testing utilities and ``nose`` towards standard supported ones (``unittest`` more or less sprinkled with ``pytest``, some components heavily relying on it, some are pure ``unittest``).
Carreau: Yes, the point is for each package to duplicate version of the utilities if they need to; ipython_genutils is an artifact of [The big split (2015)](https://blog.jupyter.org/the-big-split-9d7b88a031a7) and the complexity at the time to support both  Python 2 and 3 (as it predates `six`). 

In many cases I even expect ipython_genutils to be replaced by no-op.
 - here I'm for example pretty certain that `info['key']` will always be str,
 - most import logic can be replaced by `imp`/`importlib`. ( I expect most to actually be replaced by traitlets, as traitlet's `Any`, or `DottedObjectName`  do the same with properties and `import_item` exists in `traitlets.utils.importstring`, though I didn't wanted to potentially introduce any differences, so haven't done that here).
 - Out of 250 imports in my dev folder 144 are ipython_genutils.py3compat (so can likely be removed)
 - many are nose decorators (`skip_if_no_x11`, `skip_if_not_win32`, that will need removal/update as nose will AFAICT not be compatible Python 3.10).
 - Other misc things that are now part of Core CPython.
kevin-bates: Thank you for the responses @mcepl and @Carreau - they were helpful.  I agree that most will fall out.  

My comment was more about the methods added to `utils` - which will be imported into downstream packages.  We had this same issue with `secure_write` (moved from N locations into `jupyter_core`) and it was painful.  I'm just trying to ensure the same doesn't happen here.

> import_item exists in traitlets.utils.importstring, though I didn't wanted to potentially introduce any differences, so haven't done that here

Thanks for the hint that import_item also exists in traitlets.  The only difference between what you've added here and the version in traitlets is that the [parameter is validated to be a string](https://github.com/ipython/traitlets/blob/master/traitlets/utils/importstring.py#L24-L25) in the traitlets version. Since all of the uses of `import_item` in jupyter_client pass a string and jupyter_client has a dependency on traitlets - this seems like a good example of unnecessary duplication.
Carreau: Good point, added a commit with the following:

    Use import_item from traitlets,

    As pointed out by Kevin, the method is already there in traitlets and
    does more validation that the one in ipython_genutils.

    I thought it was not in traitlets 4.x; but is indeed there and has been
    for about 5 years.

And some import reshuffling to group similar imports from various packages. 

I hear your concern about duplicated functionality; in particular for `filefind`; though I would argue that filefind is _in this specific case_ wrong in particular because it still has some IPython specific behavior and expand specific values like IPYTHON_TEMP, and remove trailing/leading quotes because of magics. Unbundling it would allow to remove those behaviors;  But I think that would be a candidate for another PR.

I can make utils private if that makes you more comfortable ?
Carreau: I've made the two functions in utils private.
Carreau: Failures appear to be due to the new 6.0.0a0 of ipykernel. 
kevin-bates: > Failures appear to be due to the new 6.0.0a0 of ipykernel.

I agree.  In the meantime, should we adjust our CI dependency installation to not pull with `--pre`, or do you know if the issues understood and will be addressed soon?

Patch:
diff --git a/docs/changelog.rst b/docs/changelog.rst
index fb8ce84..498a2f0 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -9,7 +9,18 @@ dev
 
 - Shutdown request sequence has been modified to be more graceful, it now is
   preceded by interrupt, and will also send a ``SIGTERM`` before forcibly
-  killing the kernel. :ghpull:`620`
+  killing the kernel. (:ghpull:`620`)
+
+- Removal of ``ipython_genutils`` as a dependency. It was implicit before; but
+  required by at least traitlets thus avoiding issues. We are working on
+  completely removing it from all jupyter dependencies; as it might lead to
+  issues packaging for Python 3.10, and was mostly used for compatibility with
+  python 2.  (:ghpull:`620`, :ghpull:`605`)
+
+- Address a race condition between ``shutdown_kernel`` and restarter.
+  (:ghpull:`607`.)
+
+See the `full list of pull-requests<https://github.com/jupyter/jupyter_client/milestone/24?closed=1>`_
 
 6.1.11
 ======
diff --git a/jupyter_client/connect.py b/jupyter_client/connect.py
index 4a41af0..e31f9ec 100644
--- a/jupyter_client/connect.py
+++ b/jupyter_client/connect.py
@@ -22,13 +22,13 @@ import zmq
 
 from traitlets.config import LoggingConfigurable
 from .localinterfaces import localhost
-from ipython_genutils.path import filefind
-from ipython_genutils.py3compat import cast_bytes
 from traitlets import (
     Bool, Integer, Unicode, CaselessStrEnum, Instance, Type, observe
 )
 from jupyter_core.paths import jupyter_data_dir, jupyter_runtime_dir, secure_write
 
+from .utils import _filefind
+
 
 def write_connection_file(fname=None, shell_port=0, iopub_port=0, stdin_port=0, hb_port=0,
                           control_port=0, ip='', key=b'', transport='tcp',
@@ -195,7 +195,7 @@ def find_connection_file(filename='kernel-*.json', path=None, profile=None):
 
     try:
         # first, try explicit name
-        return filefind(filename, path)
+        return _filefind(filename, path)
     except IOError:
         pass
 
@@ -515,8 +515,13 @@ class ConnectionFileMixin(LoggingConfigurable):
                 # not overridden by config or cl_args
                 setattr(self, name, info[name])
 
-        if 'key' in info:
-            self.session.key = cast_bytes(info['key'])
+        if "key" in info:
+            key = info["key"]
+            if isinstance(key, str):
+                key = key.encode()
+            assert isinstance(key, bytes)
+
+            self.session.key = key
         if 'signature_scheme' in info:
             self.session.signature_scheme = info['signature_scheme']
 
diff --git a/jupyter_client/consoleapp.py b/jupyter_client/consoleapp.py
index 033b1fe..42ce2fb 100644
--- a/jupyter_client/consoleapp.py
+++ b/jupyter_client/consoleapp.py
@@ -16,7 +16,6 @@ import warnings
 
 
 from traitlets.config.application import boolean_flag
-from ipython_genutils.path import filefind
 from traitlets import (
     Dict, List, Unicode, CUnicode, CBool, Any, Type
 )
@@ -32,6 +31,7 @@ from .session import Session
 ConnectionFileMixin = connect.ConnectionFileMixin
 
 from .localinterfaces import localhost
+from .utils import _filefind
 
 #-----------------------------------------------------------------------------
 # Aliases and Flags
@@ -186,7 +186,9 @@ class JupyterConsoleApp(ConnectionFileMixin):
                     cf = self.connection_file
                 self.connection_file = cf
         try:
-            self.connection_file = filefind(self.connection_file, ['.', self.runtime_dir])
+            self.connection_file = _filefind(
+                self.connection_file, [".", self.runtime_dir]
+            )
         except IOError:
             self.log.debug("Connection File not found: %s", self.connection_file)
             return
diff --git a/jupyter_client/manager.py b/jupyter_client/manager.py
index 434f79a..cf9fb5d 100644
--- a/jupyter_client/manager.py
+++ b/jupyter_client/manager.py
@@ -16,12 +16,12 @@ from enum import Enum
 
 import zmq
 
-from ipython_genutils.importstring import import_item
 from .localinterfaces import is_local_ip, local_ips
 from traitlets import (
     Any, Float, Instance, Unicode, List, Bool, Type, DottedObjectName,
     default, observe, observe_compat
 )
+from traitlets.utils.importstring import import_item
 from jupyter_client import (
     launch_kernel,
     kernelspec,
diff --git a/jupyter_client/multikernelmanager.py b/jupyter_client/multikernelmanager.py
index a04c970..5907d7b 100644
--- a/jupyter_client/multikernelmanager.py
+++ b/jupyter_client/multikernelmanager.py
@@ -11,7 +11,7 @@ import socket
 import zmq
 
 from traitlets.config.configurable import LoggingConfigurable
-from ipython_genutils.importstring import import_item
+from traitlets.utils.importstring import import_item
 from traitlets import (
     Any, Bool, Dict, DottedObjectName, Instance, Unicode, default, observe
 )
diff --git a/jupyter_client/session.py b/jupyter_client/session.py
index 4f6713f..88fc474 100644
--- a/jupyter_client/session.py
+++ b/jupyter_client/session.py
@@ -21,14 +21,15 @@ import pickle
 import pprint
 import random
 import warnings
+
 from datetime import datetime
+from datetime import timezone
 
 PICKLE_PROTOCOL = pickle.DEFAULT_PROTOCOL
 
 # We are using compare_digest to limit the surface of timing attacks
 from hmac import compare_digest
 
-from datetime import timezone
 utc = timezone.utc
 
 import zmq
@@ -36,17 +37,18 @@ from zmq.utils import jsonapi
 from zmq.eventloop.ioloop import IOLoop
 from zmq.eventloop.zmqstream import ZMQStream
 
-from traitlets.config.configurable import Configurable, LoggingConfigurable
-from ipython_genutils.importstring import import_item
+
 from jupyter_client.jsonutil import extract_dates, squash_dates, date_default
+from jupyter_client import protocol_version
+from jupyter_client.adapter import adapt
+
 from traitlets import (
     CBytes, Unicode, Bool, Any, Instance, Set, DottedObjectName, CUnicode,
     Dict, Integer, TraitError, observe
 )
-from jupyter_client import protocol_version
-from jupyter_client.adapter import adapt
 from traitlets.log import get_logger
-
+from traitlets.utils.importstring import import_item
+from traitlets.config.configurable import Configurable, LoggingConfigurable
 
 #-----------------------------------------------------------------------------
 # utility functions
diff --git a/jupyter_client/utils.py b/jupyter_client/utils.py
new file mode 100644
index 0000000..2f49d21
--- /dev/null
+++ b/jupyter_client/utils.py
@@ -0,0 +1,84 @@
+"""
+Utils vendored from ipython_genutils that should be retired at some point.
+"""
+
+import os
+
+
+def _filefind(filename, path_dirs=None):
+    """Find a file by looking through a sequence of paths.
+
+    This iterates through a sequence of paths looking for a file and returns
+    the full, absolute path of the first occurence of the file.  If no set of
+    path dirs is given, the filename is tested as is, after running through
+    :func:`expandvars` and :func:`expanduser`.  Thus a simple call::
+
+        filefind('myfile.txt')
+
+    will find the file in the current working dir, but::
+
+        filefind('~/myfile.txt')
+
+    Will find the file in the users home directory.  This function does not
+    automatically try any paths, such as the cwd or the user's home directory.
+
+    Parameters
+    ----------
+    filename : str
+        The filename to look for.
+    path_dirs : str, None or sequence of str
+        The sequence of paths to look for the file in.  If None, the filename
+        need to be absolute or be in the cwd.  If a string, the string is
+        put into a sequence and the searched.  If a sequence, walk through
+        each element and join with ``filename``, calling :func:`expandvars`
+        and :func:`expanduser` before testing for existence.
+
+    Returns
+    -------
+    Raises :exc:`IOError` or returns absolute path to file.
+    """
+
+    # If paths are quoted, abspath gets confused, strip them...
+    filename = filename.strip('"').strip("'")
+    # If the input is an absolute path, just check it exists
+    if os.path.isabs(filename) and os.path.isfile(filename):
+        return filename
+
+    if path_dirs is None:
+        path_dirs = ("",)
+    elif isinstance(path_dirs, str):
+        path_dirs = (path_dirs,)
+
+    for path in path_dirs:
+        if path == ".":
+            path = os.getcwd()
+        testname = _expand_path(os.path.join(path, filename))
+        if os.path.isfile(testname):
+            return os.path.abspath(testname)
+
+    raise IOError(
+        "File {!r} does not exist in any of the search paths: {!r}".format(filename, path_dirs)
+    )
+
+
+def _expand_path(s):
+    """Expand $VARS and ~names in a string, like a shell
+
+    :Examples:
+
+       In [2]: os.environ['FOO']='test'
+
+       In [3]: expand_path('variable FOO is $FOO')
+       Out[3]: 'variable FOO is test'
+    """
+    # This is a pretty subtle hack. When expand user is given a UNC path
+    # on Windows (\\server\share$\%username%), os.path.expandvars, removes
+    # the $ to get (\\server\share\%username%). I think it considered $
+    # alone an empty var. But, we need the $ to remains there (it indicates
+    # a hidden share).
+    if os.name == "nt":
+        s = s.replace("$\\", "IPYTHON_TEMP")
+    s = os.path.expandvars(os.path.expanduser(s))
+    if os.name == "nt":
+        s = s.replace("IPYTHON_TEMP", "$\\")
+    return s


Test patch:
diff --git a/jupyter_client/tests/test_connect.py b/jupyter_client/tests/test_connect.py
index 0a5c989..fe0f353 100644
--- a/jupyter_client/tests/test_connect.py
+++ b/jupyter_client/tests/test_connect.py
@@ -14,13 +14,33 @@ from traitlets.config import Config
 from jupyter_core.application import JupyterApp
 from jupyter_core.paths import jupyter_runtime_dir
 from tempfile import TemporaryDirectory
-from ipython_genutils.tempdir import TemporaryWorkingDirectory
 from jupyter_client import connect, KernelClient
 from jupyter_client.consoleapp import JupyterConsoleApp
 from jupyter_client.session import Session
 from jupyter_client.connect import secure_write
 
 
+class TemporaryWorkingDirectory(TemporaryDirectory):
+    """
+    Creates a temporary directory and sets the cwd to that directory.
+    Automatically reverts to previous cwd upon cleanup.
+    Usage example:
+
+        with TemporaryWorkingDirectory() as tmpdir:
+            ...
+    """
+
+    def __enter__(self):
+        self.old_wd = os.getcwd()
+        os.chdir(self.name)
+        return super().__enter__()
+
+    def __exit__(self, exc, value, tb):
+        os.chdir(self.old_wd)
+        return super().__exit__(exc, value, tb)
+
+
+
 class DummyConsoleApp(JupyterApp, JupyterConsoleApp):
     def initialize(self, argv=[]):
         JupyterApp.initialize(self, argv=argv)