Merge pull request #2108 from minrk/merge-server-extensions

merge nbserver_extensions

notebook/nbextensions.py

@@ -21,8 +21,8 @@
     from urllib import urlretrieve
 
 from jupyter_core.paths import (
-    jupyter_data_dir, jupyter_config_dir, jupyter_config_path,
-    SYSTEM_JUPYTER_PATH, ENV_JUPYTER_PATH, ENV_CONFIG_PATH, SYSTEM_CONFIG_PATH
+    jupyter_data_dir, jupyter_config_path, jupyter_path,
+    SYSTEM_JUPYTER_PATH, ENV_JUPYTER_PATH,
 )
 from ipython_genutils.path import ensure_dir_exists
 from ipython_genutils.py3compat import string_types, cast_unicode_py2
@@ -484,7 +484,7 @@ def validate_nbextension(require, logger=None):
     infos = []
 
     js_exists = False
-    for exts in _nbextension_dirs():
+    for exts in jupyter_path('nbextensions'):
         # Does the Javascript entrypoint actually exist on disk?
         js = u"{}.js".format(os.path.join(exts, *require.split("/")))
         js_exists = os.path.exists(js)
@@ -1014,18 +1014,6 @@ def _get_nbextension_dir(user=False, sys_prefix=False, prefix=None, nbextensions
     return nbext
 
 
-def _nbextension_dirs():
-    """The possible locations of nbextensions.
-
-    Returns a list of known base extension locations
-    """
-    return [
-        pjoin(jupyter_data_dir(), u'nbextensions'),
-        pjoin(ENV_JUPYTER_PATH[0], u'nbextensions'),
-        pjoin(SYSTEM_JUPYTER_PATH[0], 'nbextensions')
-    ]
-
-
 def _get_nbextension_metadata(module):
     """Get the list of nbextension paths associated with a Python module.
 

notebook/notebookapp.py

@@ -86,6 +86,7 @@
 from jupyter_core.application import (
     JupyterApp, base_flags, base_aliases,
 )
+from jupyter_core.paths import jupyter_config_path
 from jupyter_client import KernelManager
 from jupyter_client.kernelspec import KernelSpecManager, NoSuchKernel, NATIVE_KERNEL_NAME
 from jupyter_client.session import Session
@@ -1232,9 +1233,28 @@ def init_server_extensions(self):
             # in the new traitlet
             if not modulename in self.nbserver_extensions:
                 self.nbserver_extensions[modulename] = True
-
-        for modulename in sorted(self.nbserver_extensions):
-            if self.nbserver_extensions[modulename]:
+
+        # Load server extensions with ConfigManager.
+        # This enables merging on keys, which we want for extension enabling.
+        # Regular config loading only merges at the class level,
+        # so each level (user > env > system) clobbers the previous.
+        config_path = jupyter_config_path()
+        if self.config_dir not in config_path:
+            # add self.config_dir to the front, if set manually
+            config_path.insert(0, self.config_dir)
+        manager = ConfigManager(read_config_path=config_path)
+        section = manager.get(self.config_file_name)
+        extensions = section.get('NotebookApp', {}).get('nbserver_extensions', {})
+
+        for modulename, enabled in self.nbserver_extensions.items():
+            if modulename not in extensions:
+                # not present in `extensions` means it comes from Python config,
+                # so we need to add it.
+                # Otherwise, trust ConfigManager to have loaded it.
+                extensions[modulename] = enabled
+
+        for modulename, enabled in sorted(extensions.items()):
+            if enabled:
                 try:
                     mod = importlib.import_module(modulename)
                     func = getattr(mod, 'load_jupyter_server_extension', None)

notebook/tests/test_serverextensions.py

@@ -1,3 +1,4 @@
+import imp
 import os
 import sys
 from unittest import TestCase
@@ -11,9 +12,10 @@
 
 from traitlets.config.manager import BaseJSONConfigManager
 from traitlets.tests.utils import check_help_all_output
+from jupyter_core import paths
 
 from notebook.serverextensions import toggle_serverextension_python
-from notebook import nbextensions
+from notebook import nbextensions, serverextensions, extensions
 from notebook.notebookapp import NotebookApp
 from notebook.nbextensions import _get_config_dir
 
@@ -33,8 +35,24 @@ def test_help_output():
     check_help_all_output('notebook.serverextensions', ['install'])
     check_help_all_output('notebook.serverextensions', ['uninstall'])
 
+outer_file = __file__
 
-class TestInstallServerExtension(TestCase):
+class MockExtensionModule(object):
+    __file__ = outer_file
+
+    @staticmethod
+    def _jupyter_server_extension_paths():
+        return [{
+            'module': '_mockdestination/index'
+        }]
+
+    loaded = False
+
+    def load_jupyter_server_extension(self, app):
+        self.loaded = True
+
+
+class MockEnvTestCase(TestCase):
 
     def tempdir(self):
         td = TemporaryDirectory()
@@ -43,40 +61,55 @@ def tempdir(self):
 
     def setUp(self):
         self.tempdirs = []
+        self._mock_extensions = []
 
         self.test_dir = self.tempdir()
         self.data_dir = os.path.join(self.test_dir, 'data')
         self.config_dir = os.path.join(self.test_dir, 'config')
         self.system_data_dir = os.path.join(self.test_dir, 'system_data')
+        self.system_config_dir = os.path.join(self.test_dir, 'system_config')
         self.system_path = [self.system_data_dir]
+        self.system_config_path = [self.system_config_dir]
 
-        self.patch_env = patch.dict('os.environ', {
+        self.patches = []
+        p = patch.dict('os.environ', {
             'JUPYTER_CONFIG_DIR': self.config_dir,
             'JUPYTER_DATA_DIR': self.data_dir,
         })
-        self.patch_env.start()
-        self.patch_system_path = patch.object(nbextensions,
-            'SYSTEM_JUPYTER_PATH', self.system_path)
-        self.patch_system_path.start()
+        self.patches.append(p)
+        for mod in (paths, nbextensions):
+            p = patch.object(mod,
+                'SYSTEM_JUPYTER_PATH', self.system_path)
+            self.patches.append(p)
+            p = patch.object(mod,
+                'ENV_JUPYTER_PATH', [])
+            self.patches.append(p)
+        for mod in (paths, extensions):
+            p = patch.object(mod,
+                'SYSTEM_CONFIG_PATH', self.system_config_path)
+            self.patches.append(p)
+            p = patch.object(mod,
+                'ENV_CONFIG_PATH', [])
+            self.patches.append(p)
+        for p in self.patches:
+            p.start()
+            self.addCleanup(p.stop)
+        # verify our patches
+        self.assertEqual(paths.jupyter_config_path(), [self.config_dir] + self.system_config_path)
+        self.assertEqual(extensions._get_config_dir(user=False), self.system_config_dir)
+        self.assertEqual(paths.jupyter_path(), [self.data_dir] + self.system_path)
 
     def tearDown(self):
-        self.patch_env.stop()
-        self.patch_system_path.stop()
+        for modulename in self._mock_extensions:
+            sys.modules.pop(modulename)
 
-    def _inject_mock_extension(self):
-        outer_file = __file__
+    def _inject_mock_extension(self, modulename='mockextension'):
 
-        class mock():
-            __file__ = outer_file
+        sys.modules[modulename] = ext = MockExtensionModule()
+        self._mock_extensions.append(modulename)
+        return ext
 
-            @staticmethod
-            def _jupyter_server_extension_paths():
-                return [{
-                    'module': '_mockdestination/index'
-                }]
-
-        import sys
-        sys.modules['mockextension'] = mock
+class TestInstallServerExtension(MockEnvTestCase):
 
     def _get_config(self, user=True):
         cm = BaseJSONConfigManager(config_dir=_get_config_dir(user))
@@ -98,13 +131,37 @@ def test_disable(self):
         config = self._get_config()
         assert not config['mockextension']
 
+    def test_merge_config(self):
+        # enabled at sys level
+        mock_sys = self._inject_mock_extension('mockext_sys')
+        # enabled at sys, disabled at user
+        mock_both = self._inject_mock_extension('mockext_both')
+        # enabled at user
+        mock_user = self._inject_mock_extension('mockext_user')
+        # enabled at Python
+        mock_py = self._inject_mock_extension('mockext_py')
+
+        toggle_serverextension_python('mockext_sys', enabled=True, user=False)
+        toggle_serverextension_python('mockext_user', enabled=True, user=True)
+        toggle_serverextension_python('mockext_both', enabled=True, user=False)
+        toggle_serverextension_python('mockext_both', enabled=False, user=True)
+
+        app = NotebookApp(nbserver_extensions={'mockext_py': True})
+        app.init_server_extensions()
+
+        assert mock_user.loaded
+        assert mock_sys.loaded
+        assert mock_py.loaded
+        assert not mock_both.loaded
+
 
-class TestOrderedServerExtension(TestCase):
+class TestOrderedServerExtension(MockEnvTestCase):
     """
     Test that Server Extensions are loaded _in order_
     """
 
     def setUp(self):
+        super(TestOrderedServerExtension, self).setUp()
         mockextension1 = SimpleNamespace()
         mockextension2 = SimpleNamespace()
 
@@ -124,6 +181,7 @@ def load_jupyter_server_extension(obj):
         sys.modules['mockextension1'] = mockextension1
 
     def tearDown(self):
+        super(TestOrderedServerExtension, self).tearDown()
         del sys.modules['mockextension2']
         del sys.modules['mockextension1']