From ac50d2e5306a758fed6f2fb3bcdbfd8ebe5995d3 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Wed, 14 Oct 2020 09:53:35 -0700 Subject: [PATCH 1/4] Add support for creating terminals via GET --- notebook/terminal/__init__.py | 3 ++- notebook/terminal/handlers.py | 9 ++++++++ notebook/terminal/terminalmanager.py | 10 ++++++++ notebook/terminal/tests/test_terminals_api.py | 23 ++++++++++++++++++- 4 files changed, 43 insertions(+), 2 deletions(-) diff --git a/notebook/terminal/__init__.py b/notebook/terminal/__init__.py index 00cb7b0fbe..9c01dbeb8e 100644 --- a/notebook/terminal/__init__.py +++ b/notebook/terminal/__init__.py @@ -10,7 +10,7 @@ from ipython_genutils.py3compat import which from notebook.utils import url_path_join as ujoin from .terminalmanager import TerminalManager -from .handlers import TerminalHandler, TermSocket +from .handlers import TerminalHandler, TermSocket, NewTerminalHandler from . import api_handlers @@ -45,6 +45,7 @@ def initialize(nb_app): (ujoin(base_url, r"/terminals/(\w+)"), TerminalHandler), (ujoin(base_url, r"/terminals/websocket/(\w+)"), TermSocket, {'term_manager': terminal_manager}), + (ujoin(base_url, r"/terminals/new/(\w+)"), NewTerminalHandler), (ujoin(base_url, r"/api/terminals"), api_handlers.TerminalRootHandler), (ujoin(base_url, r"/api/terminals/(\w+)"), api_handlers.TerminalHandler), ] diff --git a/notebook/terminal/handlers.py b/notebook/terminal/handlers.py index 94d304559a..c949f20000 100644 --- a/notebook/terminal/handlers.py +++ b/notebook/terminal/handlers.py @@ -18,6 +18,15 @@ def get(self, term_name): ws_path="terminals/websocket/%s" % term_name)) +class NewTerminalHandler(IPythonHandler): + """Render the terminal interface.""" + @web.authenticated + def get(self, term_name): + self.terminal_manager.create_with_name(term_name) + new_path = self.request.path.replace("new/{}".format(term_name), term_name) + self.redirect(new_path) + + class TermSocket(WebSocketMixin, IPythonHandler, terminado.TermSocket): def origin_check(self): diff --git a/notebook/terminal/terminalmanager.py b/notebook/terminal/terminalmanager.py index c1bdecd8db..ed901310ec 100644 --- a/notebook/terminal/terminalmanager.py +++ b/notebook/terminal/terminalmanager.py @@ -44,6 +44,16 @@ def __init__(self, *args, **kwargs): def create(self): """Create a new terminal.""" name, term = self.new_named_terminal() + return self._finish_create(name, term) + + def create_with_name(self, name): + """Create a new terminal.""" + if name in self.terminals: + raise web.HTTPError(409, "A terminal with name '{}' already exists.".format(name)) + term = self.get_terminal(name) + return self._finish_create(name, term) + + def _finish_create(self, name, term): # Monkey-patch last-activity, similar to kernels. Should we need # more functionality per terminal, we can look into possible sub- # classing or containment then. diff --git a/notebook/terminal/tests/test_terminals_api.py b/notebook/terminal/tests/test_terminals_api.py index 430df48165..fcbbf1bdd9 100644 --- a/notebook/terminal/tests/test_terminals_api.py +++ b/notebook/terminal/tests/test_terminals_api.py @@ -54,7 +54,7 @@ def tearDown(self): self.term_api.shutdown(k['name']) def test_no_terminals(self): - # Make sure there are no terminals running at the start + # Make sure there are no terminals are running at the start terminals = self.term_api.list().json() self.assertEqual(terminals, []) @@ -65,6 +65,27 @@ def test_create_terminal(self): self.assertEqual(r.status_code, 200) self.assertIsInstance(term1, dict) + def test_create_terminal_via_get(self): + # Test creation of terminal via GET against terminals/new/ + r = self.term_api._req('GET', 'terminals/new/foo') + self.assertEqual(r.status_code, 200) + + r = self.term_api.get('foo') + foo_term = r.json() + self.assertEqual(r.status_code, 200) + self.assertIsInstance(foo_term, dict) + self.assertEqual(foo_term['name'], 'foo') + + with assert_http_error(409): + self.term_api._req('GET', 'terminals/new/foo') + + r = self.term_api.shutdown('foo') + self.assertEqual(r.status_code, 204) + + # Make sure there are no terminals are running + terminals = self.term_api.list().json() + self.assertEqual(terminals, []) + def test_terminal_root_handler(self): # POST request r = self.term_api.start() From 7778dc3202399161cbc4a61c258cef8ef0229455 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Wed, 14 Oct 2020 14:57:57 -0700 Subject: [PATCH 2/4] Allow for redundant accesses of same endpoint --- notebook/terminal/handlers.py | 11 +++++++++-- notebook/terminal/tests/test_terminals_api.py | 12 ++++++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/notebook/terminal/handlers.py b/notebook/terminal/handlers.py index c949f20000..0bb36df987 100644 --- a/notebook/terminal/handlers.py +++ b/notebook/terminal/handlers.py @@ -3,6 +3,7 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. +import json from tornado import web import terminado from notebook._tz import utcnow @@ -19,11 +20,17 @@ def get(self, term_name): class NewTerminalHandler(IPythonHandler): - """Render the terminal interface.""" + """Renders a new terminal interface using the named argument.""" @web.authenticated def get(self, term_name): - self.terminal_manager.create_with_name(term_name) new_path = self.request.path.replace("new/{}".format(term_name), term_name) + if term_name in self.terminal_manager.terminals: + self.set_header('Location', new_path) + self.set_status(302) + self.finish(json.dumps(self.terminal_manager.get_terminal_model(term_name))) + return + + self.terminal_manager.create_with_name(term_name) self.redirect(new_path) diff --git a/notebook/terminal/tests/test_terminals_api.py b/notebook/terminal/tests/test_terminals_api.py index fcbbf1bdd9..6b808c5c85 100644 --- a/notebook/terminal/tests/test_terminals_api.py +++ b/notebook/terminal/tests/test_terminals_api.py @@ -76,8 +76,16 @@ def test_create_terminal_via_get(self): self.assertIsInstance(foo_term, dict) self.assertEqual(foo_term['name'], 'foo') - with assert_http_error(409): - self.term_api._req('GET', 'terminals/new/foo') + # hit the same endpoint a second time and ensure 302 with Location is returned + r = self.term_api._req('GET', 'terminals/new/foo') + # Access the "interesting" response from the history + self.assertEqual(len(r.history), 1) + r = r.history[0] + foo_term = r.json() + self.assertEqual(r.status_code, 302) + self.assertEqual(r.headers['Location'], self.url_prefix + "terminals/foo") + self.assertIsInstance(foo_term, dict) + self.assertEqual(foo_term['name'], 'foo') r = self.term_api.shutdown('foo') self.assertEqual(r.status_code, 204) From 84ba421e86ba4dc9eba2ce9be35d7f46bb46a171 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Thu, 15 Oct 2020 14:58:19 -0700 Subject: [PATCH 3/4] Create auto-named terminals via /terminals/new --- notebook/terminal/handlers.py | 12 +++++- notebook/terminal/tests/test_terminals_api.py | 41 ++++++++++++++++++- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/notebook/terminal/handlers.py b/notebook/terminal/handlers.py index 0bb36df987..06bd10f554 100644 --- a/notebook/terminal/handlers.py +++ b/notebook/terminal/handlers.py @@ -15,14 +15,22 @@ class TerminalHandler(IPythonHandler): """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)) + if term_name == 'new': + model = self.terminal_manager.create() + term_name = model['name'] + new_path = self.request.path.replace("terminals/new", "terminals/" + term_name) + self.redirect(new_path) + else: + self.write(self.render_template('terminal.html', + ws_path="terminals/websocket/%s" % term_name)) class NewTerminalHandler(IPythonHandler): """Renders a new terminal interface using the named argument.""" @web.authenticated def get(self, term_name): + if term_name == 'new': + raise web.HTTPError(400, "Terminal name 'new' is reserved.") new_path = self.request.path.replace("new/{}".format(term_name), term_name) if term_name in self.terminal_manager.terminals: self.set_header('Location', new_path) diff --git a/notebook/terminal/tests/test_terminals_api.py b/notebook/terminal/tests/test_terminals_api.py index 6b808c5c85..9e0cba82da 100644 --- a/notebook/terminal/tests/test_terminals_api.py +++ b/notebook/terminal/tests/test_terminals_api.py @@ -66,6 +66,41 @@ def test_create_terminal(self): self.assertIsInstance(term1, dict) def test_create_terminal_via_get(self): + # Test creation of terminal via GET against terminals/new/ + r = self.term_api._req('GET', 'terminals/new') + self.assertEqual(r.status_code, 200) + + r = self.term_api.get('1') + term1 = r.json() + self.assertEqual(r.status_code, 200) + self.assertIsInstance(term1, dict) + self.assertEqual(term1['name'], '1') + + # hit the same endpoint a second time and ensure a second named terminal is created + r = self.term_api._req('GET', 'terminals/new') + self.assertEqual(r.status_code, 200) + + r = self.term_api.get('2') + term2 = r.json() + self.assertEqual(r.status_code, 200) + self.assertIsInstance(term2, dict) + self.assertEqual(term2['name'], '2') + + r = self.term_api.shutdown('2') + self.assertEqual(r.status_code, 204) + + # Make sure there is 1 terminal running + terminals = self.term_api.list().json() + self.assertEqual(len(terminals), 1) + + r = self.term_api.shutdown('1') + self.assertEqual(r.status_code, 204) + + # Make sure there are no terminals are running + terminals = self.term_api.list().json() + self.assertEqual(len(terminals), 0) + + def test_create_terminal_with_name(self): # Test creation of terminal via GET against terminals/new/ r = self.term_api._req('GET', 'terminals/new/foo') self.assertEqual(r.status_code, 200) @@ -92,7 +127,11 @@ def test_create_terminal_via_get(self): # Make sure there are no terminals are running terminals = self.term_api.list().json() - self.assertEqual(terminals, []) + self.assertEqual(len(terminals), 0) + + # hit terminals/new/new and ensure that 400 is raised + with assert_http_error(400): + self.term_api._req('GET', 'terminals/new/new') def test_terminal_root_handler(self): # POST request From 639da8d8131245319856db6f8d7b1c3d89c86e5e Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Thu, 15 Oct 2020 15:42:55 -0700 Subject: [PATCH 4/4] Add separate handler for /terminals/new --- notebook/terminal/__init__.py | 5 +++-- notebook/terminal/handlers.py | 20 ++++++++++++-------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/notebook/terminal/__init__.py b/notebook/terminal/__init__.py index 9c01dbeb8e..57e74c9bf4 100644 --- a/notebook/terminal/__init__.py +++ b/notebook/terminal/__init__.py @@ -10,7 +10,7 @@ from ipython_genutils.py3compat import which from notebook.utils import url_path_join as ujoin from .terminalmanager import TerminalManager -from .handlers import TerminalHandler, TermSocket, NewTerminalHandler +from .handlers import TerminalHandler, TermSocket, NewTerminalHandler, NamedTerminalHandler from . import api_handlers @@ -42,10 +42,11 @@ def initialize(nb_app): terminal_manager.log = nb_app.log base_url = nb_app.web_app.settings['base_url'] handlers = [ + (ujoin(base_url, r"/terminals/new"), NamedTerminalHandler), + (ujoin(base_url, r"/terminals/new/(\w+)"), NewTerminalHandler), (ujoin(base_url, r"/terminals/(\w+)"), TerminalHandler), (ujoin(base_url, r"/terminals/websocket/(\w+)"), TermSocket, {'term_manager': terminal_manager}), - (ujoin(base_url, r"/terminals/new/(\w+)"), NewTerminalHandler), (ujoin(base_url, r"/api/terminals"), api_handlers.TerminalRootHandler), (ujoin(base_url, r"/api/terminals/(\w+)"), api_handlers.TerminalHandler), ] diff --git a/notebook/terminal/handlers.py b/notebook/terminal/handlers.py index 06bd10f554..0e026d00ab 100644 --- a/notebook/terminal/handlers.py +++ b/notebook/terminal/handlers.py @@ -15,18 +15,22 @@ class TerminalHandler(IPythonHandler): """Render the terminal interface.""" @web.authenticated def get(self, term_name): - if term_name == 'new': - model = self.terminal_manager.create() - term_name = model['name'] - new_path = self.request.path.replace("terminals/new", "terminals/" + term_name) - self.redirect(new_path) - else: - self.write(self.render_template('terminal.html', + self.write(self.render_template('terminal.html', ws_path="terminals/websocket/%s" % term_name)) +class NamedTerminalHandler(IPythonHandler): + """Creates and renders a named terminal interface.""" + @web.authenticated + def get(self): + model = self.terminal_manager.create() + term_name = model['name'] + new_path = self.request.path.replace("terminals/new", "terminals/" + term_name) + self.redirect(new_path) + + class NewTerminalHandler(IPythonHandler): - """Renders a new terminal interface using the named argument.""" + """Creates and renders a terminal interface using the named argument.""" @web.authenticated def get(self, term_name): if term_name == 'new':