Merge pull request #177 from nitind/notebook-http-as-plug-in-traitlets

[Issue: 103] Refactor mode-specific traitlets

docs/source/config-options.md

@@ -33,10 +33,6 @@ KernelGatewayApp options
 --KernelGatewayApp.allow_methods=<Unicode>
     Default: ''
     Sets the Access-Control-Allow-Methods header. (KG_ALLOW_METHODS env var)
---KernelGatewayApp.allow_notebook_download=<Bool>
-    Default: False
-    Optional API to download the notebook source code in notebook-http mode,
-    defaults to not allow (KG_ALLOW_NOTEBOOK_DOWNLOAD env var)
 --KernelGatewayApp.allow_origin=<Unicode>
     Default: ''
     Sets the Access-Control-Allow-Origin header. (KG_ALLOW_ORIGIN env var)
@@ -74,11 +70,6 @@ KernelGatewayApp options
 --KernelGatewayApp.ip=<Unicode>
     Default: ''
     IP address on which to listen (KG_IP env var)
---KernelGatewayApp.list_kernels=<Bool>
-    Default: False
-    Permits listing of the running kernels using API endpoints /api/kernels and
-    /api/sessions (KG_LIST_KERNELS env var). Note: Jupyter Notebook allows this
-    by default but kernel gateway does not.
 --KernelGatewayApp.log_datefmt=<Unicode>
     Default: '%Y-%m-%d %H:%M:%S'
     The date format used by logging formatters for %(asctime)s
@@ -111,4 +102,22 @@ KernelGatewayApp options
     Default: ''
     Runs the notebook (.ipynb) at the given URI on every kernel launched.
     (KG_SEED_URI env var)
