changes after session manager code review

IPython/html/base/handlers.py

@@ -28,6 +28,7 b' import os'
 import stat
 import sys
 import threading
+import traceback
 
 from tornado import web
 from tornado import websocket
@@ -248,6 +249,17 b' 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 b' 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 b' 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()

IPython/html/services/sessions/handlers.py

@@ -16,12 +16,11 b' 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 b' 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 b' class SessionHandler(IPythonHandler):'
 _session_id_regex = r"(?P<session_id>\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)
 ]
 

IPython/html/services/sessions/sessionmanager.py

@@ -42,7 +42,7 b' 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 b' 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 b' 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 b' 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 b' 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):

IPython/html/services/sessions/tests/test_sessionmanager.py

@@ -15,16 +15,16 b' 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 b' 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 b' 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
 

IPython/html/services/sessions/tests/test_sessions_api.py

@@ -4,10 +4,9 b''
 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 b' 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 b' 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 b' 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

IPython/html/static/services/sessions/js/session.js

@@ -11,33 +11,39 b''
 
 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;