From 030e879eb7eb68c8e24b9af2c27e9f460960fea9 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Wed, 20 May 2020 10:47:21 -0700 Subject: [PATCH 1/5] Restore detection of missing terminado package --- notebook/notebookapp.py | 37 +++++++++++++++++++++-------------- notebook/terminal/__init__.py | 5 ++--- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/notebook/notebookapp.py b/notebook/notebookapp.py index 6f64ef82b0..4c4018f392 100755 --- a/notebook/notebookapp.py +++ b/notebook/notebookapp.py @@ -91,7 +91,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 @@ -132,6 +131,13 @@ except ImportError: async_kernel_mgmt_available = False +# Tolerate missing terminado package. +try: + from .terminal import TerminalManager + terminals_available = True +except ImportError: + terminals_available = False + #----------------------------------------------------------------------------- # Module globals #----------------------------------------------------------------------------- @@ -300,7 +306,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=terminals_available, ) # allow custom overrides for the tornado web app. @@ -660,9 +666,12 @@ class NotebookApp(JupyterApp): classes = [ KernelManager, Session, MappingKernelManager, KernelSpecManager, - ContentsManager, FileContentsManager, NotebookNotary, TerminalManager, + ContentsManager, FileContentsManager, NotebookNotary, GatewayKernelManager, GatewayKernelSpecManager, GatewaySessionManager, GatewayClient, ] + if terminals_available: # Only necessary when terminals are available + classes.append(TerminalManager) + flags = Dict(flags) aliases = Dict(aliases) @@ -1759,8 +1768,8 @@ def init_terminals(self): try: from .terminal import initialize initialize(nb_app=self) - self.web_app.settings['terminals_available'] = True except ImportError as e: + self.terminals_enabled = False self.log.warning(_("Terminals not available (error was %s)"), e) def init_signal(self): @@ -1908,13 +1917,12 @@ def shutdown_no_activity(self): if len(km) != 0: return # Kernels still running - try: - term_mgr = self.web_app.settings['terminal_manager'] - except KeyError: - pass # Terminals not enabled - else: - if term_mgr.terminals: - return # Terminals still running + if not self.terminals_enabled: + return + + term_mgr = self.web_app.settings['terminal_manager'] + if term_mgr.terminals: + return # Terminals still running seconds_since_active = \ (utcnow() - self.web_app.last_activity()).total_seconds() @@ -2000,11 +2008,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_enabled: + 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) diff --git a/notebook/terminal/__init__.py b/notebook/terminal/__init__.py index 0e43915184..ce07cf6047 100644 --- a/notebook/terminal/__init__.py +++ b/notebook/terminal/__init__.py @@ -3,11 +3,10 @@ import terminado from ..utils import check_version -if not check_version(terminado.__version__, '0.8.1'): - raise ImportError("terminado >= 0.8.1 required, found %s" % terminado.__version__) +if not check_version(terminado.__version__, '0.8.3'): + raise ImportError("terminado >= 0.8.3 required, found %s" % terminado.__version__) from ipython_genutils.py3compat import which -from tornado.log import app_log from notebook.utils import url_path_join as ujoin from .terminalmanager import TerminalManager from .handlers import TerminalHandler, TermSocket From a06c79ba700edf0b7359769ed1ce84632b7e21e4 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Wed, 20 May 2020 12:48:18 -0700 Subject: [PATCH 2/5] Properly handle terminals_enabled config --- notebook/notebookapp.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/notebook/notebookapp.py b/notebook/notebookapp.py index 4c4018f392..c287879745 100755 --- a/notebook/notebookapp.py +++ b/notebook/notebookapp.py @@ -178,13 +178,13 @@ class NotebookWebApplication(web.Application): def __init__(self, jupyter_app, kernel_manager, contents_manager, session_manager, kernel_spec_manager, config_manager, extra_services, log, - base_url, default_url, settings_overrides, jinja_env_options): + base_url, default_url, settings_overrides, jinja_env_options, terminals_enabled): settings = self.init_settings( jupyter_app, kernel_manager, contents_manager, session_manager, kernel_spec_manager, config_manager, extra_services, log, base_url, - default_url, settings_overrides, jinja_env_options) + default_url, settings_overrides, jinja_env_options, terminals_enabled) handlers = self.init_handlers(settings) super(NotebookWebApplication, self).__init__(handlers, **settings) @@ -193,7 +193,7 @@ def init_settings(self, jupyter_app, kernel_manager, contents_manager, session_manager, kernel_spec_manager, config_manager, extra_services, log, base_url, default_url, settings_overrides, - jinja_env_options=None): + jinja_env_options=None, terminals_enabled=True): _template_path = settings_overrides.get( "template_path", @@ -306,7 +306,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=terminals_available, + terminals_available=terminals_available and terminals_enabled, ) # allow custom overrides for the tornado web app. @@ -1632,7 +1632,7 @@ def init_webapp(self): self.session_manager, self.kernel_spec_manager, self.config_manager, self.extra_services, self.log, self.base_url, self.default_url, self.tornado_settings, - self.jinja_environment_options, + self.jinja_environment_options, self.terminals_enabled, ) ssl_options = self.ssl_options if self.certfile: From 77aa2d9a923ac58d6e86bc380342034b28c6e253 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Wed, 20 May 2020 13:36:17 -0700 Subject: [PATCH 3/5] Disambiguate terminado availability from web settings --- notebook/notebookapp.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/notebook/notebookapp.py b/notebook/notebookapp.py index c287879745..3d863e122e 100755 --- a/notebook/notebookapp.py +++ b/notebook/notebookapp.py @@ -134,9 +134,9 @@ # Tolerate missing terminado package. try: from .terminal import TerminalManager - terminals_available = True + terminado_available = True except ImportError: - terminals_available = False + terminado_available = False #----------------------------------------------------------------------------- # Module globals @@ -306,7 +306,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=terminals_available and terminals_enabled, + terminals_available=terminado_available and terminals_enabled, ) # allow custom overrides for the tornado web app. @@ -669,7 +669,7 @@ class NotebookApp(JupyterApp): ContentsManager, FileContentsManager, NotebookNotary, GatewayKernelManager, GatewayKernelSpecManager, GatewaySessionManager, GatewayClient, ] - if terminals_available: # Only necessary when terminals are available + if terminado_available: # Only necessary when terminado is available classes.append(TerminalManager) flags = Dict(flags) From 1841897ddbf0ea4209c5846ae359826a6a238186 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Thu, 21 May 2020 07:24:57 -0700 Subject: [PATCH 4/5] Further clarity on terminal availability --- notebook/notebookapp.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/notebook/notebookapp.py b/notebook/notebookapp.py index 3d863e122e..9282a56340 100755 --- a/notebook/notebookapp.py +++ b/notebook/notebookapp.py @@ -178,13 +178,13 @@ class NotebookWebApplication(web.Application): def __init__(self, jupyter_app, kernel_manager, contents_manager, session_manager, kernel_spec_manager, config_manager, extra_services, log, - base_url, default_url, settings_overrides, jinja_env_options, terminals_enabled): + base_url, default_url, settings_overrides, jinja_env_options): settings = self.init_settings( jupyter_app, kernel_manager, contents_manager, session_manager, kernel_spec_manager, config_manager, extra_services, log, base_url, - default_url, settings_overrides, jinja_env_options, terminals_enabled) + default_url, settings_overrides, jinja_env_options) handlers = self.init_handlers(settings) super(NotebookWebApplication, self).__init__(handlers, **settings) @@ -193,7 +193,7 @@ def init_settings(self, jupyter_app, kernel_manager, contents_manager, session_manager, kernel_spec_manager, config_manager, extra_services, log, base_url, default_url, settings_overrides, - jinja_env_options=None, terminals_enabled=True): + jinja_env_options=None): _template_path = settings_overrides.get( "template_path", @@ -306,7 +306,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=terminado_available and terminals_enabled, + terminals_available=terminado_available and jupyter_app.terminals_enabled, ) # allow custom overrides for the tornado web app. @@ -1478,6 +1478,12 @@ def _update_server_extensions(self, change): is not available. """)) + # Since use of terminals is also a function of whether the terminado package is + # avaialble, 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. + terminals_in_use = False + def parse_command_line(self, argv=None): super(NotebookApp, self).parse_command_line(argv) @@ -1632,7 +1638,7 @@ def init_webapp(self): self.session_manager, self.kernel_spec_manager, self.config_manager, self.extra_services, self.log, self.base_url, self.default_url, self.tornado_settings, - self.jinja_environment_options, self.terminals_enabled, + self.jinja_environment_options, ) ssl_options = self.ssl_options if self.certfile: @@ -1768,8 +1774,8 @@ def init_terminals(self): try: from .terminal import initialize initialize(nb_app=self) + self.terminals_in_use = True except ImportError as e: - self.terminals_enabled = False self.log.warning(_("Terminals not available (error was %s)"), e) def init_signal(self): @@ -1917,7 +1923,7 @@ def shutdown_no_activity(self): if len(km) != 0: return # Kernels still running - if not self.terminals_enabled: + if not self.terminals_in_use: return term_mgr = self.web_app.settings['terminal_manager'] @@ -2008,7 +2014,7 @@ def cleanup_terminals(self): The terminals will shutdown themselves when this process no longer exists, but explicit shutdown allows the TerminalManager to cleanup. """ - if not self.terminals_enabled: + if not self.terminals_in_use: return terminal_manager = self.web_app.settings['terminal_manager'] From 4b36b47d16a63cf079a4472a9ccffe0e35fd3c3f Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Sun, 24 May 2020 07:50:48 -0700 Subject: [PATCH 5/5] Rename terminals_in_use back to terminals_available --- notebook/notebookapp.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/notebook/notebookapp.py b/notebook/notebookapp.py index 9282a56340..7ac20b2a1b 100755 --- a/notebook/notebookapp.py +++ b/notebook/notebookapp.py @@ -1479,10 +1479,13 @@ def _update_server_extensions(self, change): """)) # Since use of terminals is also a function of whether the terminado package is - # avaialble, this variable holds the "final indication" of whether terminal functionality + # 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. - terminals_in_use = False + # 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 def parse_command_line(self, argv=None): super(NotebookApp, self).parse_command_line(argv) @@ -1774,7 +1777,7 @@ def init_terminals(self): try: from .terminal import initialize initialize(nb_app=self) - self.terminals_in_use = True + self.terminals_available = True except ImportError as e: self.log.warning(_("Terminals not available (error was %s)"), e) @@ -1887,7 +1890,6 @@ def init_server_extensions(self): The extension API is experimental, and may change in future releases. """ - for modulename, enabled in sorted(self.nbserver_extensions.items()): if enabled: @@ -1916,14 +1918,13 @@ def init_mime_overrides(self): mimetypes.add_type('text/css', '.css') mimetypes.add_type('application/javascript', '.js') - def shutdown_no_activity(self): """Shutdown server on timeout when there are no kernels or terminals.""" km = self.kernel_manager if len(km) != 0: return # Kernels still running - if not self.terminals_in_use: + if not self.terminals_available: return term_mgr = self.web_app.settings['terminal_manager'] @@ -2014,7 +2015,7 @@ def cleanup_terminals(self): The terminals will shutdown themselves when this process no longer exists, but explicit shutdown allows the TerminalManager to cleanup. """ - if not self.terminals_in_use: + if not self.terminals_available: return terminal_manager = self.web_app.settings['terminal_manager']