From 96183a60a0e4b1b046f54135589954cbc5eb452a Mon Sep 17 00:00:00 2001 From: Min RK Date: Sun, 2 Nov 2014 11:08:47 -0800 Subject: [PATCH 1/3] create new terminals with POST /api/terminals instead of GET terminals/new to be consistent with creating new notebooks. We had to stop using GET notebooks/new because browsers would create new notebooks when making preview thumbnails for commonly visited pages, etc. I assume the same issue would apply to terminals --- IPython/html/base/handlers.py | 4 +++ IPython/html/static/tree/js/terminallist.js | 27 ++++++++++++++++++--- IPython/html/terminal/__init__.py | 7 +++--- IPython/html/terminal/api_handlers.py | 15 +++++++++--- IPython/html/terminal/handlers.py | 7 ------ 5 files changed, 42 insertions(+), 18 deletions(-) diff --git a/IPython/html/base/handlers.py b/IPython/html/base/handlers.py index c64ebd7c5..ca8c0fc7e 100644 --- a/IPython/html/base/handlers.py +++ b/IPython/html/base/handlers.py @@ -157,6 +157,10 @@ class IPythonHandler(AuthenticatedHandler): def session_manager(self): return self.settings['session_manager'] + @property + def terminal_manager(self): + return self.settings['terminal_manager'] + @property def kernel_spec_manager(self): return self.settings['kernel_spec_manager'] diff --git a/IPython/html/static/tree/js/terminallist.js b/IPython/html/static/tree/js/terminallist.js index c7ef391e2..625b6d06d 100644 --- a/IPython/html/static/tree/js/terminallist.js +++ b/IPython/html/static/tree/js/terminallist.js @@ -39,11 +39,30 @@ define([ $('#new_terminal').click($.proxy(this.new_terminal, this)); }; - TerminalList.prototype.new_terminal = function() { - var url = utils.url_join_encode(this.base_url, 'terminals/new'); - window.open(url, '_blank'); + TerminalList.prototype.new_terminal = function () { + var settings = { + type : "POST", + cache : false, + async : false, + success : function (data, status, xhr) { + var name = data.name; + window.open( + utils.url_join_encode( + this.base_url, + 'terminals', + name), + '_blank' + ); + }, + error : utils.log_ajax_error, + }; + var url = utils.url_join_encode( + this.base_url, + 'api/terminals' + ); + $.ajax(url, settings); }; - + TerminalList.prototype.load_terminals = function() { var that = this; var url = utils.url_join_encode(this.base_url, 'api/terminals'); diff --git a/IPython/html/terminal/__init__.py b/IPython/html/terminal/__init__.py index e4e33a29a..d8d9945b1 100644 --- a/IPython/html/terminal/__init__.py +++ b/IPython/html/terminal/__init__.py @@ -1,18 +1,17 @@ import os from terminado import NamedTermManager from IPython.html.utils import url_path_join as ujoin -from .handlers import TerminalHandler, NewTerminalHandler, TermSocket +from .handlers import TerminalHandler, TermSocket from . import api_handlers def initialize(webapp): shell = os.environ.get('SHELL', 'sh') - webapp.terminal_manager = NamedTermManager(shell_command=[shell]) + terminal_manager = webapp.settings['terminal_manager'] = NamedTermManager(shell_command=[shell]) base_url = webapp.settings['base_url'] handlers = [ - (ujoin(base_url, "/terminals/new"), NewTerminalHandler), (ujoin(base_url, r"/terminals/(\w+)"), TerminalHandler), (ujoin(base_url, r"/terminals/websocket/(\w+)"), TermSocket, - {'term_manager': webapp.terminal_manager}), + {'term_manager': terminal_manager}), (ujoin(base_url, r"/api/terminals"), api_handlers.TerminalRootHandler), (ujoin(base_url, r"/api/terminals/(\w+)"), api_handlers.TerminalHandler), ] diff --git a/IPython/html/terminal/api_handlers.py b/IPython/html/terminal/api_handlers.py index 90fcd22d0..4c7600cf2 100644 --- a/IPython/html/terminal/api_handlers.py +++ b/IPython/html/terminal/api_handlers.py @@ -1,22 +1,31 @@ import json from tornado import web from ..base.handlers import IPythonHandler, json_errors +from ..utils import url_path_join class TerminalRootHandler(IPythonHandler): @web.authenticated @json_errors def get(self): - tm = self.application.terminal_manager + tm = self.terminal_manager terms = [{'name': name} for name in tm.terminals] self.finish(json.dumps(terms)) + @web.authenticated + @json_errors + def post(self): + """POST /terminals creates a new terminal and redirects to it""" + name, _ = self.terminal_manager.new_named_terminal() + self.finish(json.dumps({'name': name})) + + class TerminalHandler(IPythonHandler): SUPPORTED_METHODS = ('GET', 'DELETE') @web.authenticated @json_errors def get(self, name): - tm = self.application.terminal_manager + tm = self.terminal_manager if name in tm.terminals: self.finish(json.dumps({'name': name})) else: @@ -25,7 +34,7 @@ class TerminalHandler(IPythonHandler): @web.authenticated @json_errors def delete(self, name): - tm = self.application.terminal_manager + tm = self.terminal_manager if name in tm.terminals: tm.kill(name) # XXX: Should this wait for terminal to finish before returning? diff --git a/IPython/html/terminal/handlers.py b/IPython/html/terminal/handlers.py index 20e0fa7de..404cf41c6 100644 --- a/IPython/html/terminal/handlers.py +++ b/IPython/html/terminal/handlers.py @@ -16,13 +16,6 @@ class TerminalHandler(IPythonHandler): self.write(self.render_template('terminal.html', ws_path="terminals/websocket/%s" % term_name)) -class NewTerminalHandler(IPythonHandler): - """Redirect to a new terminal.""" - @web.authenticated - def get(self): - name, _ = self.application.terminal_manager.new_named_terminal() - self.redirect(name, permanent=False) - class TermSocket(terminado.TermSocket, IPythonHandler): def get(self, *args, **kwargs): if not self.get_current_user(): From 8da4e89e3ae3ef4aaf8adf0268814000adb32d2e Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Tue, 4 Nov 2014 09:53:17 -0800 Subject: [PATCH 2/3] Make a window immediately, and set its location on response Avoids the need for async:false --- IPython/html/static/tree/js/terminallist.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/IPython/html/static/tree/js/terminallist.js b/IPython/html/static/tree/js/terminallist.js index 625b6d06d..720883dea 100644 --- a/IPython/html/static/tree/js/terminallist.js +++ b/IPython/html/static/tree/js/terminallist.js @@ -40,19 +40,14 @@ define([ }; TerminalList.prototype.new_terminal = function () { + var w = window.open(); + var base_url = this.base_url; var settings = { type : "POST", - cache : false, - async : false, + dataType: "json", success : function (data, status, xhr) { var name = data.name; - window.open( - utils.url_join_encode( - this.base_url, - 'terminals', - name), - '_blank' - ); + w.location = utils.url_join_encode(base_url, 'terminals', name); }, error : utils.log_ajax_error, }; From 538fcbc02559c175af482da3a07c070998be6c04 Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Tue, 4 Nov 2014 10:44:08 -0800 Subject: [PATCH 3/3] Close new window on error creating terminal --- IPython/html/static/tree/js/terminallist.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/IPython/html/static/tree/js/terminallist.js b/IPython/html/static/tree/js/terminallist.js index 720883dea..9ccf7c6da 100644 --- a/IPython/html/static/tree/js/terminallist.js +++ b/IPython/html/static/tree/js/terminallist.js @@ -49,7 +49,10 @@ define([ var name = data.name; w.location = utils.url_join_encode(base_url, 'terminals', name); }, - error : utils.log_ajax_error, + error : function(jqXHR, status, error){ + w.close(); + utils.log_ajax_error(jqXHR, status, error); + }, }; var url = utils.url_join_encode( this.base_url,