diff --git a/notebook/notebookapp.py b/notebook/notebookapp.py index 78d1d57d76..b7ce5c2cee 100755 --- a/notebook/notebookapp.py +++ b/notebook/notebookapp.py @@ -121,6 +121,7 @@ urlencode_unix_socket_path, urljoin, ) +from .traittypes import TypeFromClasses # Check if we can use async kernel management try: @@ -1379,13 +1380,41 @@ def _update_mathjax_config(self, change): (shutdown the notebook server).""" ) - contents_manager_class = Type( + # We relax this trait to handle Contents Managers using jupyter_server + # as the core backend. + contents_manager_class = TypeFromClasses( default_value=LargeFileManager, - klass=ContentsManager, + klasses=[ + ContentsManager, + # To make custom ContentsManagers both forward+backward + # compatible, we'll relax the strictness of this trait + # and allow jupyter_server contents managers to pass + # through. If jupyter_server is not installed, this class + # will be ignored. + 'jupyter_server.contents.services.managers.ContentsManager' + ], config=True, help=_('The notebook manager class to use.') ) + # Throws a deprecation warning to jupyter_server 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('jupyter_server'): + self.log.warning( + "The specified 'contents_manager_class' class inherits a manager from the " + "'jupyter_server' package. These (future-looking) managers are not " + "guaranteed to work with the 'notebook' package. For longer term support " + "consider switching to NBClassic—a notebook frontend that leverages " + "Jupyter Server as its server backend." + ) + kernel_manager_class = Type( default_value=MappingKernelManager, klass=MappingKernelManager, diff --git a/notebook/services/sessions/sessionmanager.py b/notebook/services/sessions/sessionmanager.py index 63e1844829..92b2a73454 100644 --- a/notebook/services/sessions/sessionmanager.py +++ b/notebook/services/sessions/sessionmanager.py @@ -18,24 +18,34 @@ from traitlets import Instance from notebook.utils import maybe_future - +from notebook.traittypes import InstanceFromClasses class SessionManager(LoggingConfigurable): kernel_manager = Instance('notebook.services.kernels.kernelmanager.MappingKernelManager') - contents_manager = Instance('notebook.services.contents.manager.ContentsManager') - + contents_manager = InstanceFromClasses( + klasses=[ + 'notebook.services.contents.manager.ContentsManager', + # To make custom ContentsManagers both forward+backward + # compatible, we'll relax the strictness of this trait + # and allow jupyter_server contents managers to pass + # through. If jupyter_server is not installed, this class + # will be ignored. + 'jupyter_server.services.contents.manager.ContentsManager' + ] + ) + # Session database initialized below _cursor = None _connection = None _columns = {'session_id', 'path', 'name', 'type', 'kernel_id'} - + @property def cursor(self): """Start a cursor and create a database called 'session'""" if self._cursor is None: self._cursor = self.connection.cursor() - self._cursor.execute("""CREATE TABLE session + self._cursor.execute("""CREATE TABLE session (session_id, path, name, type, kernel_id)""") return self._cursor @@ -46,7 +56,7 @@ def connection(self): self._connection = sqlite3.connect(':memory:') self._connection.row_factory = sqlite3.Row return self._connection - + def close(self): """Close the sqlite connection""" if self._cursor is not None: @@ -106,11 +116,11 @@ def start_kernel_for_session(self, session_id, path, name, type, kernel_name): @gen.coroutine def save_session(self, session_id, path=None, name=None, type=None, kernel_id=None): """Saves the items for the session with the given session_id - + Given a session_id (and any other of the arguments), this method creates a row in the sqlite session database that holds the information for a session. - + Parameters ---------- session_id : str @@ -123,7 +133,7 @@ def save_session(self, session_id, path=None, name=None, type=None, kernel_id=No the type of the session kernel_id : str a uuid for the kernel associated with this session - + Returns ------- model : dict @@ -138,7 +148,7 @@ def save_session(self, session_id, path=None, name=None, type=None, kernel_id=No @gen.coroutine def get_session(self, **kwargs): """Returns the model for a particular session. - + Takes a keyword argument and searches for the value in the session database, then returns the rest of the session's info. @@ -151,7 +161,7 @@ def get_session(self, **kwargs): Returns ------- model : dict - returns a dictionary that includes all the information from the + returns a dictionary that includes all the information from the session described by the kwarg. """ if not kwargs: @@ -185,17 +195,17 @@ def get_session(self, **kwargs): @gen.coroutine def update_session(self, session_id, **kwargs): """Updates the values in the session database. - + Changes the values of the session with the given session_id - with the values from the keyword arguments. - + with the values from the keyword arguments. + Parameters ---------- session_id : str a uuid that identifies a session in the sqlite3 database **kwargs : str the key must correspond to a column title in session database, - and the value replaces the current value in the session + and the value replaces the current value in the session with session_id. """ yield maybe_future(self.get_session(session_id=session_id)) @@ -228,7 +238,7 @@ def row_to_model(self, row, tolerate_culled=False): # If caller wishes to tolerate culled kernels, log a warning # and return None. Otherwise, raise KeyError with a similar # message. - self.cursor.execute("DELETE FROM session WHERE session_id=?", + self.cursor.execute("DELETE FROM session WHERE session_id=?", (row['session_id'],)) msg = "Kernel '{kernel_id}' appears to have been culled or died unexpectedly, " \ "invalidating session '{session_id}'. The session has been removed.".\ diff --git a/notebook/tests/test_traittypes.py b/notebook/tests/test_traittypes.py new file mode 100644 index 0000000000..69c268223f --- /dev/null +++ b/notebook/tests/test_traittypes.py @@ -0,0 +1,80 @@ +import pytest +from traitlets import HasTraits, TraitError +from traitlets.utils.importstring import import_item + +from notebook.traittypes import ( + InstanceFromClasses, + TypeFromClasses +) +from notebook.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, + 'notebook.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, + 'notebook.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) diff --git a/notebook/traittypes.py b/notebook/traittypes.py new file mode 100644 index 0000000000..226657c1f4 --- /dev/null +++ b/notebook/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 notebook does; instead +# notebook 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)