Apply review comments, revert interval validation

Author: kevin-bates

jupyter_server/serverapp.py

@@ -1547,7 +1547,7 @@ def init_terminals(self):
 
         try:
             from .terminal import initialize
-            initialize(self.web_app, self.root_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(_i18n("Terminals not available (error was %s)"), e)

jupyter_server/terminal/__init__.py

@@ -15,12 +15,12 @@
 from .terminalmanager import TerminalManager
 
 
-def initialize(webapp, root_dir, connection_url, settings, parent):
+def initialize(parent):
     if os.name == 'nt':
         default_shell = 'powershell.exe'
     else:
         default_shell = which('sh')
-    shell_override = settings.get('shell_command')
+    shell_override = parent.terminado_settings.get('shell_command')
     shell = (
         [os.environ.get('SHELL') or default_shell]
         if shell_override is None
@@ -33,19 +33,19 @@ def initialize(webapp, root_dir, connection_url, settings, parent):
     # the user has specifically set a preferred shell command.
     if os.name != 'nt' and shell_override is None and not sys.stdout.isatty():
         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': root_dir,
-                   'JUPYTER_SERVER_URL': connection_url,
+        extra_env={'JUPYTER_SERVER_ROOT': parent.root_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/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)

jupyter_server/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 ...",