From 4b38296ef833a721e485afc68107517840d3b9a5 Mon Sep 17 00:00:00 2001 From: Jitse Niesen Date: Thu, 9 Jan 2020 14:24:04 +0000 Subject: [PATCH 1/3] Check status code when retrieving list of sessions from server --- spyder_notebook/widgets/client.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/spyder_notebook/widgets/client.py b/spyder_notebook/widgets/client.py index 1801ab97..283e5f1d 100644 --- a/spyder_notebook/widgets/client.py +++ b/spyder_notebook/widgets/client.py @@ -34,7 +34,6 @@ # Local imports from ..widgets.dom import DOMWidget - # ----------------------------------------------------------------------------- # Templates # ----------------------------------------------------------------------------- @@ -229,8 +228,14 @@ def get_kernel_id(self): Return a str with the kernel id or None. """ sessions_url = self.get_session_url() - sessions_req = requests.get(sessions_url).content.decode() - sessions = json.loads(sessions_req) + sessions_response = requests.get(sessions_url) + sessions = json.loads(sessions_response.content.decode()) + if sessions_response.status_code != requests.codes.ok: + msg = _('Spyder could not get a list of sessions ' + 'from the Jupyter Notebook server. ' + 'Message: {}').format(sessions.get('message')) + QMessageBox.warning(self, _('Server error'), msg) + return None if os.name == 'nt': path = self.path.replace('\\', '/') From 6403ab2b652f7025a34200623cc7cafe55c702ec Mon Sep 17 00:00:00 2001 From: Jitse Niesen Date: Mon, 13 Jan 2020 14:30:31 +0000 Subject: [PATCH 2/3] Add test, ensure client is not destroyed too early in tests We need to keep a reference to the plugin in the tests because otherwise the plugin and the client (which is a Qt child of the plugin) gets destroyed. --- spyder_notebook/widgets/tests/test_client.py | 38 +++++++++++++++----- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/spyder_notebook/widgets/tests/test_client.py b/spyder_notebook/widgets/tests/test_client.py index fd980024..d8a5e777 100644 --- a/spyder_notebook/widgets/tests/test_client.py +++ b/spyder_notebook/widgets/tests/test_client.py @@ -8,6 +8,7 @@ # Third-party imports import pytest from qtpy.QtWidgets import QWidget +import requests # Local imports from spyder_notebook.widgets.client import NotebookClient @@ -17,38 +18,59 @@ class MockPlugin(QWidget): def get_plugin_actions(self): return [] - @pytest.fixture -def client(qtbot): - """Construct a NotebookClient for use in tests.""" +def plugin(qtbot): + """ + Construct mock plugin with NotebookClient for use in tests. + + Use `plugin.client` to access the client. + """ plugin = MockPlugin() + qtbot.addWidget(plugin) client = NotebookClient(plugin, '/path/notebooks/ham.ipynb') + plugin.client = client server_info = {'notebook_dir': '/path/notebooks', 'url': 'fake_url', 'token': 'fake_token'} client.register(server_info) - return client + return plugin -def test_notebookclient_get_kernel_id(client, mocker): +def test_notebookclient_get_kernel_id(plugin, mocker): """Basic unit test for NotebookClient.get_kernel_id().""" response = mocker.Mock() content = b'[{"kernel": {"id": "42"}, "notebook": {"path": "ham.ipynb"}}]' response.content = content + response.status_code = requests.codes.ok mocker.patch('requests.get', return_value=response) - kernel_id = client.get_kernel_id() + kernel_id = plugin.client.get_kernel_id() assert kernel_id == '42' -def test_notebookclient_get_kernel_id_with_fields_missing(client, mocker): +def test_notebookclient_get_kernel_id_with_fields_missing(plugin, mocker): """Test NotebookClient.get_kernel_id() if response has fields missing.""" response = mocker.Mock() content = (b'[{"kernel": {"id": "1"}, "notebook": {"spam": "eggs"}},' b' {"kernel": {"id": "2"}},' b' {"kernel": {"id": "3"}, "notebook": {"path": "ham.ipynb"}}]') response.content = content + response.status_code = requests.codes.ok mocker.patch('requests.get', return_value=response) - kernel_id = client.get_kernel_id() + kernel_id = plugin.client.get_kernel_id() assert kernel_id == '3' + + +def test_notebookclient_get_kernel_id_with_error_status(plugin, mocker): + """Test NotebookClient.get_kernel_id() when response has error status.""" + response = mocker.Mock() + content = b'{"message": "error"}' + response.content = content + response.status_code = requests.codes.forbidden + mocker.patch('requests.get', return_value=response) + MockMessageBox = mocker.patch('spyder_notebook.widgets.client.QMessageBox') + + plugin.client.get_kernel_id() + + MockMessageBox.warning.assert_called() From a2fc45abf25316ffce59e4015d9d678aa0c58fef Mon Sep 17 00:00:00 2001 From: Jitse Niesen Date: Tue, 14 Jan 2020 14:46:51 +0000 Subject: [PATCH 3/3] Handle requests exceptions in .get_kernel_id() --- spyder_notebook/widgets/client.py | 13 +++++++++++-- spyder_notebook/widgets/tests/test_client.py | 11 +++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/spyder_notebook/widgets/client.py b/spyder_notebook/widgets/client.py index 283e5f1d..5d7f6e54 100644 --- a/spyder_notebook/widgets/client.py +++ b/spyder_notebook/widgets/client.py @@ -225,10 +225,19 @@ def get_kernel_id(self): """ Get the kernel id of the client. - Return a str with the kernel id or None. + Return a str with the kernel id or None. On error, display a dialog + box and return None. """ sessions_url = self.get_session_url() - sessions_response = requests.get(sessions_url) + try: + sessions_response = requests.get(sessions_url) + except requests.exceptions.RequestException as exception: + msg = _('Spyder could not get a list of sessions ' + 'from the Jupyter Notebook server. ' + 'Message: {}').format(exception) + QMessageBox.warning(self, _('Server error'), msg) + return None + sessions = json.loads(sessions_response.content.decode()) if sessions_response.status_code != requests.codes.ok: msg = _('Spyder could not get a list of sessions ' diff --git a/spyder_notebook/widgets/tests/test_client.py b/spyder_notebook/widgets/tests/test_client.py index d8a5e777..8b26e190 100644 --- a/spyder_notebook/widgets/tests/test_client.py +++ b/spyder_notebook/widgets/tests/test_client.py @@ -74,3 +74,14 @@ def test_notebookclient_get_kernel_id_with_error_status(plugin, mocker): plugin.client.get_kernel_id() MockMessageBox.warning.assert_called() + + +def test_notebookclient_get_kernel_id_with_exception(plugin, mocker): + """Test NotebookClient.get_kernel_id() when request raises an exception.""" + exception = requests.exceptions.ProxyError('kaboom') + mocker.patch('requests.get', side_effect=exception) + MockMessageBox = mocker.patch('spyder_notebook.widgets.client.QMessageBox') + + plugin.client.get_kernel_id() + + MockMessageBox.warning.assert_called()