From 93a35e1590c6ace52114ef57d4d8c2cb84e7d1c1 Mon Sep 17 00:00:00 2001 From: Jitse Niesen Date: Fri, 31 Mar 2023 16:09:44 +0100 Subject: [PATCH 1/3] Specify file for storing connection info when starting server Add a command line parameter to the notebook server for specifying the file in which connection information is to be stored. Use this parameter to set the file name to spynbserver-NNNN-MM.json where NNNN is the PID of the Spyder process and MM distinguishes between the notebook servers started by Spyder. The reason for this is that Jupyter by default uses nbserver-XXXX.json where XXXX is the PID of the notebook server process, but in a venv in Windows there is no good method for finding this PID. --- spyder_notebook/server/main.py | 17 +++++++-- spyder_notebook/utils/servermanager.py | 35 +++++++++++------ .../utils/tests/test_servermanager.py | 38 ++++++++++--------- .../widgets/tests/test_serverinfo.py | 2 + 4 files changed, 60 insertions(+), 32 deletions(-) diff --git a/spyder_notebook/server/main.py b/spyder_notebook/server/main.py index 17e47a15..8e6a85cb 100644 --- a/spyder_notebook/server/main.py +++ b/spyder_notebook/server/main.py @@ -6,18 +6,19 @@ import os from jinja2 import FileSystemLoader from notebook.base.handlers import IPythonHandler, FileFindHandler -from notebook.notebookapp import flags, NotebookApp +from notebook.notebookapp import aliases, flags, NotebookApp from notebook.utils import url_path_join as ujoin -from traitlets import Bool +from traitlets import Bool, Dict, Unicode HERE = os.path.dirname(__file__) +aliases['info-file'] = 'SpyderNotebookServer.info_file_cmdline' + flags['dark'] = ( {'SpyderNotebookServer': {'dark_theme': True}}, 'Use dark theme when rendering notebooks' ) - class NotebookHandler(IPythonHandler): """ Serve a notebook file from the filesystem in the notebook interface @@ -55,15 +56,23 @@ def get_template(self, name): class SpyderNotebookServer(NotebookApp): """Server rendering notebooks in HTML and serving them over HTTP.""" - flags = flags + flags = Dict(flags) + aliases = Dict(aliases) dark_theme = Bool( False, config=True, help='Whether to use dark theme when rendering notebooks') + info_file_cmdline = Unicode( + '', config=True, + help='Name of file in Jupyter runtime dir with connection info') + def init_webapp(self): """Initialize tornado webapp and httpserver.""" self.tornado_settings['dark_theme'] = self.dark_theme + if self.info_file_cmdline: + self.info_file = os.path.join( + self.runtime_dir, self.info_file_cmdline) super().init_webapp() diff --git a/spyder_notebook/utils/servermanager.py b/spyder_notebook/utils/servermanager.py index 61c46584..d46cbc76 100644 --- a/spyder_notebook/utils/servermanager.py +++ b/spyder_notebook/utils/servermanager.py @@ -10,6 +10,7 @@ import enum import json import logging +import os import os.path as osp import sys @@ -53,8 +54,9 @@ class ServerProcess: This is a data class. """ - def __init__(self, process, notebook_dir, interpreter, starttime=None, - state=ServerState.STARTING, server_info=None, output=''): + def __init__(self, process, notebook_dir, interpreter, infofile, + starttime=None, state=ServerState.STARTING, server_info=None, + output=''): """ Construct a ServerProcess. @@ -66,15 +68,17 @@ def __init__(self, process, notebook_dir, interpreter, starttime=None, Directory from which the server can render notebooks. interpreter : str File name of Python interpreter used to render notebooks. + infofile : str + Name of JSON file in jupyter_runtime_dir() with connection + information for the server. starttime : datetime or None, optional Time at which the process was started. The default is None, meaning that the current time should be used. state : ServerState, optional State of the server process. The default is ServerState.STARTING. server_info : dict or None, optional - If set, this is a dict with information given by the server in - a JSON file in jupyter_runtime_dir(). It has keys like 'url' and - 'token'. The default is None. + If set, this is a dict with the information in infofile. It has + keys like 'url' and 'token'. The default is None. output : str Output of the server process from stdout and stderr. The default is ''. @@ -82,6 +86,7 @@ def __init__(self, process, notebook_dir, interpreter, starttime=None, self.process = process self.notebook_dir = notebook_dir self.interpreter = interpreter + self.infofile = infofile self.starttime = starttime or datetime.datetime.now() self.state = state self.server_info = server_info @@ -175,6 +180,10 @@ def start_server(self, filename, interpreter): will check periodically whether the server is accepting requests and emit `sig_server_started` or `sig_server_timed_out` when appropriate. + Every server uses a unique file to store its connection number in. + The name of this file is based on `self.servers`, under the assumption + that entries are never removed from this list. + Parameters ---------- filename : str @@ -192,11 +201,14 @@ def start_server(self, filename, interpreter): process = QProcess(None) serverscript = osp.join(osp.dirname(__file__), '../server/main.py') serverscript = osp.normpath(serverscript) + my_pid = os.getpid() + server_index = len(self.servers) + 1 + infofile = f'spynbserver-{my_pid}-{server_index}.json' arguments = [serverscript, '--no-browser', - '--notebook-dir={}'.format(nbdir), + f'--info-file={infofile}', + f'--notebook-dir={nbdir}', '--NotebookApp.password=', - '--KernelSpecManager.kernel_spec_class={}'.format( - KERNELSPEC)] + f'--KernelSpecManager.kernel_spec_class={KERNELSPEC}'] if self.dark_theme: arguments.append('--dark') logger.debug('Arguments: %s', repr(arguments)) @@ -207,7 +219,8 @@ def start_server(self, filename, interpreter): process.setProcessEnvironment(env) server_process = ServerProcess( - process, notebook_dir=nbdir, interpreter=interpreter) + process, notebook_dir=nbdir, interpreter=interpreter, + infofile=infofile) process.setProcessChannelMode(QProcess.MergedChannels) process.readyReadStandardOutput.connect( lambda: self.read_server_output(server_process)) @@ -246,14 +259,14 @@ def _check_server_started(self, server_process): if server_process.state != ServerState.STARTING: return - pid = server_process.process.processId() runtime_dir = jupyter_runtime_dir() - filename = osp.join(runtime_dir, 'nbserver-{}.json'.format(pid)) + filename = osp.join(runtime_dir, server_process.infofile) try: with open(filename, encoding='utf-8') as f: server_info = json.load(f) except OSError: # E.g., file does not exist + logger.debug('_check_server_started: OSError') delay = datetime.datetime.now() - server_process.starttime if delay > datetime.timedelta(seconds=SERVER_TIMEOUT_DELAY): logger.debug('Notebook server for %s timed out', diff --git a/spyder_notebook/utils/tests/test_servermanager.py b/spyder_notebook/utils/tests/test_servermanager.py index 5db597d5..a90c54cd 100755 --- a/spyder_notebook/utils/tests/test_servermanager.py +++ b/spyder_notebook/utils/tests/test_servermanager.py @@ -59,7 +59,7 @@ def test_get_server_with_server( server_info = mocker.Mock(spec=dict) server = ServerProcess( mocker.Mock(spec=QProcess), osp.abspath(nbdir), interpreter, - state=state, server_info=server_info) + 'info.json', state=state, server_info=server_info) serverManager.servers.append(server) res = serverManager.get_server(filename, interpreter='ham') @@ -114,15 +114,16 @@ def test_check_server_started_if_started(mocker, qtbot): mocker.mock_open(read_data='{"foo": 42}')) mocker.patch('spyder_notebook.utils.servermanager.jupyter_runtime_dir', return_value='runtimedir') - mock_process = mocker.Mock(spec=QProcess, processId=lambda: 7) - server_process = ServerProcess(mock_process, 'notebookdir', 'interpreter') + mock_process = mocker.Mock(spec=QProcess) + server_process = ServerProcess( + mock_process, 'notebookdir', 'interpreter', 'info.json') serverManager = ServerManager() with qtbot.waitSignal(serverManager.sig_server_started): serverManager._check_server_started(server_process) fake_open.assert_called_once_with( - osp.join('runtimedir', 'nbserver-7.json'), encoding='utf-8') + osp.join('runtimedir', 'info.json'), encoding='utf-8') assert server_process.state == ServerState.RUNNING assert server_process.server_info == {'foo': 42} @@ -136,14 +137,15 @@ def test_check_server_started_if_not_started(mocker, qtbot): return_value='runtimedir') mock_QTimer = mocker.patch('spyder_notebook.utils.servermanager.QTimer', spec=QTimer) - mock_process = mocker.Mock(spec=QProcess, processId=lambda: 7) - server_process = ServerProcess(mock_process, 'notebookdir', 'interpreter') + mock_process = mocker.Mock(spec=QProcess) + server_process = ServerProcess( + mock_process, 'notebookdir', 'interpreter', 'info.json') serverManager = ServerManager() serverManager._check_server_started(server_process) fake_open.assert_called_once_with( - osp.join('runtimedir', 'nbserver-7.json'), encoding='utf-8') + osp.join('runtimedir', 'info.json'), encoding='utf-8') assert server_process.state == ServerState.STARTING mock_QTimer.singleShot.assert_called_once() @@ -155,17 +157,18 @@ def test_check_server_started_if_timed_out(mocker, qtbot): side_effect=OSError) mocker.patch('spyder_notebook.utils.servermanager.jupyter_runtime_dir', return_value='runtimedir') - mock_process = mocker.Mock(spec=QProcess, processId=lambda: 7) + mock_process = mocker.Mock(spec=QProcess) one_hour_ago = datetime.datetime.now() - datetime.timedelta(hours=1) server_process = ServerProcess( - mock_process, 'notebookdir', 'interpreter', starttime=one_hour_ago) + mock_process, 'notebookdir', 'interpreter', 'info.json', + starttime=one_hour_ago) serverManager = ServerManager() with qtbot.waitSignal(serverManager.sig_server_timed_out): serverManager._check_server_started(server_process) fake_open.assert_called_once_with( - osp.join('runtimedir', 'nbserver-7.json'), encoding='utf-8') + osp.join('runtimedir', 'info.json'), encoding='utf-8') assert server_process.state == ServerState.TIMED_OUT @@ -175,9 +178,10 @@ def test_check_server_started_if_errored(mocker, qtbot): fake_open = mocker.patch('spyder_notebook.utils.servermanager.open') mock_QTimer = mocker.patch('spyder_notebook.utils.servermanager.QTimer', spec=QTimer) - mock_process = mocker.Mock(spec=QProcess, processId=lambda: 7) + mock_process = mocker.Mock(spec=QProcess) server_process = ServerProcess( - mock_process, 'notebookdir', 'interpreter', state=ServerState.ERROR) + mock_process, 'notebookdir', 'interpreter', 'info.json', + state=ServerState.ERROR) serverManager = ServerManager() serverManager._check_server_started(server_process) @@ -193,10 +197,10 @@ def test_shutdown_all_servers(mocker): mock_shutdown = mocker.patch( 'spyder_notebook.utils.servermanager.notebookapp.shutdown_server') server1 = ServerProcess( - mocker.Mock(spec=QProcess), '', '', state=ServerState.RUNNING, + mocker.Mock(spec=QProcess), '', '', '', state=ServerState.RUNNING, server_info=mocker.Mock(dict)) server2 = ServerProcess( - mocker.Mock(spec=QProcess), '', '', state=ServerState.ERROR, + mocker.Mock(spec=QProcess), '', '', '', state=ServerState.ERROR, server_info=mocker.Mock(dict)) serverManager = ServerManager() serverManager.servers = [server1, server2] @@ -214,7 +218,7 @@ def test_read_standard_output(mocker): output = 'Αθήνα\n' # check that we can handle non-ascii mock_read = mocker.Mock(return_value=QByteArray(output.encode())) mock_process = mocker.Mock(spec=QProcess, readAllStandardOutput=mock_read) - server = ServerProcess(mock_process, '', '', output=before) + server = ServerProcess(mock_process, '', '', '', output=before) serverManager = ServerManager() serverManager.servers = [server] @@ -226,7 +230,7 @@ def test_read_standard_output(mocker): def test_handle_error(mocker, qtbot): """Test that .handle_error() changes the state and emits signal.""" - server = ServerProcess(mocker.Mock(spec=QProcess), '', '') + server = ServerProcess(mocker.Mock(spec=QProcess), '', '', '') serverManager = ServerManager() with qtbot.waitSignal(serverManager.sig_server_errored): @@ -237,7 +241,7 @@ def test_handle_error(mocker, qtbot): def test_handle_finished(mocker, qtbot): """Test that .handle_finished() changes the state.""" - server = ServerProcess(mocker.Mock(spec=QProcess), '', '') + server = ServerProcess(mocker.Mock(spec=QProcess), '', '', '') serverManager = ServerManager() serverManager.handle_finished(server, mocker.Mock(), mocker.Mock()) diff --git a/spyder_notebook/widgets/tests/test_serverinfo.py b/spyder_notebook/widgets/tests/test_serverinfo.py index e414eb2e..ca2a8c3b 100755 --- a/spyder_notebook/widgets/tests/test_serverinfo.py +++ b/spyder_notebook/widgets/tests/test_serverinfo.py @@ -30,10 +30,12 @@ def dialog(qtbot): """Construct and return dialog window for testing.""" servers = [ServerProcess(FakeProcess(42), '/my/home/dir', interpreter='/ham/interpreter', + infofile='info1.json', state=ServerState.RUNNING, output='Nicely humming along...\n'), ServerProcess(FakeProcess(404), '/some/other/dir', interpreter='/spam/interpreter', + infofile='info2.json', state=ServerState.FINISHED, output='Terminated for some reason...\n')] res = ServerInfoDialog(servers) From ba27cb8c767ee5c8249d53d59fffbd641192f62c Mon Sep 17 00:00:00 2001 From: Jitse Niesen Date: Fri, 31 Mar 2023 19:29:37 +0100 Subject: [PATCH 2/3] Do not compare PID when server is started This is part of the change in the previous commit: In a venv on Windows, the server process is different from the process that starts the server. --- spyder_notebook/utils/servermanager.py | 2 +- spyder_notebook/widgets/notebooktabwidget.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/spyder_notebook/utils/servermanager.py b/spyder_notebook/utils/servermanager.py index d46cbc76..4af5a254 100644 --- a/spyder_notebook/utils/servermanager.py +++ b/spyder_notebook/utils/servermanager.py @@ -266,7 +266,7 @@ def _check_server_started(self, server_process): with open(filename, encoding='utf-8') as f: server_info = json.load(f) except OSError: # E.g., file does not exist - logger.debug('_check_server_started: OSError') + logger.debug(f'OSError when opening {filename}') delay = datetime.datetime.now() - server_process.starttime if delay > datetime.timedelta(seconds=SERVER_TIMEOUT_DELAY): logger.debug('Notebook server for %s timed out', diff --git a/spyder_notebook/widgets/notebooktabwidget.py b/spyder_notebook/widgets/notebooktabwidget.py index ecb18ec6..4a616f48 100755 --- a/spyder_notebook/widgets/notebooktabwidget.py +++ b/spyder_notebook/widgets/notebooktabwidget.py @@ -450,14 +450,13 @@ def handle_server_started(self, process): process : ServerProcess Info about the server that has started. """ - pid = process.process.processId() for client_index in range(self.count()): client = self.widget(client_index) if not client.static and not client.server_url: logger.debug('Getting server for %s', client.filename) server_info = self.server_manager.get_server( client.filename, process.interpreter, start=False) - if server_info and server_info['pid'] == pid: + if server_info: logger.debug('Success') client.register(server_info) client.load_notebook() From f1dd1e85a114cd8d7c36a62f57530d2c99301fb2 Mon Sep 17 00:00:00 2001 From: Jitse Niesen Date: Sat, 1 Apr 2023 10:28:42 +0100 Subject: [PATCH 3/3] Rename infofile to info_file Suggestion from code review. Co-authored-by: Carlos Cordoba --- spyder_notebook/utils/servermanager.py | 16 ++++++++-------- spyder_notebook/widgets/tests/test_serverinfo.py | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/spyder_notebook/utils/servermanager.py b/spyder_notebook/utils/servermanager.py index 4af5a254..9b4185e5 100644 --- a/spyder_notebook/utils/servermanager.py +++ b/spyder_notebook/utils/servermanager.py @@ -54,7 +54,7 @@ class ServerProcess: This is a data class. """ - def __init__(self, process, notebook_dir, interpreter, infofile, + def __init__(self, process, notebook_dir, interpreter, info_file, starttime=None, state=ServerState.STARTING, server_info=None, output=''): """ @@ -68,7 +68,7 @@ def __init__(self, process, notebook_dir, interpreter, infofile, Directory from which the server can render notebooks. interpreter : str File name of Python interpreter used to render notebooks. - infofile : str + info_file : str Name of JSON file in jupyter_runtime_dir() with connection information for the server. starttime : datetime or None, optional @@ -77,7 +77,7 @@ def __init__(self, process, notebook_dir, interpreter, infofile, state : ServerState, optional State of the server process. The default is ServerState.STARTING. server_info : dict or None, optional - If set, this is a dict with the information in infofile. It has + If set, this is a dict with the information in info_file. It has keys like 'url' and 'token'. The default is None. output : str Output of the server process from stdout and stderr. The default @@ -86,7 +86,7 @@ def __init__(self, process, notebook_dir, interpreter, infofile, self.process = process self.notebook_dir = notebook_dir self.interpreter = interpreter - self.infofile = infofile + self.info_file = info_file self.starttime = starttime or datetime.datetime.now() self.state = state self.server_info = server_info @@ -203,9 +203,9 @@ def start_server(self, filename, interpreter): serverscript = osp.normpath(serverscript) my_pid = os.getpid() server_index = len(self.servers) + 1 - infofile = f'spynbserver-{my_pid}-{server_index}.json' + info_file = f'spynbserver-{my_pid}-{server_index}.json' arguments = [serverscript, '--no-browser', - f'--info-file={infofile}', + f'--info-file={info_file}', f'--notebook-dir={nbdir}', '--NotebookApp.password=', f'--KernelSpecManager.kernel_spec_class={KERNELSPEC}'] @@ -220,7 +220,7 @@ def start_server(self, filename, interpreter): server_process = ServerProcess( process, notebook_dir=nbdir, interpreter=interpreter, - infofile=infofile) + info_file=info_file) process.setProcessChannelMode(QProcess.MergedChannels) process.readyReadStandardOutput.connect( lambda: self.read_server_output(server_process)) @@ -260,7 +260,7 @@ def _check_server_started(self, server_process): return runtime_dir = jupyter_runtime_dir() - filename = osp.join(runtime_dir, server_process.infofile) + filename = osp.join(runtime_dir, server_process.info_file) try: with open(filename, encoding='utf-8') as f: diff --git a/spyder_notebook/widgets/tests/test_serverinfo.py b/spyder_notebook/widgets/tests/test_serverinfo.py index ca2a8c3b..f4dfd3ec 100755 --- a/spyder_notebook/widgets/tests/test_serverinfo.py +++ b/spyder_notebook/widgets/tests/test_serverinfo.py @@ -30,12 +30,12 @@ def dialog(qtbot): """Construct and return dialog window for testing.""" servers = [ServerProcess(FakeProcess(42), '/my/home/dir', interpreter='/ham/interpreter', - infofile='info1.json', + info_file='info1.json', state=ServerState.RUNNING, output='Nicely humming along...\n'), ServerProcess(FakeProcess(404), '/some/other/dir', interpreter='/spam/interpreter', - infofile='info2.json', + info_file='info2.json', state=ServerState.FINISHED, output='Terminated for some reason...\n')] res = ServerInfoDialog(servers)