From 5ffec87aa9bf553b23373f3bb1515922fe0ce1e6 Mon Sep 17 00:00:00 2001
From: "Afshin T. Darian" <git@darian.af>
Date: Thu, 21 Jan 2021 16:08:36 +0000
Subject: [PATCH 1/3] Enable notebook ContentsManager in jupyter_server

---
 jupyter_server/serverapp.py                   |  36 +-
 .../services/sessions/sessionmanager.py       |   8 +-
 jupyter_server/traittypes.py                  | 349 ++++++++++++++++++
 tests/test_traittypes.py                      |  80 ++++
 4 files changed, 470 insertions(+), 3 deletions(-)
 create mode 100644 jupyter_server/traittypes.py
 create mode 100644 tests/test_traittypes.py

diff --git a/jupyter_server/serverapp.py b/jupyter_server/serverapp.py
index 7602e4569d..b4b6716f0d 100755
--- a/jupyter_server/serverapp.py
+++ b/jupyter_server/serverapp.py
@@ -31,6 +31,7 @@
 import warnings
 import webbrowser
 import urllib
+import inspect
 
 from base64 import encodebytes
 try:
@@ -106,6 +107,7 @@
 from jupyter_server.extension.serverextension import ServerExtensionApp
 from jupyter_server.extension.manager import ExtensionManager
 from jupyter_server.extension.config import ExtensionConfigManager
+from jupyter_server.traittypes import TypeFromClasses
 
 #-----------------------------------------------------------------------------
 # Module globals
@@ -1134,13 +1136,43 @@ def template_file_path(self):
         help="""If True, display controls to shut down the Jupyter server, such as menu items or buttons."""
     )
 
