Apply review comments, revert interval validation, bump terminado to 0.8.3

Author: kevin-bates

notebook/notebookapp.py

@@ -1743,7 +1743,7 @@ def init_terminals(self):
 
         try:
             from .terminal import initialize
-            initialize(self.web_app, self.notebook_dir, self.connection_url, self.terminado_settings, self)
+            initialize(parent=self)
             self.web_app.settings['terminals_available'] = True
         except ImportError as e:
             self.log.warning(_("Terminals not available (error was %s)"), e)

notebook/terminal/__init__.py

@@ -14,31 +14,31 @@
 from . import api_handlers
 
 
-def initialize(webapp, notebook_dir, connection_url, settings, parent):
+def initialize(parent):
     if os.name == 'nt':
         default_shell = 'powershell.exe'
     else:
         default_shell = which('sh')
-    shell = settings.get('shell_command',
+    shell = parent.terminado_settings.get('shell_command',
         [os.environ.get('SHELL') or default_shell]
     )
     # Enable login mode - to automatically source the /etc/profile script
     if os.name != 'nt':
         shell.append('-l')
-    terminal_manager = webapp.settings['terminal_manager'] = TerminalManager(
+    terminal_manager = parent.web_app.settings['terminal_manager'] = TerminalManager(
         shell_command=shell,
-        extra_env={'JUPYTER_SERVER_ROOT': notebook_dir,
-                   'JUPYTER_SERVER_URL': connection_url,
+        extra_env={'JUPYTER_SERVER_ROOT': parent.notebook_dir,
+                   'JUPYTER_SERVER_URL': parent.connection_url,
                    },
         parent=parent,
     )
-    terminal_manager.log = app_log
-    base_url = webapp.settings['base_url']
+    terminal_manager.log = parent.log
+    base_url = parent.web_app.settings['base_url']
     handlers = [
         (ujoin(base_url, r"/terminals/(\w+)"), TerminalHandler),
         (ujoin(base_url, r"/terminals/websocket/(\w+)"), TermSocket,
              {'term_manager': terminal_manager}),
         (ujoin(base_url, r"/api/terminals"), api_handlers.TerminalRootHandler),
         (ujoin(base_url, r"/api/terminals/(\w+)"), api_handlers.TerminalHandler),
     ]
-    webapp.add_handlers(".*$", handlers)
+    parent.web_app.add_handlers(".*$", handlers)

notebook/terminal/terminalmanager.py

@@ -14,11 +14,11 @@
 from tornado import web
 from tornado.ioloop import IOLoop, PeriodicCallback
 from traitlets import Integer, validate
-from traitlets.config import Configurable
+from traitlets.config import LoggingConfigurable
 from ..prometheus.metrics import TERMINAL_CURRENTLY_RUNNING_TOTAL
 
 
-class TerminalManager(Configurable, NamedTermManager):
+class TerminalManager(LoggingConfigurable, NamedTermManager):
     """  """
 
     _culler_callback = None
@@ -35,15 +35,6 @@ class TerminalManager(Configurable, NamedTermManager):
         help="""The interval (in seconds) on which to check for terminals exceeding the inactive timeout value."""
                             )
 
-    @validate('cull_interval')
-    def _cull_interval_validate(self, proposal):
-        value = proposal['value']
-        if value <= 0:
-            warnings.warn("Invalid value for 'cull_interval' detected ({}) - using default value ({}).".
-                          format(value, self.cull_interval_default))
-            value = self.cull_interval_default
-        return value
-
     # -------------------------------------------------------------------------
     # Methods for managing terminals
     # -------------------------------------------------------------------------
@@ -118,6 +109,10 @@ def _initialize_culler(self):
         if not self._initialized_culler and self.cull_inactive_timeout > 0:
             if self._culler_callback is None:
                 loop = IOLoop.current()
+                if self.cull_interval <= 0:  # handle case where user set invalid value
+                    self.log.warning("Invalid value for 'cull_interval' detected (%s) - using default value (%s).",
+                                     self.cull_interval, self.cull_interval_default)
+                    self.cull_interval = self.cull_interval_default
                 self._culler_callback = PeriodicCallback(
                     self._cull_terminals, 1000 * self.cull_interval)
                 self.log.info("Culling terminals with inactivity > %s seconds at %s second intervals ...",

setup.py

@@ -110,7 +110,7 @@
         'nbconvert',
         'ipykernel', # bless IPython kernel for now
         'Send2Trash',
-        'terminado>=0.8.1',
+        'terminado>=0.8.3',
         'prometheus_client'
     ],
     extras_require = {