Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore detection of missing terminado package #5465

Merged
merged 6 commits into from
May 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 26 additions & 13 deletions notebook/notebookapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -132,6 +131,13 @@
except ImportError:
async_kernel_mgmt_available = False

# Tolerate missing terminado package.
try:
from .terminal import TerminalManager
terminado_available = True
except ImportError:
terminado_available = False

#-----------------------------------------------------------------------------
# Module globals
#-----------------------------------------------------------------------------
Expand Down Expand Up @@ -303,7 +309,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,
)

# allow custom overrides for the tornado web app.
Expand Down Expand Up @@ -673,9 +679,12 @@ class NotebookApp(JupyterApp):

classes = [
KernelManager, Session, MappingKernelManager, KernelSpecManager,
ContentsManager, FileContentsManager, NotebookNotary, TerminalManager,
ContentsManager, FileContentsManager, NotebookNotary,
GatewayKernelManager, GatewayKernelSpecManager, GatewaySessionManager, GatewayClient,
]
if terminado_available: # Only necessary when terminado is available
classes.append(TerminalManager)

flags = Dict(flags)
aliases = Dict(aliases)

Expand Down Expand Up @@ -1486,6 +1495,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

def parse_command_line(self, argv=None):
super(NotebookApp, self).parse_command_line(argv)

Expand Down Expand Up @@ -1777,7 +1795,7 @@ def init_terminals(self):
try:
from .terminal import initialize
initialize(nb_app=self)
self.web_app.settings['terminals_available'] = True
self.terminals_available = True
except ImportError as e:
self.log.warning(_("Terminals not available (error was %s)"), e)

Expand Down Expand Up @@ -1919,18 +1937,14 @@ 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

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

Expand Down Expand Up @@ -2018,11 +2032,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)
Expand Down
5 changes: 2 additions & 3 deletions notebook/terminal/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down