-    contents_manager_class = Type(
+    # REMOVE in VERSION 2.0
+    # Temporarily allow content managers to inherit from the 'notebook'
+    # package. We will deprecate this in the next major release.
+    contents_manager_class = TypeFromClasses(
         default_value=LargeFileManager,
-        klass=ContentsManager,
+        klasses=[
+            'jupyter_server.services.contents.manager.ContentsManager',
+            'notebook.services.contents.manager.ContentsManager'
+        ],
         config=True,
         help=_('The content manager class to use.')
     )
 
+    # Throws a deprecation warning to notebook based contents managers.
+    @observe('contents_manager_class')
+    def _observe_contents_manager_class(self, change):
+        new = change['new']
+        # If 'new' is a class, get a string representing the import
+        # module path.
+        if inspect.isclass(new):
+            new = new.__module__
+
+        if new.startswith('notebook'):
+            self.log.warn(
+                "The specified 'contents_manager_class' class inherits a manager from the "
+                "'notebook' package. This is not guaranteed to work in future "
+                "releases of Jupyter Server. Instead, consider switching the "
+                "manager to inherit from the 'jupyter_server' managers. "
+                "Jupyter Server will temporarily allow 'notebook' managers "
+                "until its next major release (2.x)."
+            )
+
+    @observe('notebook_dir')
+    def _update_notebook_dir(self, change):
+        self.log.warning(_("notebook_dir is deprecated, use root_dir"))
+        self.root_dir = change['new']
+
     kernel_manager_class = Type(
         default_value=MappingKernelManager,
         config=True,
diff --git a/jupyter_server/services/sessions/sessionmanager.py b/jupyter_server/services/sessions/sessionmanager.py
index 9d59ba7150..eeba4a352d 100644
--- a/jupyter_server/services/sessions/sessionmanager.py
+++ b/jupyter_server/services/sessions/sessionmanager.py
@@ -18,12 +18,18 @@
 from traitlets import Instance
 
 from jupyter_server.utils import ensure_async
+from jupyter_server.traittypes import InstanceFromClasses
 
 
 class SessionManager(LoggingConfigurable):
 
     kernel_manager = Instance('jupyter_server.services.kernels.kernelmanager.MappingKernelManager')
-    contents_manager = Instance('jupyter_server.services.contents.manager.ContentsManager')
+    contents_manager = InstanceFromClasses(
+        [
+            'jupyter_server.services.contents.manager.ContentsManager',
+            'notebook.services.contents.manager.ContentsManager'
+        ]
+    )
 
     # Session database initialized below
     _cursor = None
diff --git a/jupyter_server/traittypes.py b/jupyter_server/traittypes.py
new file mode 100644
index 0000000000..7076d875dd
--- /dev/null
+++ b/jupyter_server/traittypes.py
@@ -0,0 +1,349 @@
+import inspect
+from traitlets import ClassBasedTraitType, Undefined, warn
+
+# Traitlet's 5.x includes a set of utilities for building
+# description strings for objects. Traitlets 5.x does not
+# support Python 3.6, but jupyter_server does; instead
+# jupyter_server uses traitlets 4.3.x which doesn't have
+# this `descriptions` submodule. This chunk in the except
+# clause is a copy-and-paste from traitlets 5.0.5.
+try:
+    from traitlets.utils.descriptions import describe
+except ImportError:
+    import inspect
+    import re
+    import types
+
+    def describe(article, value, name=None, verbose=False, capital=False):
+        """Return string that describes a value
+        Parameters
+        ----------
+        article : str or None
+            A definite or indefinite article. If the article is
+            indefinite (i.e. "a" or "an") the appropriate one
+            will be infered. Thus, the arguments of ``describe``
+            can themselves represent what the resulting string
+            will actually look like. If None, then no article
+            will be prepended to the result. For non-articled
+            description, values that are instances are treated
+            definitely, while classes are handled indefinitely.
+        value : any
+            The value which will be named.
+        name : str or None (default: None)
+            Only applies when ``article`` is "the" - this
+            ``name`` is a definite reference to the value.
+            By default one will be infered from the value's
+            type and repr methods.
+        verbose : bool (default: False)
+            Whether the name should be concise or verbose. When
+            possible, verbose names include the module, and/or
+            class name where an object was defined.
+        capital : bool (default: False)
+            Whether the first letter of the article should
+            be capitalized or not. By default it is not.
+        Examples
+        --------
+        Indefinite description:
+        >>> describe("a", object())
+        'an object'
+        >>> describe("a", object)
+        'an object'
+        >>> describe("a", type(object))
+        'a type'
+        Definite description:
+        >>> describe("the", object())
+        "the object at '0x10741f1b0'"
+        >>> describe("the", object)
+        "the type 'object'"
+        >>> describe("the", type(object))
+        "the type 'type'"
+        Definitely named description:
+        >>> describe("the", object(), "I made")
+        'the object I made'
+        >>> describe("the", object, "I will use")
+        'the object I will use'
+        """
+        if isinstance(article, str):
+            article = article.lower()
+
+        if not inspect.isclass(value):
+            typename = type(value).__name__
+        else:
+            typename = value.__name__
+        if verbose:
+            typename = _prefix(value) + typename
+
+        if article == "the" or (article is None and not inspect.isclass(value)):
+            if name is not None:
+                result = "{} {}".format(typename, name)
+                if article is not None:
+                    return add_article(result, True, capital)
+                else:
+                    return result
+            else:
+                tick_wrap = False
+                if inspect.isclass(value):
+                    name = value.__name__
+                elif isinstance(value, types.FunctionType):
+                    name = value.__name__
+                    tick_wrap = True
+                elif isinstance(value, types.MethodType):
+                    name = value.__func__.__name__
+                    tick_wrap = True
+                elif type(value).__repr__ in (object.__repr__, type.__repr__):
+                    name = "at '%s'" % hex(id(value))
+                    verbose = False
+                else:
+                    name = repr(value)
+                    verbose = False
+                if verbose:
+                    name = _prefix(value) + name
+                if tick_wrap:
+                    name = name.join("''")
+                return describe(article, value, name=name,
+                    verbose=verbose, capital=capital)
+        elif article in ("a", "an") or article is None:
+            if article is None:
+                return typename
+            return add_article(typename, False, capital)
+        else:
+            raise ValueError("The 'article' argument should "
+                "be 'the', 'a', 'an', or None not %r" % article)
+
+
+    def add_article(name, definite=False, capital=False):
+        """Returns the string with a prepended article.
+        The input does not need to begin with a charater.
+        Parameters
+        ----------
+        definite : bool (default: False)
+            Whether the article is definite or not.
+            Indefinite articles being 'a' and 'an',
+            while 'the' is definite.
+        capital : bool (default: False)
+            Whether the added article should have
+            its first letter capitalized or not.
+        """
+        if definite:
+            result = "the " + name
+        else:
+            first_letters = re.compile(r'[\W_]+').sub('', name)
+            if first_letters[:1].lower() in 'aeiou':
+                result = 'an ' + name
+            else:
+                result = 'a ' + name
+        if capital:
+            return result[0].upper() + result[1:]
+        else:
+            return result
+
+
+class TypeFromClasses(ClassBasedTraitType):
+    """A trait whose value must be a subclass of a class in a specified list of classes."""
+
+    def __init__(self, default_value=Undefined, klasses=None, **kwargs):
+        """Construct a Type trait
+        A Type trait specifies that its values must be subclasses of
+        a class in a list of possible classes.
+        If only ``default_value`` is given, it is used for the ``klasses`` as
+        well. If neither are given, both default to ``object``.
+        Parameters
+        ----------
+        default_value : class, str or None
+            The default value must be a subclass of klass.  If an str,
+            the str must be a fully specified class name, like 'foo.bar.Bah'.
+            The string is resolved into real class, when the parent
+            :class:`HasTraits` class is instantiated.
+        klasses : list of class, str [ default object ]
+            Values of this trait must be a subclass of klass.  The klass
+            may be specified in a string like: 'foo.bar.MyClass'.
+            The string is resolved into real class, when the parent
+            :class:`HasTraits` class is instantiated.
+        allow_none : bool [ default False ]
+            Indicates whether None is allowed as an assignable value.
+        """
+        if default_value is Undefined:
+            new_default_value = object if (klasses is None) else klasses
+        else:
+            new_default_value = default_value
+
+        if klasses is None:
+            if (default_value is None) or (default_value is Undefined):
+                klasses = [object]
+            else:
+                klasses = [default_value]
+
+        # OneOfType requires a list of klasses to be specified (different than Type).
+        if not isinstance(klasses, (list, tuple, set)):
+            raise TraitError("`klasses` must be a list of class names (type is str) or classes.")
+
+        for klass in klasses:
+            if not (inspect.isclass(klass) or isinstance(klass, str)):
+                raise TraitError("A OneOfType trait must specify a list of classes.")
+
+        # Store classes.
+        self.klasses = klasses
+
+        super().__init__(new_default_value, **kwargs)
+
+    def subclass_from_klasses(self, value):
+        "Check that a given class is a subclasses found in the klasses list."
+        return any(issubclass(value, klass) for klass in self.importable_klasses)
+
+    def validate(self, obj, value):
+        """Validates that the value is a valid object instance."""
+        if isinstance(value, str):
+            try:
+                value = self._resolve_string(value)
+            except ImportError:
+                raise TraitError("The '%s' trait of %s instance must be a type, but "
+                                 "%r could not be imported" % (self.name, obj, value))
+        try:
+            if self.subclass_from_klasses(value):
+                return value
+        except Exception:
+            pass
+
+        self.error(obj, value)
+
+    def info(self):
+        """Returns a description of the trait."""
+        result = "a subclass of "
+        for klass in self.klasses:
+            if not isinstance(klass, str):
+                klass = klass.__module__ + '.' + klass.__name__
+            result += f"{klass} or "
+        # Strip the last "or"
+        result = result.strip(" or ")
+        if self.allow_none:
+            return result + ' or None'
+        return result
+
+    def instance_init(self, obj):
+        self._resolve_classes()
+        super().instance_init(obj)
+
+    def _resolve_classes(self):
+        # Resolve all string names to actual classes.
+        self.importable_klasses = []
+        for klass in self.klasses:
+            if isinstance(klass, str):
+                try:
+                    klass = self._resolve_string(klass)
+                    self.importable_klasses.append(klass)
+                except:
+                    warn(f"{klass} is not importable. Is it installed?", ImportWarning)
+            else:
+                self.importable_klasses.append(klass)
+
+        if isinstance(self.default_value, str):
+            self.default_value = self._resolve_string(self.default_value)
+
+    def default_value_repr(self):
+        value = self.default_value
+        if isinstance(value, str):
+            return repr(value)
+        else:
+            return repr(f'{value.__module__}.{value.__name__}')
+
+
+class InstanceFromClasses(ClassBasedTraitType):
+    """A trait whose value must be an instance of a class in a specified list of classes.
+    The value can also be an instance of a subclass of the specified classes.
+    Subclasses can declare default classes by overriding the klass attribute
+    """
+    def __init__(self, klasses=None, args=None, kw=None, **kwargs):
+        """Construct an Instance trait.
+        This trait allows values that are instances of a particular
+        class or its subclasses.  Our implementation is quite different
+        from that of enthough.traits as we don't allow instances to be used
+        for klass and we handle the ``args`` and ``kw`` arguments differently.
+        Parameters
+        ----------
+        klasses : list of classes or class_names (str)
+            The class that forms the basis for the trait.  Class names
+            can also be specified as strings, like 'foo.bar.Bar'.
+        args : tuple
+            Positional arguments for generating the default value.
+        kw : dict
+            Keyword arguments for generating the default value.
+        allow_none : bool [ default False ]
+            Indicates whether None is allowed as a value.
+        Notes
+        -----
+        If both ``args`` and ``kw`` are None, then the default value is None.
+        If ``args`` is a tuple and ``kw`` is a dict, then the default is
+        created as ``klass(*args, **kw)``.  If exactly one of ``args`` or ``kw`` is
+        None, the None is replaced by ``()`` or ``{}``, respectively.
+        """
+        # If class
+        if klasses is None:
+            self.klasses = klasses
+        # Verify all elements are either classes or strings.
+        elif all(inspect.isclass(k) or isinstance(k, str) for k in klasses):
+            self.klasses = klasses
+        else:
+            raise TraitError('The klasses attribute must be a list of class names or classes'
+                                ' not: %r' % klass)
+
+        if (kw is not None) and not isinstance(kw, dict):
+            raise TraitError("The 'kw' argument must be a dict or None.")
+        if (args is not None) and not isinstance(args, tuple):
+            raise TraitError("The 'args' argument must be a tuple or None.")
+
+        self.default_args = args
+        self.default_kwargs = kw
+
+        super(InstanceFromClasses, self).__init__(**kwargs)
+
+    def instance_from_importable_klasses(self, value):
+        "Check that a given class is a subclasses found in the klasses list."
+        return any(isinstance(value, klass) for klass in self.importable_klasses)
+
+    def validate(self, obj, value):
+        if self.instance_from_importable_klasses(value):
+            return value
+        else:
+            self.error(obj, value)
+
+    def info(self):
+        result = "an instance of "
+        for klass in self.klasses:
+            if isinstance(klass, str):
+                result += klass
+            else:
+                result += describe("a", klass)
+            result += " or "
+        result = result.strip(" or ")
+        if self.allow_none:
+            result += ' or None'
+        return result
+
+    def instance_init(self, obj):
+        self._resolve_classes()
+        super().instance_init(obj)
+
+    def _resolve_classes(self):
+        # Resolve all string names to actual classes.
+        self.importable_klasses = []
+        for klass in self.klasses:
+            if isinstance(klass, str):
+                try:
+                    klass = self._resolve_string(klass)
+                    self.importable_klasses.append(klass)
+                except:
+                    warn(f"{klass} is not importable. Is it installed?", ImportWarning)
+            else:
+                self.importable_klasses.append(klass)
+
+    def make_dynamic_default(self):
+        if (self.default_args is None) and (self.default_kwargs is None):
+            return None
+        return self.klass(*(self.default_args or ()),
+                          **(self.default_kwargs or {}))
+
+    def default_value_repr(self):
+        return repr(self.make_dynamic_default())
+
+    def from_string(self, s):
+        return _safe_literal_eval(s)
diff --git a/tests/test_traittypes.py b/tests/test_traittypes.py
new file mode 100644
index 0000000000..a0f9e2a7df
--- /dev/null
+++ b/tests/test_traittypes.py
@@ -0,0 +1,80 @@
+import pytest
+from traitlets import HasTraits, TraitError
+from traitlets.utils.importstring import import_item
+
+from jupyter_server.traittypes import (
+    InstanceFromClasses,
+    TypeFromClasses
+)
+from jupyter_server.services.contents.largefilemanager import LargeFileManager
+
+
+class DummyClass:
+    """Dummy class for testing Instance"""
+
+
+class DummyInt(int):
+    """Dummy class for testing types."""
+
+
+class Thing(HasTraits):
+
+    a = InstanceFromClasses(
+        default_value=2,
+        klasses=[
+            int,
+            str,
+            DummyClass,
+        ]
+    )
+
+    b = TypeFromClasses(
+        default_value=None,
+        allow_none=True,
+        klasses=[
+            DummyClass,
+            int,
+            'jupyter_server.services.contents.manager.ContentsManager'
+        ]
+    )
+
+
+class TestInstanceFromClasses:
+
+    @pytest.mark.parametrize(
+        'value',
+        [1, 'test', DummyClass()]
+    )
+    def test_good_values(self, value):
+        thing = Thing(a=value)
+        assert thing.a == value
+
+    @pytest.mark.parametrize(
+        'value',
+        [2.4, object()]
+    )
+    def test_bad_values(self, value):
+        with pytest.raises(TraitError) as e:
+            thing = Thing(a=value)
+
+
+class TestTypeFromClasses:
+
+    @pytest.mark.parametrize(
+        'value',
+        [DummyClass, DummyInt, LargeFileManager,
+            'jupyter_server.services.contents.manager.ContentsManager']
+    )
+    def test_good_values(self, value):
+        thing = Thing(b=value)
+        if isinstance(value, str):
+            value = import_item(value)
+        assert thing.b == value
+
+    @pytest.mark.parametrize(
+        'value',
+        [float, object]
+    )
+    def test_bad_values(self, value):
+        with pytest.raises(TraitError) as e:
+            thing = Thing(b=value)

From a45a53b84069fad6e3692e22fa1355bc63c87095 Mon Sep 17 00:00:00 2001
From: Zachary Sailer <zachsailer@gmail.com>
Date: Thu, 21 Jan 2021 10:07:48 -0800
Subject: [PATCH 2/3] Update jupyter_server/serverapp.py

Co-authored-by: Kevin Bates <kbates4@gmail.com>
---
 jupyter_server/serverapp.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/jupyter_server/serverapp.py b/jupyter_server/serverapp.py
index b4b6716f0d..10c284974b 100755
--- a/jupyter_server/serverapp.py
+++ b/jupyter_server/serverapp.py
@@ -1159,7 +1159,7 @@ def _observe_contents_manager_class(self, change):
             new = new.__module__
 
         if new.startswith('notebook'):
-            self.log.warn(
+            self.log.warning(
                 "The specified 'contents_manager_class' class inherits a manager from the "
                 "'notebook' package. This is not guaranteed to work in future "
                 "releases of Jupyter Server. Instead, consider switching the "

From 7978e46e69d42ff26c09d85341f4fc19ce060575 Mon Sep 17 00:00:00 2001
From: "Afshin T. Darian" <git@darian.af>
Date: Fri, 29 Jan 2021 19:01:24 +0000
Subject: [PATCH 3/3] Fix tests

---
 .github/workflows/python-windows.yml | 7 ++++++-
 jupyter_server/traittypes.py         | 6 ++++--
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/.github/workflows/python-windows.yml b/.github/workflows/python-windows.yml
index 2e476e77f2..5e7cb06693 100644
--- a/.github/workflows/python-windows.yml
+++ b/.github/workflows/python-windows.yml
@@ -44,7 +44,12 @@ jobs:
         pip check
     - name: Run the tests
       run: |
-        pytest -vv
+        # Disable capturing (-s) output from Pytest on Windows.
+        # For an unknown reason, capturing output interferes with
+        # the file descriptions opened by the asyncio IOLoop.
+        # This leads to a nasty, flaky race condition that we haven't
+        # been able to solve.
+        pytest -vv -s
     - name: Install the Python dependencies for the examples
       run: |
         cd examples/simple && pip install -e .
diff --git a/jupyter_server/traittypes.py b/jupyter_server/traittypes.py
index 7076d875dd..8e8cb3b3ff 100644
--- a/jupyter_server/traittypes.py
+++ b/jupyter_server/traittypes.py
@@ -228,11 +228,12 @@ def _resolve_classes(self):
         self.importable_klasses = []
         for klass in self.klasses:
             if isinstance(klass, str):
+                # Try importing the classes to compare. Silently, ignore if not importable.
                 try:
                     klass = self._resolve_string(klass)
                     self.importable_klasses.append(klass)
                 except:
-                    warn(f"{klass} is not importable. Is it installed?", ImportWarning)
+                    pass
             else:
                 self.importable_klasses.append(klass)
 
@@ -328,11 +329,12 @@ def _resolve_classes(self):
         self.importable_klasses = []
         for klass in self.klasses:
             if isinstance(klass, str):
+                # Try importing the classes to compare. Silently, ignore if not importable.
                 try:
                     klass = self._resolve_string(klass)
                     self.importable_klasses.append(klass)
                 except:
-                    warn(f"{klass} is not importable. Is it installed?", ImportWarning)
+                    pass
             else:
                 self.importable_klasses.append(klass)