+
+
+
+JupyterWebsocketPersonality options
+-----------------------------------
+--JupyterWebsocketPersonality.list_kernels=<Bool>
+    Default: False
+    Permits listing of the running kernels using API endpoints /api/kernels and
+    /api/sessions (KG_LIST_KERNELS env var). Note: Jupyter Notebook allows this
+    by default but kernel gateway does not.
+
+
+NotebookHTTPPersonality options
+-------------------------------
+--NotebookHTTPPersonality.allow_notebook_download=<Bool>
+    Default: False
+    Optional API to download the notebook source code in notebook-http mode,
+    defaults to not allow (KG_ALLOW_NOTEBOOK_DOWNLOAD env var)
 ```

kernel_gateway/gatewayapp.py

@@ -34,6 +34,10 @@
 from .services.activity.manager import ActivityManager
 from .services.kernels.manager import SeedingMappingKernelManager
 
+# Only present for generating help documentation
+from kernel_gateway.notebook_http import NotebookHTTPPersonality
+from kernel_gateway.jupyter_websocket import JupyterWebsocketPersonality
+
 class KernelGatewayApp(JupyterApp):
     """Application that provisions Jupyter kernels and proxies HTTP/Websocket
     traffic to the kernels.
@@ -52,6 +56,9 @@ class KernelGatewayApp(JupyterApp):
         to them.
     """
 
+    # Also include when generating help options
+    classes = [NotebookHTTPPersonality, JupyterWebsocketPersonality]
+
     # Server IP / PORT binding
     port_env = 'KG_PORT'
     port = Integer(config=True,
@@ -181,16 +188,6 @@ def default_kernel_name_default(self):
         # defaults to Jupyter's default kernel name on empty string
         return os.getenv(self.default_kernel_name_env, '')
 
-    list_kernels_env = 'KG_LIST_KERNELS'
-    list_kernels = Bool(config=True,
-        help="""Permits listing of the running kernels using API endpoints /api/kernels
-            and /api/sessions (KG_LIST_KERNELS env var). Note: Jupyter Notebook
-            allows this by default but kernel gateway does not."""
-    )
-    @default('list_kernels')
-    def list_kernels_default(self):
-        return os.getenv(self.list_kernels_env, 'False') == 'True'
-
     api_env = 'KG_API'
     api = Unicode('kernel_gateway.jupyter_websocket',
         config=True,
@@ -209,15 +206,6 @@ def _api_changed(self, name, old, new):
         if new_module is None:
             raise ValueError('Invalid API value, module {} was not found'.format(new))
 
-    allow_notebook_download_env = 'KG_ALLOW_NOTEBOOK_DOWNLOAD'
-    allow_notebook_download = Bool(
-        config=True,
-        help="Optional API to download the notebook source code in notebook-http mode, defaults to not allow"
-    )
-    @default('allow_notebook_download')
-    def allow_notebook_download_default(self):
-        return os.getenv(self.allow_notebook_download_env, 'False') == 'True'
-
     def _load_api_module(self, module_name):
         '''Tries to import the given module name'''
         api_module = None
@@ -358,13 +346,21 @@ def init_webapp(self):
             kg_expose_headers=self.expose_headers,
             kg_max_age=self.max_age,
             kg_max_kernels=self.max_kernels,
-            kg_list_kernels=self.list_kernels,
             kg_api=self.api,
             # Also set the allow_origin setting used by notebook so that the
             # check_origin method used everywhere respects the value
             allow_origin=self.allow_origin
         )
 
+        # promote the current personality's "config" tagged traitlet values to webapp settings
+        for trait_name, trait_value in self.personality.class_traits(config=True).items():
+            kg_name = 'kg_' + trait_name
+            # a personality's traitlets may not overwrite the kernel gateway's
+            if kg_name not in self.web_app.settings:
+                self.web_app.settings[kg_name] = trait_value.get(obj=self.personality)
+            else:
+                self.log.warning('The personality trait name, %s, conflicts with a kernel gateway trait.' , trait_name)
+
     def init_http_server(self):
         """Initializes a HTTP server for the Tornado web application on the
         configured interface and port.

kernel_gateway/jupyter_websocket/__init__.py

@@ -2,6 +2,7 @@
 # Distributed under the terms of the Modified BSD License.
 """Jupyter websocket personality for the Kernel Gateway"""
 
+import os
 from ..base.handlers import default_handlers as default_base_handlers
 from ..services.activity.handlers import ActivityHandler
 from ..services.kernels.pool import KernelPool
@@ -10,13 +11,24 @@
 from ..services.sessions.handlers import default_handlers as default_session_handlers
 from .handlers import default_handlers as default_api_handlers
 from notebook.utils import url_path_join
+from traitlets import Bool, default
 from traitlets.config.configurable import LoggingConfigurable
 
 class JupyterWebsocketPersonality(LoggingConfigurable):
     """Personality for standard websocket functionality, registering
     endpoints that are part of the Jupyter Kernel Gateway API
     """
 
+    list_kernels_env = 'KG_LIST_KERNELS'
+    list_kernels = Bool(config=True,
+        help="""Permits listing of the running kernels using API endpoints /api/kernels
+            and /api/sessions (KG_LIST_KERNELS env var). Note: Jupyter Notebook
+            allows this by default but kernel gateway does not."""
+    )
+    @default('list_kernels')
+    def list_kernels_default(self):
+        return os.getenv(self.list_kernels_env, 'False') == 'True'
+
     def init_configurables(self):
         self.kernel_pool = KernelPool(
             self.parent.prespawn_count,

kernel_gateway/notebook_http/__init__.py

@@ -2,12 +2,14 @@
 # Distributed under the terms of the Modified BSD License.
 """Notebook HTTP personality for the Kernel Gateway"""
 
+import os
 from ..base.handlers import default_handlers as default_base_handlers
 from ..services.kernels.pool import ManagedKernelPool
 from .cell.parser import APICellParser
 from .swagger.handlers import SwaggerSpecHandler
 from .handlers import NotebookAPIHandler, parameterize_path, NotebookDownloadHandler
 from notebook.utils import url_path_join
+from traitlets import Bool, default
 from traitlets.config.configurable import LoggingConfigurable
 
 class NotebookHTTPPersonality(LoggingConfigurable):
@@ -18,6 +20,15 @@ def __init__(self, *args, **kwargs):
         super(NotebookHTTPPersonality, self).__init__(*args, **kwargs)
         self.api_parser = APICellParser(self.parent.kernel_manager.seed_kernelspec)
 
+    allow_notebook_download_env = 'KG_ALLOW_NOTEBOOK_DOWNLOAD'
+    allow_notebook_download = Bool(
+        config=True,
+        help="Optional API to download the notebook source code in notebook-http mode, defaults to not allow"
+    )
+    @default('allow_notebook_download')
+    def allow_notebook_download_default(self):
+        return os.getenv(self.allow_notebook_download_env, 'False') == 'True'
+
     def init_configurables(self):
         self.kernel_pool = ManagedKernelPool(
             self.parent.prespawn_count,
@@ -31,7 +42,7 @@ def create_request_handlers(self):
         """
         handlers = []
         # Register the NotebookDownloadHandler if configuration allows
-        if self.parent.allow_notebook_download:
+        if self.allow_notebook_download:
             handlers.append((
                 url_path_join('/', self.parent.base_url, r'/_api/source'),
                 NotebookDownloadHandler,

kernel_gateway/tests/test_gatewayapp.py

@@ -39,8 +39,6 @@ def test_config_env_vars(self):
         os.environ['KG_SEED_URI'] = 'fake-notebook.ipynb'
         os.environ['KG_PRESPAWN_COUNT'] = '1'
         os.environ['KG_DEFAULT_KERNEL_NAME'] = 'fake_kernel'
-        os.environ['KG_LIST_KERNELS'] = 'True'
-        os.environ['KG_ALLOW_NOTEBOOK_DOWNLOAD'] = 'True'
 
         app = KernelGatewayApp()
 
@@ -59,8 +57,6 @@ def test_config_env_vars(self):
         self.assertEqual(app.seed_uri, 'fake-notebook.ipynb')
         self.assertEqual(app.prespawn_count, 1)
         self.assertEqual(app.default_kernel_name, 'fake_kernel')
-        self.assertEqual(app.list_kernels, True)
-        self.assertEqual(app.allow_notebook_download, True)
 
 class TestGatewayAppBase(AsyncHTTPTestCase, LogTrapTestCase):
     """Base class for integration style tests using HTTP/Websockets against an
@@ -90,6 +86,7 @@ def get_app(self):
         self.app = KernelGatewayApp(log_level=logging.CRITICAL)
         self.setup_app()
         self.app.init_configurables()
+        self.setup_configurables()
         self.app.init_webapp()
         return self.app.web_app
 
@@ -98,3 +95,8 @@ def setup_app(self):
         configurables and the web app.
         """
         pass
+
+    def setup_configurables(self):
+        """Override to configure further settings, such as the personality.
+        """
+        pass

kernel_gateway/tests/test_jupyter_websocket.py

@@ -560,9 +560,9 @@ def test_default_kernel_name(self):
 
 class TestEnableDiscovery(TestJupyterWebsocket):
     """Tests gateway behavior with kernel listing enabled."""
-    def setup_app(self):
+    def setup_configurables(self):
         """Enables kernel listing for all tests."""
-        self.app.list_kernels = True
+        self.app.personality.list_kernels = True
 
     @gen_test
     def test_enable_kernel_list(self):
@@ -614,7 +614,10 @@ class TestBaseURL(TestJupyterWebsocket):
     def setup_app(self):
         """Sets the custom base URL and enables kernel listing."""
         self.app.base_url = '/fake/path'
-        self.app.list_kernels = True
+
+    def setup_configurables(self):
+        """Enables kernel listing for all tests."""
+        self.app.personality.list_kernels = True
 
     @gen_test
     def test_base_url(self):
@@ -777,9 +780,9 @@ def test_seed_language_support(self):
 
 class TestActivityAPI(TestJupyterWebsocket):
     """Tests gateway behavior when the activity API is enabled."""
-    def setup_app(self):
+    def setup_configurables(self):
         """Enables kernel listing so the activity API is available."""
-        self.app.list_kernels = True
+        self.app.personality.list_kernels = True
 
     @gen_test
     def test_api_lists_kernels_with_flag_set(self):

kernel_gateway/tests/test_notebook_http.py

@@ -242,7 +242,9 @@ def setup_app(self):
         self.app.api = 'kernel_gateway.notebook_http'
         self.app.seed_uri = os.path.join(RESOURCES,
                                          'kernel_api{}.ipynb'.format(sys.version_info.major))
-        self.app.allow_notebook_download = True
+
+    def setup_configurables(self):
+        self.app.personality.allow_notebook_download = True
 
     @gen_test
     def test_download_notebook_source(self):
@@ -414,10 +416,12 @@ def setup_app(self):
         """Sets the custom base URL and enables the notebook-defined API."""
         self.app.base_url = '/fake/path'
         self.app.api = 'kernel_gateway.notebook_http'
-        self.app.allow_notebook_download = True
         self.app.seed_uri = os.path.join(RESOURCES,
                                          'kernel_api{}.ipynb'.format(sys.version_info.major))
 
+    def setup_configurables(self):
+        self.app.personality.allow_notebook_download = True
+
     @gen_test
     def test_base_url(self):
         """Server should mount resources under the configured base."""