Merge pull request #5957 from afshin/contents-manager

kevin-bates · web-flow · commit 8cd9a5f78af8 · 2021-02-02T14:54:37.000-08:00
Allow jupyter_server-based contents managers in notebook
diff --git a/notebook/notebookapp.py b/notebook/notebookapp.py
@@ -117,6 +117,7 @@
     urlencode_unix_socket_path,
     urljoin,
 )
+from .traittypes import TypeFromClasses
 
 # Check if we can use async kernel management
 try:
@@ -1375,13 +1376,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
@@ -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
@@ -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
