From c4a6408b9647c86bcb8058c8f016633165172994 2013-10-17 21:09:12 From: Zachary Sailer Date: 2013-10-17 21:09:12 Subject: [PATCH] changes after session manager code review --- diff --git a/IPython/html/base/handlers.py b/IPython/html/base/handlers.py index 11d8bde..cd4e8d4 100644 --- a/IPython/html/base/handlers.py +++ b/IPython/html/base/handlers.py @@ -28,6 +28,7 @@ import os import stat import sys import threading +import traceback from tornado import web from tornado import websocket @@ -248,6 +249,17 @@ class IPythonHandler(AuthenticatedHandler): use_less=self.use_less, ) + def get_json_body(self): + """Return the body of the request as JSON data.""" + if not self.request.body: + return None + # Do we need to call body.decode('utf-8') here? + body = self.request.body.strip().decode(u'utf-8') + try: + model = json.loads(body) + except: + raise web.HTTPError(400, u'Invalid JSON in body of request') + return model class AuthenticatedFileHandler(IPythonHandler, web.StaticFileHandler): """static files should only be accessible when logged in""" @@ -282,7 +294,8 @@ def json_errors(method): status = 400 message = u"Unknown server error" self.set_status(status) - reply = dict(message=message) + tb_text = ''.join(traceback.format_exception(t, value, tb)) + reply = dict(message=message, traceback=tb_text) self.finish(json.dumps(reply, default=date_default)) else: return result @@ -381,6 +394,12 @@ class FileFindHandler(web.StaticFileHandler): if if_since >= modified: self.set_status(304) return + + if os.path.splitext(path)[1] == '.ipynb': + raise HTTPError(404, 'HAHA') + name = os.path.splitext(os.path.split(path))[0] + self.set_header('Content-Type', 'application/json') + self.set_header('Content-Disposition','attachment; filename="%s.ipynb"' % name) with open(abspath, "rb") as file: data = file.read() diff --git a/IPython/html/services/sessions/handlers.py b/IPython/html/services/sessions/handlers.py index 68d93f7..4db1004 100644 --- a/IPython/html/services/sessions/handlers.py +++ b/IPython/html/services/sessions/handlers.py @@ -16,12 +16,11 @@ Authors: # Imports #----------------------------------------------------------------------------- -from tornado import web - -from zmq.utils import jsonapi +import json +from tornado import web from IPython.utils.jsonutil import date_default -from ...base.handlers import IPythonHandler +from ...base.handlers import IPythonHandler, json_errors #----------------------------------------------------------------------------- # Session web service handlers @@ -31,63 +30,76 @@ from ...base.handlers import IPythonHandler class SessionRootHandler(IPythonHandler): @web.authenticated + @json_errors def get(self): # Return a list of running sessions sm = self.session_manager - nbm = self.notebook_manager - km = self.kernel_manager sessions = sm.list_sessions() - self.finish(jsonapi.dumps(sessions)) + self.finish(json.dumps(sessions, default=date_default)) @web.authenticated + @json_errors def post(self): # Creates a new session #(unless a session already exists for the named nb) sm = self.session_manager nbm = self.notebook_manager km = self.kernel_manager - notebook_path = self.get_argument('notebook_path', default=None) - name, path = nbm.named_notebook_path(notebook_path) + model = self.get_json_body() + if model is None: + raise HTTPError(400, "No JSON data provided") + try: + name = model['notebook']['name'] + except KeyError: + raise HTTPError(400, "Missing field in JSON data: name") + try: + path = model['notebook']['path'] + except KeyError: + raise HTTPError(400, "Missing field in JSON data: path") # Check to see if session exists if sm.session_exists(name=name, path=path): model = sm.get_session(name=name, path=path) - kernel_id = model['kernel']['id'] - km.start_kernel(kernel_id, cwd=nbm.notebook_dir) else: - session_id = sm.get_session_id() - sm.save_session(session_id=session_id, name=name, path=path) kernel_id = km.start_kernel(cwd=nbm.notebook_dir) - kernel = km.kernel_model(kernel_id, self.ws_url) - sm.update_session(session_id, kernel=kernel_id) - model = sm.get_session(id=session_id) - self.set_header('Location', '{0}kernels/{1}'.format(self.base_kernel_url, kernel_id)) - self.finish(jsonapi.dumps(model)) + model = sm.create_session(name=name, path=path, kernel_id=kernel_id, ws_url=self.ws_url) + self.set_header('Location', '{0}/api/sessions/{1}'.format(self.base_project_url, model['id'])) + self.finish(json.dumps(model, default=date_default)) class SessionHandler(IPythonHandler): SUPPORTED_METHODS = ('GET', 'PATCH', 'DELETE') @web.authenticated + @json_errors def get(self, session_id): # Returns the JSON model for a single session sm = self.session_manager model = sm.get_session(id=session_id) - self.finish(jsonapi.dumps(model)) + self.finish(json.dumps(model, default=date_default)) @web.authenticated + @json_errors def patch(self, session_id): # Currently, this handler is strictly for renaming notebooks sm = self.session_manager nbm = self.notebook_manager km = self.kernel_manager - data = self.request.body - data = jsonapi.loads(self.request.body) - name, path = nbm.named_notebook_path(data['notebook_path']) - sm.update_session(session_id, name=name) + model = self.get_json_body() + if model is None: + raise HTTPError(400, "No JSON data provided") + changes = {} + if 'notebook' in model: + notebook = model['notebook'] + if 'name' in notebook: + changes['name'] = notebook['name'] + if 'path' in notebook: + changes['path'] = notebook['path'] + sm.update_session(session_id, **changes) model = sm.get_session(id=session_id) - self.finish(jsonapi.dumps(model)) + self.finish(json.dumps(model, default=date_default)) @web.authenticated + @json_errors def delete(self, session_id): # Deletes the session with given session_id sm = self.session_manager @@ -107,9 +119,7 @@ class SessionHandler(IPythonHandler): _session_id_regex = r"(?P\w+-\w+-\w+-\w+-\w+)" default_handlers = [ - (r"api/sessions/%s/" % _session_id_regex, SessionHandler), (r"api/sessions/%s" % _session_id_regex, SessionHandler), - (r"api/sessions/", SessionRootHandler), (r"api/sessions", SessionRootHandler) ] diff --git a/IPython/html/services/sessions/sessionmanager.py b/IPython/html/services/sessions/sessionmanager.py index 2f7a3e1..b85f0a7 100644 --- a/IPython/html/services/sessions/sessionmanager.py +++ b/IPython/html/services/sessions/sessionmanager.py @@ -42,7 +42,7 @@ class SessionManager(LoggingConfigurable): if self._cursor is None: self._cursor = self.connection.cursor() self._cursor.execute("""CREATE TABLE session - (id, name, path, kernel)""") + (id, name, path, kernel_id, ws_url)""") return self._cursor @property @@ -70,8 +70,15 @@ class SessionManager(LoggingConfigurable): "Create a uuid for a new session" return unicode(uuid.uuid4()) - def save_session(self, session_id, name=None, path=None, kernel=None): - """ Given a session_id (and any other of the arguments), this method + def create_session(self, name=None, path=None, kernel_id=None, ws_url=None): + """Creates a session and returns its model""" + session_id = self.get_session_id() + return self.save_session(session_id, name=name, path=path, kernel_id=kernel_id, ws_url=ws_url) + + def save_session(self, session_id, name=None, path=None, kernel_id=None, ws_url=None): + """Saves the items for the session with the given session_id + + Given a session_id (and any other of the arguments), this method creates a row in the sqlite session database that holds the information for a session. @@ -83,22 +90,32 @@ class SessionManager(LoggingConfigurable): the .ipynb notebook name that started the session path : str the path to the named notebook - kernel : str + kernel_id : str a uuid for the kernel associated with this session + ws_url : str + the websocket url + + Returns + ------- + model : dict + a dictionary of the session model """ self.cursor.execute("""INSERT INTO session VALUES - (?,?,?,?)""", (session_id, name, path, kernel)) + (?,?,?,?,?)""", (session_id, name, path, kernel_id, ws_url)) self.connection.commit() + return self.get_session(id=session_id) def get_session(self, **kwargs): - """ Takes a keyword argument and searches for the value in the session + """Returns the model for a particular session. + + Takes a keyword argument and searches for the value in the session database, then returns the rest of the session's info. Parameters ---------- **kwargs : keyword argument must be given one of the keywords and values from the session database - (i.e. session_id, name, path, kernel) + (i.e. session_id, name, path, kernel_id, ws_url) Returns ------- @@ -120,7 +137,9 @@ class SessionManager(LoggingConfigurable): return model def update_session(self, session_id, **kwargs): - """Updates the values in the session with the given session_id + """Updates the values in the session database. + + Changes the values of the session with the given session_id with the values from the keyword arguments. Parameters @@ -132,20 +151,20 @@ class SessionManager(LoggingConfigurable): and the value replaces the current value in the session with session_id. """ - column = kwargs.keys()[0] # uses only the first kwarg that is entered - value = kwargs.values()[0] - try: - self.cursor.execute("UPDATE session SET %s=? WHERE id=?" %column, (value, session_id)) - self.connection.commit() - except sqlite3.OperationalError: - raise TraitError("No session exists with ID: %s" %session_id) + column = kwargs.keys() # uses only the first kwarg that is entered + value = kwargs.values() + for kwarg in kwargs: + try: + self.cursor.execute("UPDATE session SET %s=? WHERE id=?" %kwarg, (kwargs[kwarg], session_id)) + self.connection.commit() + except sqlite3.OperationalError: + raise TraitError("No session exists with ID: %s" %session_id) def reply_to_dictionary_model(self, reply): """Takes sqlite database session row and turns it into a dictionary""" model = {'id': reply['id'], - 'name' : reply['name'], - 'path' : reply['path'], - 'kernel' : {'id':reply['kernel'], 'ws_url': ''}} + 'notebook': {'name': reply['name'], 'path': reply['path']}, + 'kernel': {'id': reply['kernel_id'], 'ws_url': reply['ws_url']}} return model def list_sessions(self): diff --git a/IPython/html/services/sessions/tests/test_sessionmanager.py b/IPython/html/services/sessions/tests/test_sessionmanager.py index 005d26a..56bb628 100644 --- a/IPython/html/services/sessions/tests/test_sessionmanager.py +++ b/IPython/html/services/sessions/tests/test_sessionmanager.py @@ -15,16 +15,16 @@ class TestSessionManager(TestCase): def test_get_session(self): sm = SessionManager() session_id = sm.get_session_id() - sm.save_session(session_id=session_id, name='test.ipynb', path='/path/to/', kernel='5678') + sm.save_session(session_id=session_id, name='test.ipynb', path='/path/to/', kernel_id='5678', ws_url='ws_url') model = sm.get_session(id=session_id) - expected = {'id':session_id, 'name':u'test.ipynb', 'path': u'/path/to/', 'kernel':{'id':u'5678', 'ws_url': u''}} + expected = {'id':session_id, 'notebook':{'name':u'test.ipynb', 'path': u'/path/to/'}, 'kernel':{'id':u'5678', 'ws_url':u'ws_url'}} self.assertEqual(model, expected) def test_bad_get_session(self): # Should raise error if a bad key is passed to the database. sm = SessionManager() session_id = sm.get_session_id() - sm.save_session(session_id=session_id, name='test.ipynb', path='/path/to/', kernel='5678') + sm.save_session(session_id=session_id, name='test.ipynb', path='/path/to/', kernel_id='5678', ws_url='ws_url') self.assertRaises(TraitError, sm.get_session, bad_id=session_id) # Bad keyword def test_list_sessions(self): @@ -32,33 +32,33 @@ class TestSessionManager(TestCase): session_id1 = sm.get_session_id() session_id2 = sm.get_session_id() session_id3 = sm.get_session_id() - sm.save_session(session_id=session_id1, name='test1.ipynb', path='/path/to/1/', kernel='5678') - sm.save_session(session_id=session_id2, name='test2.ipynb', path='/path/to/2/', kernel='5678') - sm.save_session(session_id=session_id3, name='test3.ipynb', path='/path/to/3/', kernel='5678') + sm.save_session(session_id=session_id1, name='test1.ipynb', path='/path/to/1/', kernel_id='5678', ws_url='ws_url') + sm.save_session(session_id=session_id2, name='test2.ipynb', path='/path/to/2/', kernel_id='5678', ws_url='ws_url') + sm.save_session(session_id=session_id3, name='test3.ipynb', path='/path/to/3/', kernel_id='5678', ws_url='ws_url') sessions = sm.list_sessions() - expected = [{'id':session_id1, 'name':u'test1.ipynb', - 'path': u'/path/to/1/', 'kernel':{'id':u'5678', 'ws_url': u''}}, - {'id':session_id2, 'name':u'test2.ipynb', - 'path': u'/path/to/2/', 'kernel':{'id':u'5678', 'ws_url': u''}}, - {'id':session_id3, 'name':u'test3.ipynb', - 'path': u'/path/to/3/', 'kernel':{'id':u'5678', 'ws_url': u''}}] + expected = [{'id':session_id1, 'notebook':{'name':u'test1.ipynb', + 'path': u'/path/to/1/'}, 'kernel':{'id':u'5678', 'ws_url': 'ws_url'}}, + {'id':session_id2, 'notebook': {'name':u'test2.ipynb', + 'path': u'/path/to/2/'}, 'kernel':{'id':u'5678', 'ws_url': 'ws_url'}}, + {'id':session_id3, 'notebook':{'name':u'test3.ipynb', + 'path': u'/path/to/3/'}, 'kernel':{'id':u'5678', 'ws_url': 'ws_url'}}] self.assertEqual(sessions, expected) def test_update_session(self): sm = SessionManager() session_id = sm.get_session_id() - sm.save_session(session_id=session_id, name='test.ipynb', path='/path/to/', kernel=None) - sm.update_session(session_id, kernel='5678') + sm.save_session(session_id=session_id, name='test.ipynb', path='/path/to/', kernel_id=None, ws_url='ws_url') + sm.update_session(session_id, kernel_id='5678') sm.update_session(session_id, name='new_name.ipynb') model = sm.get_session(id=session_id) - expected = {'id':session_id, 'name':u'new_name.ipynb', 'path': u'/path/to/', 'kernel':{'id':u'5678', 'ws_url': u''}} + expected = {'id':session_id, 'notebook':{'name':u'new_name.ipynb', 'path': u'/path/to/'}, 'kernel':{'id':u'5678', 'ws_url': 'ws_url'}} self.assertEqual(model, expected) def test_bad_update_session(self): # try to update a session with a bad keyword ~ raise error sm = SessionManager() session_id = sm.get_session_id() - sm.save_session(session_id=session_id, name='test.ipynb', path='/path/to/', kernel='5678') + sm.save_session(session_id=session_id, name='test.ipynb', path='/path/to/', kernel_id='5678', ws_url='ws_url') self.assertRaises(TraitError, sm.update_session, session_id=session_id, bad_kw='test.ipynb') # Bad keyword def test_delete_session(self): @@ -66,21 +66,21 @@ class TestSessionManager(TestCase): session_id1 = sm.get_session_id() session_id2 = sm.get_session_id() session_id3 = sm.get_session_id() - sm.save_session(session_id=session_id1, name='test1.ipynb', path='/path/to/1/', kernel='5678') - sm.save_session(session_id=session_id2, name='test2.ipynb', path='/path/to/2/', kernel='5678') - sm.save_session(session_id=session_id3, name='test3.ipynb', path='/path/to/3/', kernel='5678') + sm.save_session(session_id=session_id1, name='test1.ipynb', path='/path/to/1/', kernel_id='5678', ws_url='ws_url') + sm.save_session(session_id=session_id2, name='test2.ipynb', path='/path/to/2/', kernel_id='5678', ws_url='ws_url') + sm.save_session(session_id=session_id3, name='test3.ipynb', path='/path/to/3/', kernel_id='5678', ws_url='ws_url') sm.delete_session(session_id2) sessions = sm.list_sessions() - expected = [{'id':session_id1, 'name':u'test1.ipynb', - 'path': u'/path/to/1/', 'kernel':{'id':u'5678', 'ws_url': u''}}, - {'id':session_id3, 'name':u'test3.ipynb', - 'path': u'/path/to/3/', 'kernel':{'id':u'5678', 'ws_url': u''}}] + expected = [{'id':session_id1, 'notebook':{'name':u'test1.ipynb', + 'path': u'/path/to/1/'}, 'kernel':{'id':u'5678', 'ws_url': 'ws_url'}}, + {'id':session_id3, 'notebook':{'name':u'test3.ipynb', + 'path': u'/path/to/3/'}, 'kernel':{'id':u'5678', 'ws_url': 'ws_url'}}] self.assertEqual(sessions, expected) def test_bad_delete_session(self): # try to delete a session that doesn't exist ~ raise error sm = SessionManager() session_id = sm.get_session_id() - sm.save_session(session_id=session_id, name='test.ipynb', path='/path/to/', kernel='5678') + sm.save_session(session_id=session_id, name='test.ipynb', path='/path/to/', kernel_id='5678', ws_url='ws_url') self.assertRaises(TraitError, sm.delete_session, session_id='23424') # Bad keyword diff --git a/IPython/html/services/sessions/tests/test_sessions_api.py b/IPython/html/services/sessions/tests/test_sessions_api.py index e195919..3b88be2 100644 --- a/IPython/html/services/sessions/tests/test_sessions_api.py +++ b/IPython/html/services/sessions/tests/test_sessions_api.py @@ -4,10 +4,9 @@ import os import sys import json -from zmq.utils import jsonapi - import requests +from IPython.utils.jsonutil import date_default from IPython.html.utils import url_path_join from IPython.html.tests.launchnotebook import NotebookTestBase @@ -39,12 +38,12 @@ class SessionAPITest(NotebookTestBase): # POST a session url, nb = self.mknb() notebook = nb.json() - param = {'notebook_path': notebook['path'] + notebook['name']} - r = requests.post(self.session_url(), params=param) + model = {'notebook': {'name':notebook['name'], 'path': notebook['path']}} + r = requests.post(self.session_url(), data=json.dumps(model, default=date_default)) data = r.json() assert isinstance(data, dict) - self.assertIn('name', data) - self.assertEqual(data['name'], notebook['name']) + self.assertIn('name', data['notebook']) + self.assertEqual(data['notebook']['name'], notebook['name']) # GET sessions r = requests.get(self.session_url()) @@ -62,8 +61,8 @@ class SessionAPITest(NotebookTestBase): # Create a session url, nb = self.mknb() notebook = nb.json() - param = {'notebook_path': notebook['path'] + notebook['name']} - r = requests.post(self.session_url(), params=param) + model = {'notebook': {'name':notebook['name'], 'path': notebook['path']}} + r = requests.post(self.session_url(), data=json.dumps(model, default=date_default)) session = r.json() # GET a session @@ -73,15 +72,17 @@ class SessionAPITest(NotebookTestBase): self.assertEqual(r.json(), session) # PATCH a session - data = {'notebook_path': 'test.ipynb'} - r = requests.patch(sess_url, data=jsonapi.dumps(data)) + model = {'notebook': {'name':'test.ipynb', 'path': '/'}} + r = requests.patch(sess_url, data=json.dumps(model, default=date_default)) + # Patching the notebook webservice too (just for consistency) requests.patch(self.notebook_url() + '/Untitled0.ipynb', - data=jsonapi.dumps({'name':'test.ipynb'})) + data=json.dumps({'name':'test.ipynb'})) + print r.json() assert isinstance(r.json(), dict) - self.assertIn('name', r.json()) + self.assertIn('name', r.json()['notebook']) self.assertIn('id', r.json()) - self.assertEqual(r.json()['name'], 'test.ipynb') + self.assertEqual(r.json()['notebook']['name'], 'test.ipynb') self.assertEqual(r.json()['id'], session['id']) # DELETE a session diff --git a/IPython/html/static/services/sessions/js/session.js b/IPython/html/static/services/sessions/js/session.js index 328d702..8485719 100644 --- a/IPython/html/static/services/sessions/js/session.js +++ b/IPython/html/static/services/sessions/js/session.js @@ -11,33 +11,39 @@ var IPython = (function (IPython) { - var Session = function(notebook_path, Notebook){ + var Session = function(notebook_name, notebook_path, Notebook){ this.kernel = null; this.kernel_id = null; this.session_id = null; + this.notebook_name = notebook_name; this.notebook_path = notebook_path; this.notebook = Notebook; this._baseProjectUrl = Notebook.baseProjectUrl() }; - Session.prototype.start = function(){ + Session.prototype.start = function() { var that = this - var qs = $.param({notebook_path:this.notebook_path}); - var url = '/api/sessions' + '?' + qs; - $.post(url, - $.proxy(this.start_kernel, that), - 'json' - ); + var notebook = {'notebook':{'name': this.notebook_name, 'path': this.notebook_path}} + var settings = { + processData : false, + cache : false, + type : "POST", + data: JSON.stringify(notebook), + dataType : "json", + }; + var url = this._baseProjectUrl + 'api/sessions'; + $.ajax(url, settings); }; - Session.prototype.notebook_rename = function (notebook_path) { - this.notebook_path = notebook_path; - var name = {'notebook_path': notebook_path} + Session.prototype.notebook_rename = function (name, path) { + this.notebook_name = name; + this.notebook_path = path; + var notebook = {'notebook':{'name':name, 'path': path}}; var settings = { processData : false, cache : false, type : "PATCH", - data: JSON.stringify(name), + data: JSON.stringify(notebook), dataType : "json", }; var url = this._baseProjectUrl + 'api/sessions/' + this.session_id;