From 7b3d91af1770a698fd983d71d557d88e61a61317 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Mon, 13 Apr 2020 11:29:25 -0700 Subject: [PATCH 01/10] Add ability to cull terminals and track last activity This adds functionality to track a terminal's last activity and optionally cull terminals that have been inactive for some specified duration. --- jupyter_server/serverapp.py | 5 +- jupyter_server/services/api/api.yaml | 18 ++- jupyter_server/terminal/__init__.py | 9 +- jupyter_server/terminal/api_handlers.py | 40 ++---- jupyter_server/terminal/handlers.py | 18 ++- jupyter_server/terminal/terminalmanager.py | 140 +++++++++++++++++++++ 6 files changed, 185 insertions(+), 45 deletions(-) create mode 100644 jupyter_server/terminal/terminalmanager.py diff --git a/jupyter_server/serverapp.py b/jupyter_server/serverapp.py index 29e3be5d9..fecfe641c 100755 --- a/jupyter_server/serverapp.py +++ b/jupyter_server/serverapp.py @@ -83,6 +83,7 @@ from .auth.login import LoginHandler from .auth.logout import LogoutHandler from .base.handlers import FileFindHandler +from .terminal import TerminalManager from traitlets.config import Config from traitlets.config.application import catch_config_error, boolean_flag @@ -587,7 +588,7 @@ class ServerApp(JupyterApp): classes = [ KernelManager, Session, MappingKernelManager, KernelSpecManager, AsyncMappingKernelManager, ContentsManager, FileContentsManager, AsyncContentsManager, AsyncFileContentsManager, NotebookNotary, - GatewayKernelManager, GatewayKernelSpecManager, GatewaySessionManager, GatewayClient + TerminalManager, GatewayKernelSpecManager, GatewaySessionManager, GatewayClient ] subcommands = dict( @@ -1546,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) + initialize(self.web_app, self.root_dir, self.connection_url, self.terminado_settings, self) self.web_app.settings['terminals_available'] = True except ImportError as e: self.log.warning(_i18n("Terminals not available (error was %s)"), e) diff --git a/jupyter_server/services/api/api.yaml b/jupyter_server/services/api/api.yaml index 956367bdf..6ca22a0a6 100644 --- a/jupyter_server/services/api/api.yaml +++ b/jupyter_server/services/api/api.yaml @@ -568,7 +568,7 @@ paths: schema: type: array items: - $ref: '#/definitions/Terminal_ID' + $ref: '#/definitions/Terminal' 403: description: Forbidden to access 404: @@ -582,7 +582,7 @@ paths: 200: description: Succesfully created a new terminal schema: - $ref: '#/definitions/Terminal_ID' + $ref: '#/definitions/Terminal' 403: description: Forbidden to access 404: @@ -599,7 +599,7 @@ paths: 200: description: Terminal session with given id schema: - $ref: '#/definitions/Terminal_ID' + $ref: '#/definitions/Terminal' 403: description: Forbidden to access 404: @@ -845,12 +845,18 @@ definitions: type: string description: Last modified timestamp format: dateTime - Terminal_ID: - description: A Terminal_ID object + Terminal: + description: A Terminal object type: object required: - name properties: name: type: string - description: name of terminal ID + description: name of terminal + last_activity: + type: string + description: | + ISO 8601 timestamp for the last-seen activity on this terminal. Use + this to identify which terminals have been inactive since a given time. + Timestamps will be UTC, indicated 'Z' suffix. diff --git a/jupyter_server/terminal/__init__.py b/jupyter_server/terminal/__init__.py index ea51ab135..e4ad3ee4b 100644 --- a/jupyter_server/terminal/__init__.py +++ b/jupyter_server/terminal/__init__.py @@ -8,14 +8,14 @@ raise ImportError("terminado >= 0.8.3 required, found %s" % terminado.__version__) from ipython_genutils.py3compat import which -from terminado import NamedTermManager from tornado.log import app_log from jupyter_server.utils import url_path_join as ujoin from . import api_handlers -from .handlers import TermSocket +from .handlers import TerminalHandler, TermSocket +from .terminalmanager import TerminalManager -def initialize(webapp, root_dir, connection_url, settings): +def initialize(webapp, root_dir, connection_url, settings, parent): if os.name == 'nt': default_shell = 'powershell.exe' else: @@ -33,11 +33,12 @@ def initialize(webapp, root_dir, connection_url, settings): # 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'] = NamedTermManager( + terminal_manager = webapp.settings['terminal_manager'] = TerminalManager( shell_command=shell, extra_env={'JUPYTER_SERVER_ROOT': root_dir, 'JUPYTER_SERVER_URL': connection_url, }, + parent=parent, ) terminal_manager.log = app_log base_url = webapp.settings['base_url'] diff --git a/jupyter_server/terminal/api_handlers.py b/jupyter_server/terminal/api_handlers.py index 396769209..5a2b2f854 100644 --- a/jupyter_server/terminal/api_handlers.py +++ b/jupyter_server/terminal/api_handlers.py @@ -1,7 +1,6 @@ import json from tornado import web from ..base.handlers import APIHandler -from ..prometheus.metrics import TERMINAL_CURRENTLY_RUNNING_TOTAL @@ -9,25 +8,16 @@ class TerminalRootHandler(APIHandler): @web.authenticated def get(self): - tm = self.terminal_manager - terms = [{'name': name} for name in tm.terminals] - self.finish(json.dumps(terms)) - - # Update the metric below to the length of the list 'terms' - TERMINAL_CURRENTLY_RUNNING_TOTAL.set( - len(terms) - ) + models = self.terminal_manager.list() + self.finish(json.dumps(models)) @web.authenticated def post(self): """POST /terminals creates a new terminal and redirects to it""" data = self.get_json_body() or {} - name, _ = self.terminal_manager.new_named_terminal(**data) - self.finish(json.dumps({'name': name})) - - # Increase the metric by one because a new terminal was created - TERMINAL_CURRENTLY_RUNNING_TOTAL.inc() + model = self.terminal_manager.create(**data) + self.finish(json.dumps(model)) class TerminalHandler(APIHandler): @@ -35,23 +25,11 @@ class TerminalHandler(APIHandler): @web.authenticated def get(self, name): - tm = self.terminal_manager - if name in tm.terminals: - self.finish(json.dumps({'name': name})) - else: - raise web.HTTPError(404, "Terminal not found: %r" % name) + model = self.terminal_manager.get(name) + self.finish(json.dumps(model)) @web.authenticated async def delete(self, name): - tm = self.terminal_manager - if name in tm.terminals: - await tm.terminate(name, force=True) - self.set_status(204) - self.finish() - - # Decrease the metric below by one - # because a terminal has been shutdown - TERMINAL_CURRENTLY_RUNNING_TOTAL.dec() - - else: - raise web.HTTPError(404, "Terminal not found: %r" % name) + await self.terminal_manager.terminate(name, force=True) + self.set_status(204) + self.finish() diff --git a/jupyter_server/terminal/handlers.py b/jupyter_server/terminal/handlers.py index 4b5f2e65a..d88769b60 100644 --- a/jupyter_server/terminal/handlers.py +++ b/jupyter_server/terminal/handlers.py @@ -11,6 +11,14 @@ from ..base.zmqhandlers import WebSocketMixin +class TerminalHandler(JupyterHandler): + """Render the terminal interface.""" + @web.authenticated + def get(self, term_name): + self.write(self.render_template('terminal.html', + ws_path="terminals/websocket/%s" % term_name)) + + class TermSocket(WebSocketMixin, JupyterHandler, terminado.TermSocket): def origin_check(self): @@ -26,8 +34,14 @@ def get(self, *args, **kwargs): def on_message(self, message): super(TermSocket, self).on_message(message) - self.application.settings['terminal_last_activity'] = utcnow() + self._update_activity() def write_message(self, message, binary=False): super(TermSocket, self).write_message(message, binary=binary) - self.application.settings['terminal_last_activity'] = utcnow() \ No newline at end of file + self._update_activity() + + def _update_activity(self): + self.application.settings['terminal_last_activity'] = utcnow() + # terminal may not be around on deletion/cull + if self.term_name in self.terminal_manager.terminals: + self.terminal_manager.terminals[self.term_name].last_activity = utcnow() diff --git a/jupyter_server/terminal/terminalmanager.py b/jupyter_server/terminal/terminalmanager.py new file mode 100644 index 000000000..b003e0966 --- /dev/null +++ b/jupyter_server/terminal/terminalmanager.py @@ -0,0 +1,140 @@ +"""A MultiTerminalManager for use in the notebook webserver +- raises HTTPErrors +- creates REST API models +""" + +# Copyright (c) Jupyter Development Team. +# Distributed under the terms of the Modified BSD License. + + +from datetime import timedelta +from notebook._tz import utcnow, isoformat +from terminado import NamedTermManager +from tornado import web +from tornado.ioloop import IOLoop, PeriodicCallback +from traitlets import Integer +from traitlets.config import Configurable +from ..prometheus.metrics import TERMINAL_CURRENTLY_RUNNING_TOTAL + + +class TerminalManager(Configurable, NamedTermManager): + """ """ + + _culler_callback = None + + _initialized_culler = False + + cull_inactive_timeout = Integer(0, config=True, + help="""Timeout (in seconds) in which a terminal has been inactive and ready to be culled. + Values of 0 or lower disable culling.""" + ) + + cull_interval_default = 300 # 5 minutes + cull_interval = Integer(cull_interval_default, config=True, + help="""The interval (in seconds) on which to check for terminals exceeding the inactive timeout value.""" + ) + + # ------------------------------------------------------------------------- + # Methods for managing terminals + # ------------------------------------------------------------------------- + def __init__(self, *args, **kwargs): + super(TerminalManager, self).__init__(*args, **kwargs) + + def create(self): + name, term = self.new_named_terminal() + # Monkey-patch last-activity, similar to kernels. Should we need + # more functionality per terminal, we can look into possible sub- + # classing or containment then. + term.last_activity = utcnow() + model = self.terminal_model(name) + # Increase the metric by one because a new terminal was created + TERMINAL_CURRENTLY_RUNNING_TOTAL.inc() + # Ensure culler is initialized + self.initialize_culler() + return model + + def get(self, name): + model = self.terminal_model(name) + return model + + def list(self): + models = [self.terminal_model(name) for name in self.terminals] + + # Update the metric below to the length of the list 'terms' + TERMINAL_CURRENTLY_RUNNING_TOTAL.set( + len(models) + ) + return models + + async def terminate(self, name, force=False): + self._check_terminal(name) + await super(TerminalManager, self).terminate(name, force=force) + + # Decrease the metric below by one + # because a terminal has been shutdown + TERMINAL_CURRENTLY_RUNNING_TOTAL.dec() + + def terminal_model(self, name): + """Return a JSON-safe dict representing a terminal. + For use in representing terminals in the JSON APIs. + """ + self._check_terminal(name) + term = self.terminals[name] + model = { + "name": name, + "last_activity": isoformat(term.last_activity), + } + return model + + def _check_terminal(self, name): + """Check a that terminal 'name' exists and raise 404 if not.""" + if name not in self.terminals: + raise web.HTTPError(404, u'Terminal not found: %s' % name) + + def initialize_culler(self): + """Start culler if 'cull_inactive_timeout' is greater than zero. + Regardless of that value, set flag that we've been here. + """ + 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 ...", + self.cull_inactive_timeout, self.cull_interval) + self._culler_callback.start() + + self._initialized_culler = True + + async def cull_terminals(self): + self.log.debug("Polling every %s seconds for terminals inactive for > %s seconds...", + self.cull_interval, self.cull_inactive_timeout) + # Create a separate list of terminals to avoid conflicting updates while iterating + for name in list(self.terminals): + try: + await self.cull_inactive_terminal(name) + except Exception as e: + self.log.exception("The following exception was encountered while checking the " + "activity of terminal {}: {}".format(name, e)) + + async def cull_inactive_terminal(self, name): + try: + term = self.terminals[name] + except KeyError: + return # KeyErrors are somewhat expected since the terminal can be terminated as the culling check is made. + + self.log.debug("name=%s, last_activity=%s", name, term.last_activity) + if hasattr(term, 'last_activity'): + dt_now = utcnow() + dt_inactive = dt_now - term.last_activity + # Compute idle properties + is_time = dt_inactive > timedelta(seconds=self.cull_inactive_timeout) + # Cull the kernel if all three criteria are met + if (is_time): + inactivity = int(dt_inactive.total_seconds()) + self.log.warning("Culling terminal '%s' due to %s seconds of inactivity.", name, inactivity) + await self.terminate(name, force=True) From b237491049a2927c1a16abe665fc4ab6dfa7e771 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Tue, 14 Apr 2020 17:28:16 -0700 Subject: [PATCH 02/10] Terminate all terminals on server shutdown --- jupyter_server/serverapp.py | 17 +++++++++++++ jupyter_server/terminal/terminalmanager.py | 28 +++++++++++++++------- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/jupyter_server/serverapp.py b/jupyter_server/serverapp.py index fecfe641c..fd9e461d7 100755 --- a/jupyter_server/serverapp.py +++ b/jupyter_server/serverapp.py @@ -1847,6 +1847,22 @@ def cleanup_kernels(self): self.log.info(kernel_msg % n_kernels) run_sync(self.kernel_manager.shutdown_all()) + def cleanup_terminals(self): + """Shutdown all terminals. + + The terminals will shutdown themselves when this process no longer exists, + but explicit shutdown allows the TerminalManager to cleanup. + """ + try: + terminal_manager = self.web_app.settings['terminal_manager'] + except KeyError: + return # Terminals not enabled + + n_terminals = len(terminal_manager.list()) + terminal_msg = trans.ngettext('Shutting down %d terminal', 'Shutting down %d terminals', n_terminals) + self.log.info(terminal_msg % n_terminals) + run_sync(terminal_manager.terminate_all()) + def running_server_info(self, kernel_count=True): "Return the current working directory and the server url information" info = self.contents_manager.info_string() + "\n" @@ -2077,6 +2093,7 @@ def _cleanup(self): self.remove_server_info_file() self.remove_browser_open_files() self.cleanup_kernels() + self.cleanup_terminals() def start_ioloop(self): """Start the IO Loop.""" diff --git a/jupyter_server/terminal/terminalmanager.py b/jupyter_server/terminal/terminalmanager.py index b003e0966..3f0e02be0 100644 --- a/jupyter_server/terminal/terminalmanager.py +++ b/jupyter_server/terminal/terminalmanager.py @@ -8,7 +8,7 @@ from datetime import timedelta -from notebook._tz import utcnow, isoformat +from jupyter_server._tz import utcnow, isoformat from terminado import NamedTermManager from tornado import web from tornado.ioloop import IOLoop, PeriodicCallback @@ -40,8 +40,9 @@ class TerminalManager(Configurable, NamedTermManager): def __init__(self, *args, **kwargs): super(TerminalManager, self).__init__(*args, **kwargs) - def create(self): - name, term = self.new_named_terminal() + def create(self, **kwargs): + """Create a new terminal.""" + name, term = self.new_named_terminal(**kwargs) # Monkey-patch last-activity, similar to kernels. Should we need # more functionality per terminal, we can look into possible sub- # classing or containment then. @@ -50,14 +51,16 @@ def create(self): # Increase the metric by one because a new terminal was created TERMINAL_CURRENTLY_RUNNING_TOTAL.inc() # Ensure culler is initialized - self.initialize_culler() + self._initialize_culler() return model def get(self, name): + """Get terminal 'name'.""" model = self.terminal_model(name) return model def list(self): + """Get a list of all running terminals.""" models = [self.terminal_model(name) for name in self.terminals] # Update the metric below to the length of the list 'terms' @@ -67,6 +70,7 @@ def list(self): return models async def terminate(self, name, force=False): + """Terminate terminal 'name'.""" self._check_terminal(name) await super(TerminalManager, self).terminate(name, force=force) @@ -74,6 +78,12 @@ async def terminate(self, name, force=False): # because a terminal has been shutdown TERMINAL_CURRENTLY_RUNNING_TOTAL.dec() + async def terminate_all(self): + """Terminate all terminals.""" + terms = [name for name in self.terminals] + for term in terms: + await self.terminate(term, force=True) + def terminal_model(self, name): """Return a JSON-safe dict representing a terminal. For use in representing terminals in the JSON APIs. @@ -91,7 +101,7 @@ def _check_terminal(self, name): if name not in self.terminals: raise web.HTTPError(404, u'Terminal not found: %s' % name) - def initialize_culler(self): + def _initialize_culler(self): """Start culler if 'cull_inactive_timeout' is greater than zero. Regardless of that value, set flag that we've been here. """ @@ -103,25 +113,25 @@ def initialize_culler(self): 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._cull_terminals, 1000 * self.cull_interval) self.log.info("Culling terminals with inactivity > %s seconds at %s second intervals ...", self.cull_inactive_timeout, self.cull_interval) self._culler_callback.start() self._initialized_culler = True - async def cull_terminals(self): + async def _cull_terminals(self): self.log.debug("Polling every %s seconds for terminals inactive for > %s seconds...", self.cull_interval, self.cull_inactive_timeout) # Create a separate list of terminals to avoid conflicting updates while iterating for name in list(self.terminals): try: - await self.cull_inactive_terminal(name) + await self._cull_inactive_terminal(name) except Exception as e: self.log.exception("The following exception was encountered while checking the " "activity of terminal {}: {}".format(name, e)) - async def cull_inactive_terminal(self, name): + async def _cull_inactive_terminal(self, name): try: term = self.terminals[name] except KeyError: From 525906f7215142815814940210d7e6329a5c0f94 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Fri, 15 May 2020 16:09:30 -0700 Subject: [PATCH 03/10] Add cull_interval validate, rename method - per review --- jupyter_server/terminal/terminalmanager.py | 26 +++++++++++++--------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/jupyter_server/terminal/terminalmanager.py b/jupyter_server/terminal/terminalmanager.py index 3f0e02be0..fcdb2b2d0 100644 --- a/jupyter_server/terminal/terminalmanager.py +++ b/jupyter_server/terminal/terminalmanager.py @@ -6,13 +6,14 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. +import warnings from datetime import timedelta from jupyter_server._tz import utcnow, isoformat from terminado import NamedTermManager from tornado import web from tornado.ioloop import IOLoop, PeriodicCallback -from traitlets import Integer +from traitlets import Integer, validate from traitlets.config import Configurable from ..prometheus.metrics import TERMINAL_CURRENTLY_RUNNING_TOTAL @@ -25,7 +26,7 @@ class TerminalManager(Configurable, NamedTermManager): _initialized_culler = False cull_inactive_timeout = Integer(0, config=True, - help="""Timeout (in seconds) in which a terminal has been inactive and ready to be culled. + help="""Timeout (in seconds) in which a terminal has been inactive and ready to be culled. Values of 0 or lower disable culling.""" ) @@ -34,6 +35,15 @@ 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 # ------------------------------------------------------------------------- @@ -47,7 +57,7 @@ def create(self, **kwargs): # more functionality per terminal, we can look into possible sub- # classing or containment then. term.last_activity = utcnow() - model = self.terminal_model(name) + model = self.get_terminal_model(name) # Increase the metric by one because a new terminal was created TERMINAL_CURRENTLY_RUNNING_TOTAL.inc() # Ensure culler is initialized @@ -56,12 +66,12 @@ def create(self, **kwargs): def get(self, name): """Get terminal 'name'.""" - model = self.terminal_model(name) + model = self.get_terminal_model(name) return model def list(self): """Get a list of all running terminals.""" - models = [self.terminal_model(name) for name in self.terminals] + models = [self.get_terminal_model(name) for name in self.terminals] # Update the metric below to the length of the list 'terms' TERMINAL_CURRENTLY_RUNNING_TOTAL.set( @@ -84,7 +94,7 @@ async def terminate_all(self): for term in terms: await self.terminate(term, force=True) - def terminal_model(self, name): + def get_terminal_model(self, name): """Return a JSON-safe dict representing a terminal. For use in representing terminals in the JSON APIs. """ @@ -108,10 +118,6 @@ 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 ...", From da9081f8c543e61b079146c90292ac0bb2d9d298 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Mon, 18 May 2020 11:11:09 -0700 Subject: [PATCH 04/10] Apply review comments, revert interval validation --- jupyter_server/serverapp.py | 2 +- jupyter_server/terminal/__init__.py | 16 ++++++++-------- jupyter_server/terminal/terminalmanager.py | 17 ++++++----------- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/jupyter_server/serverapp.py b/jupyter_server/serverapp.py index fd9e461d7..f3d9a3396 100755 --- a/jupyter_server/serverapp.py +++ b/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) diff --git a/jupyter_server/terminal/__init__.py b/jupyter_server/terminal/__init__.py index e4ad3ee4b..62ad339af 100644 --- a/jupyter_server/terminal/__init__.py +++ b/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) diff --git a/jupyter_server/terminal/terminalmanager.py b/jupyter_server/terminal/terminalmanager.py index fcdb2b2d0..137f70837 100644 --- a/jupyter_server/terminal/terminalmanager.py +++ b/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 ...", From 261acc0d7ed4f5c4fa19f638b0580457861ac04e Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Mon, 18 May 2020 12:49:01 -0700 Subject: [PATCH 05/10] Rename parent to server_app --- jupyter_server/serverapp.py | 2 +- jupyter_server/terminal/__init__.py | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/jupyter_server/serverapp.py b/jupyter_server/serverapp.py index f3d9a3396..d2901b6df 100755 --- a/jupyter_server/serverapp.py +++ b/jupyter_server/serverapp.py @@ -1547,7 +1547,7 @@ def init_terminals(self): try: from .terminal import initialize - initialize(parent=self) + initialize(server_app=self) self.web_app.settings['terminals_available'] = True except ImportError as e: self.log.warning(_i18n("Terminals not available (error was %s)"), e) diff --git a/jupyter_server/terminal/__init__.py b/jupyter_server/terminal/__init__.py index 62ad339af..cf9549608 100644 --- a/jupyter_server/terminal/__init__.py +++ b/jupyter_server/terminal/__init__.py @@ -15,12 +15,12 @@ from .terminalmanager import TerminalManager -def initialize(parent): +def initialize(server_app): if os.name == 'nt': default_shell = 'powershell.exe' else: default_shell = which('sh') - shell_override = parent.terminado_settings.get('shell_command') + shell_override = server_app.terminado_settings.get('shell_command') shell = ( [os.environ.get('SHELL') or default_shell] if shell_override is None @@ -33,19 +33,19 @@ def initialize(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 = parent.web_app.settings['terminal_manager'] = TerminalManager( + terminal_manager = server_app.web_app.settings['terminal_manager'] = TerminalManager( shell_command=shell, - extra_env={'JUPYTER_SERVER_ROOT': parent.root_dir, - 'JUPYTER_SERVER_URL': parent.connection_url, + extra_env={'JUPYTER_SERVER_ROOT': server_app.root_dir, + 'JUPYTER_SERVER_URL': server_app.connection_url, }, - parent=parent, + parent=server_app, ) - terminal_manager.log = parent.log - base_url = parent.web_app.settings['base_url'] + terminal_manager.log = server_app.log + base_url = server_app.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), ] - parent.web_app.add_handlers(".*$", handlers) + server_app.web_app.add_handlers(".*$", handlers) From c85c1bf668e753dce34f4c6661fc70cdb2736ca3 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Fri, 5 Mar 2021 14:05:14 -0800 Subject: [PATCH 06/10] Add culling test --- jupyter_server/terminal/__init__.py | 1 - jupyter_server/terminal/api_handlers.py | 1 - jupyter_server/tests/test_terminal.py | 74 ++++++++++++++++++++++++- 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/jupyter_server/terminal/__init__.py b/jupyter_server/terminal/__init__.py index cf9549608..bcf5c74c7 100644 --- a/jupyter_server/terminal/__init__.py +++ b/jupyter_server/terminal/__init__.py @@ -8,7 +8,6 @@ raise ImportError("terminado >= 0.8.3 required, found %s" % terminado.__version__) from ipython_genutils.py3compat import which -from tornado.log import app_log from jupyter_server.utils import url_path_join as ujoin from . import api_handlers from .handlers import TerminalHandler, TermSocket diff --git a/jupyter_server/terminal/api_handlers.py b/jupyter_server/terminal/api_handlers.py index 5a2b2f854..92bb62428 100644 --- a/jupyter_server/terminal/api_handlers.py +++ b/jupyter_server/terminal/api_handlers.py @@ -3,7 +3,6 @@ from ..base.handlers import APIHandler - class TerminalRootHandler(APIHandler): @web.authenticated diff --git a/jupyter_server/tests/test_terminal.py b/jupyter_server/tests/test_terminal.py index c8534868e..18b776f5f 100644 --- a/jupyter_server/tests/test_terminal.py +++ b/jupyter_server/tests/test_terminal.py @@ -5,6 +5,9 @@ import asyncio import sys +from tornado.httpclient import HTTPClientError +from traitlets.config import Config + # Skip this whole module on Windows. The terminal API leads # to timeouts on Windows CI. if sys.platform.startswith('win'): @@ -30,12 +33,42 @@ def terminal_path(tmp_path): shutil.rmtree(str(subdir), ignore_errors=True) +CULL_TIMEOUT = 2 +CULL_INTERVAL = 3 + + +@pytest.fixture +def jp_server_config(): + return Config({ + 'ServerApp': { + 'TerminalManager': { + 'cull_inactive_timeout': CULL_TIMEOUT, + 'cull_interval': CULL_INTERVAL + } + } + }) + + +async def test_no_terminals(jp_fetch): + resp_list = await jp_fetch( + 'api', 'terminals', + method='GET', + allow_nonstandard_methods=True, + ) + + data = json.loads(resp_list.body.decode()) + + assert len(data) == 0 + + async def test_terminal_create(jp_fetch, kill_all): - await jp_fetch( + resp = await jp_fetch( 'api', 'terminals', method='POST', allow_nonstandard_methods=True, ) + term = json.loads(resp.body.decode()) + assert term['name'] == "1" resp_list = await jp_fetch( 'api', 'terminals', @@ -46,6 +79,7 @@ async def test_terminal_create(jp_fetch, kill_all): data = json.loads(resp_list.body.decode()) assert len(data) == 1 + assert data[0] == term await kill_all() @@ -104,3 +138,41 @@ async def test_terminal_create_with_cwd(jp_fetch, jp_ws_fetch, terminal_path): ws.close() assert str(terminal_path) in message_stdout + + +async def test_culling_config(jp_server_config, jp_configurable_serverapp): + terminal_mgr_config = jp_configurable_serverapp().config.ServerApp.TerminalManager + assert terminal_mgr_config.cull_inactive_timeout == CULL_TIMEOUT + assert terminal_mgr_config.cull_interval == CULL_INTERVAL + terminal_mgr_settings = jp_configurable_serverapp().web_app.settings['terminal_manager'] + assert terminal_mgr_settings.cull_inactive_timeout == CULL_TIMEOUT + assert terminal_mgr_settings.cull_interval == CULL_INTERVAL + + +async def test_culling(jp_server_config, jp_fetch): + # POST request + resp = await jp_fetch( + 'api', 'terminals', + method='POST', + allow_nonstandard_methods=True, + ) + term = json.loads(resp.body.decode()) + term_1 = term['name'] + last_activity = term['last_activity'] + + culled = False + for i in range(10): # Culling should occur in a few seconds + try: + resp = await jp_fetch( + 'api', 'terminals', term_1, + method='GET', + allow_nonstandard_methods=True, + ) + except HTTPClientError as e: + assert e.code == 404 + culled = True + break + else: + await asyncio.sleep(1) + + assert culled From 91bce709a9664bccc5b43dcc65e312ac6b96d118 Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Wed, 20 May 2020 10:19:37 +0100 Subject: [PATCH 07/10] Install terminado for docs build RTD is currently failing with an ImportError Also move to a recent Python, because I don't know how long 3.5 will be available on conda. --- docs/environment.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/environment.yml b/docs/environment.yml index 5d77bc7bb..61eee5ed3 100644 --- a/docs/environment.yml +++ b/docs/environment.yml @@ -13,4 +13,5 @@ dependencies: - sphinxcontrib_github_alt - sphinxcontrib-openapi - sphinxemoji + - terminado - git+https://github.com/pandas-dev/pydata-sphinx-theme.git@master \ No newline at end of file From 1ffe307889454a177d987cc4c0fa378274c31204 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Mon, 25 May 2020 02:47:03 -0700 Subject: [PATCH 08/10] Restore detection of missing terminado package * Restore detection of missing terminado package * Properly handle terminals_enabled config * Disambiguate terminado availability from web settings * Further clarity on terminal availability * Rename terminals_in_use back to terminals_available --- jupyter_server/serverapp.py | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/jupyter_server/serverapp.py b/jupyter_server/serverapp.py index d2901b6df..529d8f7e4 100755 --- a/jupyter_server/serverapp.py +++ b/jupyter_server/serverapp.py @@ -83,7 +83,6 @@ from .auth.login import LoginHandler from .auth.logout import LogoutHandler from .base.handlers import FileFindHandler -from .terminal import TerminalManager from traitlets.config import Config from traitlets.config.application import catch_config_error, boolean_flag @@ -117,6 +116,13 @@ from jupyter_server.extension.config import ExtensionConfigManager from jupyter_server.traittypes import TypeFromClasses +# Tolerate missing terminado package. +try: + from .terminal import TerminalManager + terminado_available = True +except ImportError: + terminado_available = False + #----------------------------------------------------------------------------- # Module globals #----------------------------------------------------------------------------- @@ -285,7 +291,7 @@ def init_settings(self, jupyter_app, kernel_manager, contents_manager, allow_password_change=jupyter_app.allow_password_change, server_root_dir=root_dir, jinja2_env=env, - terminals_available=False, # Set later if terminals are available + terminals_available=terminado_available and jupyter_app.terminals_enabled, serverapp=jupyter_app ) @@ -588,8 +594,10 @@ class ServerApp(JupyterApp): classes = [ KernelManager, Session, MappingKernelManager, KernelSpecManager, AsyncMappingKernelManager, ContentsManager, FileContentsManager, AsyncContentsManager, AsyncFileContentsManager, NotebookNotary, - TerminalManager, GatewayKernelSpecManager, GatewaySessionManager, GatewayClient + GatewayKernelManager, GatewayKernelSpecManager, GatewaySessionManager, GatewayClient ] + if terminado_available: # Only necessary when terminado is available + classes.append(TerminalManager) subcommands = dict( list=(JupyterServerListApp, JupyterServerListApp.description.splitlines()[0]), @@ -1330,6 +1338,15 @@ def _update_server_extensions(self, change): is not available. """)) + # Since use of terminals is also a function of whether the terminado package is + # available, this variable holds the "final indication" of whether terminal functionality + # should be considered (particularly during shutdown/cleanup). It is enabled only + # once both the terminals "service" can be initialized and terminals_enabled is True. + # Note: this variable is slightly different from 'terminals_available' in the web settings + # in that this variable *could* remain false if terminado is available, yet the terminal + # service's initialization still fails. As a result, this variable holds the truth. + terminals_available = False + authenticate_prometheus = Bool( True, help="""" @@ -1548,7 +1565,7 @@ def init_terminals(self): try: from .terminal import initialize initialize(server_app=self) - self.web_app.settings['terminals_available'] = True + self.terminals_available = True except ImportError as e: self.log.warning(_i18n("Terminals not available (error was %s)"), e) @@ -1694,11 +1711,8 @@ def shutdown_no_activity(self): if len(km) != 0: return # Kernels still running - try: + if self.terminals_available: term_mgr = self.web_app.settings['terminal_manager'] - except KeyError: - pass # Terminals not enabled - else: if term_mgr.terminals: return # Terminals still running @@ -1853,11 +1867,10 @@ def cleanup_terminals(self): The terminals will shutdown themselves when this process no longer exists, but explicit shutdown allows the TerminalManager to cleanup. """ - try: - terminal_manager = self.web_app.settings['terminal_manager'] - except KeyError: - return # Terminals not enabled + if not self.terminals_available: + return + terminal_manager = self.web_app.settings['terminal_manager'] n_terminals = len(terminal_manager.list()) terminal_msg = trans.ngettext('Shutting down %d terminal', 'Shutting down %d terminals', n_terminals) self.log.info(terminal_msg % n_terminals) From 7f5440df81229dc56f5f99a62d0f97f7794ffea5 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Sat, 6 Mar 2021 08:16:06 -0800 Subject: [PATCH 09/10] Fix initialization --- jupyter_server/serverapp.py | 2 +- jupyter_server/terminal/__init__.py | 19 ++++++++++--------- jupyter_server/terminal/terminalmanager.py | 7 +++---- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/jupyter_server/serverapp.py b/jupyter_server/serverapp.py index 529d8f7e4..ecd0266f9 100755 --- a/jupyter_server/serverapp.py +++ b/jupyter_server/serverapp.py @@ -1564,7 +1564,7 @@ def init_terminals(self): try: from .terminal import initialize - initialize(server_app=self) + initialize(self.web_app, self.root_dir, self.connection_url, self.terminado_settings) self.terminals_available = True except ImportError as e: self.log.warning(_i18n("Terminals not available (error was %s)"), e) diff --git a/jupyter_server/terminal/__init__.py b/jupyter_server/terminal/__init__.py index bcf5c74c7..d236a536b 100644 --- a/jupyter_server/terminal/__init__.py +++ b/jupyter_server/terminal/__init__.py @@ -14,12 +14,12 @@ from .terminalmanager import TerminalManager -def initialize(server_app): +def initialize(webapp, root_dir, connection_url, settings): if os.name == 'nt': default_shell = 'powershell.exe' else: default_shell = which('sh') - shell_override = server_app.terminado_settings.get('shell_command') + shell_override = settings.get('shell_command') shell = ( [os.environ.get('SHELL') or default_shell] if shell_override is None @@ -32,19 +32,20 @@ def initialize(server_app): # 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 = server_app.web_app.settings['terminal_manager'] = TerminalManager( + terminal_manager = webapp.settings['terminal_manager'] = TerminalManager( shell_command=shell, - extra_env={'JUPYTER_SERVER_ROOT': server_app.root_dir, - 'JUPYTER_SERVER_URL': server_app.connection_url, + extra_env={'JUPYTER_SERVER_ROOT': root_dir, + 'JUPYTER_SERVER_URL': connection_url, }, - parent=server_app, + parent=webapp.settings['serverapp'], ) - terminal_manager.log = server_app.log - base_url = server_app.web_app.settings['base_url'] + terminal_manager.log = webapp.settings['serverapp'].log + base_url = webapp.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), ] - server_app.web_app.add_handlers(".*$", handlers) + webapp.add_handlers(".*$", handlers) diff --git a/jupyter_server/terminal/terminalmanager.py b/jupyter_server/terminal/terminalmanager.py index 137f70837..38144855b 100644 --- a/jupyter_server/terminal/terminalmanager.py +++ b/jupyter_server/terminal/terminalmanager.py @@ -6,19 +6,18 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. -import warnings +import terminado from datetime import timedelta from jupyter_server._tz import utcnow, isoformat -from terminado import NamedTermManager from tornado import web from tornado.ioloop import IOLoop, PeriodicCallback -from traitlets import Integer, validate +from traitlets import Integer from traitlets.config import LoggingConfigurable from ..prometheus.metrics import TERMINAL_CURRENTLY_RUNNING_TOTAL -class TerminalManager(LoggingConfigurable, NamedTermManager): +class TerminalManager(LoggingConfigurable, terminado.NamedTermManager): """ """ _culler_callback = None From 4f5f6916be4786762938a6c1903efbdac670eb9a Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Tue, 9 Mar 2021 17:56:21 -0800 Subject: [PATCH 10/10] Remove TerminalHandler (resides in nbclassic now) --- jupyter_server/terminal/__init__.py | 3 +-- jupyter_server/terminal/handlers.py | 8 -------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/jupyter_server/terminal/__init__.py b/jupyter_server/terminal/__init__.py index d236a536b..6e9171cb9 100644 --- a/jupyter_server/terminal/__init__.py +++ b/jupyter_server/terminal/__init__.py @@ -10,7 +10,7 @@ from ipython_genutils.py3compat import which from jupyter_server.utils import url_path_join as ujoin from . import api_handlers -from .handlers import TerminalHandler, TermSocket +from .handlers import TermSocket from .terminalmanager import TerminalManager @@ -42,7 +42,6 @@ def initialize(webapp, root_dir, connection_url, settings): terminal_manager.log = webapp.settings['serverapp'].log base_url = webapp.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), diff --git a/jupyter_server/terminal/handlers.py b/jupyter_server/terminal/handlers.py index d88769b60..d01edd38e 100644 --- a/jupyter_server/terminal/handlers.py +++ b/jupyter_server/terminal/handlers.py @@ -11,14 +11,6 @@ from ..base.zmqhandlers import WebSocketMixin -class TerminalHandler(JupyterHandler): - """Render the terminal interface.""" - @web.authenticated - def get(self, term_name): - self.write(self.render_template('terminal.html', - ws_path="terminals/websocket/%s" % term_name)) - - class TermSocket(WebSocketMixin, JupyterHandler, terminado.TermSocket): def origin_check(self):