Merge pull request #6026 from takluyver/kernelspec-rest-launching

Kernelspecs in REST API for kernels and sessions

IPython/html/notebookapp.py

@@ -658,7 +658,9 b' class NotebookApp(BaseIPythonApplication):'
         kls = import_item(self.notebook_manager_class)
         self.notebook_manager = kls(parent=self, log=self.log)
         kls = import_item(self.session_manager_class)
-        self.session_manager = kls(parent=self, log=self.log)
+        self.session_manager = kls(parent=self, log=self.log,
+                                   kernel_manager=self.kernel_manager,
+                                   notebook_manager=self.notebook_manager)
         kls = import_item(self.cluster_manager_class)
         self.cluster_manager = kls(parent=self, log=self.log)
         self.cluster_manager.update_profiles()

IPython/html/services/kernels/handlers.py

@@ -27,8 +27,16 b' class MainKernelHandler(IPythonHandler):'
     @web.authenticated
     @json_errors
     def post(self):
+        model = self.get_json_body()
+        if model is None:
+            raise web.HTTPError(400, "No JSON data provided")
+        try:
+            name = model['name']
+        except KeyError:
+            raise web.HTTPError(400, "Missing field in JSON data: name")
+
         km = self.kernel_manager
-        kernel_id = km.start_kernel()
+        kernel_id = km.start_kernel(kernel_name=name)
         model = km.kernel_model(kernel_id)
         location = url_path_join(self.base_url, 'api', 'kernels', kernel_id)
         self.set_header('Location', url_escape(location))

IPython/html/services/kernels/kernelmanager.py

@@ -72,8 +72,8 b' class MappingKernelManager(MultiKernelManager):'
             os_path = os.path.dirname(os_path)
         return os_path
 
-    def start_kernel(self, kernel_id=None, path=None, **kwargs):
-        """Start a kernel for a session an return its kernel_id.
+    def start_kernel(self, kernel_id=None, path=None, kernel_name='python', **kwargs):
+        """Start a kernel for a session and return its kernel_id.
 
         Parameters
         ----------
@@ -84,12 +84,16 b' class MappingKernelManager(MultiKernelManager):'
         path : API path
             The API path (unicode, '/' delimited) for the cwd.
             Will be transformed to an OS path relative to root_dir.
+        kernel_name : str
+            The name identifying which kernel spec to launch. This is ignored if
+            an existing kernel is returned, but it may be checked in the future.
         """
         if kernel_id is None:
             kwargs['extra_arguments'] = self.kernel_argv
             if path is not None:
                 kwargs['cwd'] = self.cwd_for_path(path)
-            kernel_id = super(MappingKernelManager, self).start_kernel(**kwargs)
+            kernel_id = super(MappingKernelManager, self).start_kernel(
+                                            kernel_name=kernel_name, **kwargs)
             self.log.info("Kernel started: %s" % kernel_id)
             self.log.debug("Kernel args: %r" % kwargs)
             # register callback for failed auto-restart
@@ -111,7 +115,8 b' class MappingKernelManager(MultiKernelManager):'
         """Return a dictionary of kernel information described in the
         JSON standard model."""
         self._check_kernel_id(kernel_id)
-        model = {"id":kernel_id}
+        model = {"id":kernel_id,
+                 "name": self._kernels[kernel_id].kernel_name}
         return model
 
     def list_kernels(self):

IPython/html/services/kernels/tests/test_kernels_api.py

@@ -1,6 +1,6 b''
 """Test the kernels service API."""
 
-
+import json
 import requests
 
 from IPython.html.utils import url_path_join
@@ -30,8 +30,9 b' class KernelAPI(object):'
     def get(self, id):
         return self._req('GET', id)
 
-    def start(self):
-        return self._req('POST', '')
+    def start(self, name='python'):
+        body = json.dumps({'name': name})
+        return self._req('POST', '', body)
 
     def shutdown(self, id):
         return self._req('DELETE', id)
@@ -69,6 +70,7 b' class KernelAPITest(NotebookTestBase):'
         self.assertEqual(r.status_code, 200)
         assert isinstance(r.json(), list)
         self.assertEqual(r.json()[0]['id'], kern1['id'])
+        self.assertEqual(r.json()[0]['name'], kern1['name'])
 
         # create another kernel and check that they both are added to the
         # list of kernels from a GET request
@@ -89,6 +91,7 b' class KernelAPITest(NotebookTestBase):'
         self.assertEqual(r.headers['Location'], '/api/kernels/'+kern2['id'])
         rekern = r.json()
         self.assertEqual(rekern['id'], kern2['id'])
+        self.assertEqual(rekern['name'], kern2['name'])
 
     def test_kernel_handler(self):
         # GET kernel with given id

IPython/html/services/sessions/handlers.py

@@ -45,27 +45,28 b' class SessionRootHandler(IPythonHandler):'
         # 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
+
         model = self.get_json_body()
         if model is None:
             raise web.HTTPError(400, "No JSON data provided")
         try:
             name = model['notebook']['name']
         except KeyError:
-            raise web.HTTPError(400, "Missing field in JSON data: name")
+            raise web.HTTPError(400, "Missing field in JSON data: notebook.name")
         try:
             path = model['notebook']['path']
         except KeyError:
-            raise web.HTTPError(400, "Missing field in JSON data: path")
+            raise web.HTTPError(400, "Missing field in JSON data: notebook.path")
+        try:
+            kernel_name = model['kernel']['name']
+        except KeyError:
+            raise web.HTTPError(400, "Missing field in JSON data: kernel.name")
+
         # Check to see if session exists
         if sm.session_exists(name=name, path=path):
             model = sm.get_session(name=name, path=path)
         else:
-            # allow nbm to specify kernels cwd
-            kernel_path = nbm.get_kernel_path(name=name, path=path)
-            kernel_id = km.start_kernel(path=kernel_path)
-            model = sm.create_session(name=name, path=path, kernel_id=kernel_id)
+            model = sm.create_session(name=name, path=path, kernel_name=kernel_name)
         location = url_path_join(self.base_url, 'api', 'sessions', model['id'])
         self.set_header('Location', url_escape(location))
         self.set_status(201)
@@ -108,10 +109,7 b' class SessionHandler(IPythonHandler):'
     def delete(self, session_id):
         # Deletes the session with given session_id
         sm = self.session_manager
-        km = self.kernel_manager
-        session = sm.get_session(session_id=session_id)
         sm.delete_session(session_id)
-        km.shutdown_kernel(session['kernel']['id'])
         self.set_status(204)
         self.finish()
 

IPython/html/services/sessions/sessionmanager.py

@@ -23,12 +23,16 b' from tornado import web'
 
 from IPython.config.configurable import LoggingConfigurable
 from IPython.utils.py3compat import unicode_type
+from IPython.utils.traitlets import Instance
 
 #-----------------------------------------------------------------------------
 # Classes
 #-----------------------------------------------------------------------------
 
 class SessionManager(LoggingConfigurable):
+
+    kernel_manager = Instance('IPython.html.services.kernels.kernelmanager.MappingKernelManager')
+    notebook_manager = Instance('IPython.html.services.notebooks.nbmanager.NotebookManager', args=())
     
     # Session database initialized below
     _cursor = None
@@ -69,10 +73,15 b' class SessionManager(LoggingConfigurable):'
         "Create a uuid for a new session"
         return unicode_type(uuid.uuid4())
 
-    def create_session(self, name=None, path=None, kernel_id=None):
+    def create_session(self, name=None, path=None, kernel_name='python'):
         """Creates a session and returns its model"""
         session_id = self.new_session_id()
-        return self.save_session(session_id, name=name, path=path, kernel_id=kernel_id)
+        # allow nbm to specify kernels cwd
+        kernel_path = self.notebook_manager.get_kernel_path(name=name, path=path)
+        kernel_id = self.kernel_manager.start_kernel(path=kernel_path,
+                                                     kernel_name=kernel_name)
+        return self.save_session(session_id, name=name, path=path,
+                                 kernel_id=kernel_id)
 
     def save_session(self, session_id, name=None, path=None, kernel_id=None):
         """Saves the items for the session with the given session_id
@@ -170,8 +179,7 b' class SessionManager(LoggingConfigurable):'
         query = "UPDATE session SET %s WHERE session_id=?" % (', '.join(sets))
         self.cursor.execute(query, list(kwargs.values()) + [session_id])
 
-    @staticmethod
-    def row_factory(cursor, row):
+    def row_factory(self, cursor, row):
         """Takes sqlite database session row and turns it into a dictionary"""
         row = sqlite3.Row(cursor, row)
         model = {
@@ -180,9 +188,7 b' class SessionManager(LoggingConfigurable):'
                 'name': row['name'],
                 'path': row['path']
             },
-            'kernel': {
-                'id': row['kernel_id'],
-            }
+            'kernel': self.kernel_manager.kernel_model(row['kernel_id'])
         }
         return model
 
@@ -195,5 +201,6 b' class SessionManager(LoggingConfigurable):'
     def delete_session(self, session_id):
         """Deletes the row in the session database with given session_id"""
         # Check that session exists before deleting
-        self.get_session(session_id=session_id)
+        session = self.get_session(session_id=session_id)
+        self.kernel_manager.shutdown_kernel(session['kernel']['id'])
         self.cursor.execute("DELETE FROM session WHERE session_id=?", (session_id,))

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

@@ -5,79 +5,101 b' from unittest import TestCase'
 from tornado import web
 
 from ..sessionmanager import SessionManager
+from IPython.html.services.kernels.kernelmanager import MappingKernelManager
+
+class DummyKernel(object):
+    def __init__(self, kernel_name='python'):
+        self.kernel_name = kernel_name
+
+class DummyMKM(MappingKernelManager):
+    """MappingKernelManager interface that doesn't start kernels, for testing"""
+    def __init__(self, *args, **kwargs):
+        super(DummyMKM, self).__init__(*args, **kwargs)
+        self.id_letters = iter(u'ABCDEFGHIJK')
+
+    def _new_id(self):
+        return next(self.id_letters)
+    
+    def start_kernel(self, kernel_id=None, path=None, kernel_name='python', **kwargs):
+        kernel_id = kernel_id or self._new_id()
+        self._kernels[kernel_id] = DummyKernel(kernel_name=kernel_name)
+        return kernel_id
+
+    def shutdown_kernel(self, kernel_id, now=False):
+        del self._kernels[kernel_id]
 
 class TestSessionManager(TestCase):
     
     def test_get_session(self):
-        sm = SessionManager()
-        session_id = sm.new_session_id()
-        sm.save_session(session_id=session_id, name='test.ipynb', path='/path/to/', kernel_id='5678')
+        sm = SessionManager(kernel_manager=DummyMKM())
+        session_id = sm.create_session(name='test.ipynb', path='/path/to/',
+                                       kernel_name='bar')['id']
         model = sm.get_session(session_id=session_id)
-        expected = {'id':session_id, 'notebook':{'name':u'test.ipynb', 'path': u'/path/to/'}, 'kernel':{'id':u'5678'}}
+        expected = {'id':session_id,
+                    'notebook':{'name':u'test.ipynb', 'path': u'/path/to/'},
+                    'kernel': {'id':u'A', 'name': 'bar'}}
         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.new_session_id()
-        sm.save_session(session_id=session_id, name='test.ipynb', path='/path/to/', kernel_id='5678')
+        sm = SessionManager(kernel_manager=DummyMKM())
+        session_id = sm.create_session(name='test.ipynb', path='/path/to/',
+                                       kernel_name='foo')['id']
         self.assertRaises(TypeError, sm.get_session, bad_id=session_id) # Bad keyword
 
     def test_list_sessions(self):
-        sm = SessionManager()
-        session_id1 = sm.new_session_id()
-        session_id2 = sm.new_session_id()
-        session_id3 = sm.new_session_id()
-        sm.save_session(session_id=session_id1, name='test1.ipynb', path='/path/to/1/', kernel_id='5678')
-        sm.save_session(session_id=session_id2, name='test2.ipynb', path='/path/to/2/', kernel_id='5678')
-        sm.save_session(session_id=session_id3, name='test3.ipynb', path='/path/to/3/', kernel_id='5678')
+        sm = SessionManager(kernel_manager=DummyMKM())
+        sessions = [
+            sm.create_session(name='test1.ipynb', path='/path/to/1/', kernel_name='python'),
+            sm.create_session(name='test2.ipynb', path='/path/to/2/', kernel_name='python'),
+            sm.create_session(name='test3.ipynb', path='/path/to/3/', kernel_name='python'),
+        ]
         sessions = sm.list_sessions()
-        expected = [{'id':session_id1, 'notebook':{'name':u'test1.ipynb', 
-                    'path': u'/path/to/1/'}, 'kernel':{'id':u'5678'}},
-                    {'id':session_id2, 'notebook': {'name':u'test2.ipynb', 
-                    'path': u'/path/to/2/'}, 'kernel':{'id':u'5678'}},
-                    {'id':session_id3, 'notebook':{'name':u'test3.ipynb', 
-                    'path': u'/path/to/3/'}, 'kernel':{'id':u'5678'}}]
+        expected = [{'id':sessions[0]['id'], 'notebook':{'name':u'test1.ipynb', 
+                    'path': u'/path/to/1/'}, 'kernel':{'id':u'A', 'name':'python'}},
+                    {'id':sessions[1]['id'], 'notebook': {'name':u'test2.ipynb', 
+                    'path': u'/path/to/2/'}, 'kernel':{'id':u'B', 'name':'python'}},
+                    {'id':sessions[2]['id'], 'notebook':{'name':u'test3.ipynb', 
+                    'path': u'/path/to/3/'}, 'kernel':{'id':u'C', 'name':'python'}}]
         self.assertEqual(sessions, expected)
 
     def test_update_session(self):
-        sm = SessionManager()
-        session_id = sm.new_session_id()
-        sm.save_session(session_id=session_id, name='test.ipynb', path='/path/to/', kernel_id=None)
-        sm.update_session(session_id, kernel_id='5678')
+        sm = SessionManager(kernel_manager=DummyMKM())
+        session_id = sm.create_session(name='test.ipynb', path='/path/to/',
+                                       kernel_name='julia')['id']
         sm.update_session(session_id, name='new_name.ipynb')
         model = sm.get_session(session_id=session_id)
-        expected = {'id':session_id, 'notebook':{'name':u'new_name.ipynb', 'path': u'/path/to/'}, 'kernel':{'id':u'5678'}}
+        expected = {'id':session_id,
+                    'notebook':{'name':u'new_name.ipynb', 'path': u'/path/to/'},
+                    'kernel':{'id':u'A', 'name':'julia'}}
         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.new_session_id()
-        sm.save_session(session_id=session_id, name='test.ipynb', path='/path/to/', kernel_id='5678')
+        sm = SessionManager(kernel_manager=DummyMKM())
+        session_id = sm.create_session(name='test.ipynb', path='/path/to/',
+                                       kernel_name='ir')['id']
         self.assertRaises(TypeError, sm.update_session, session_id=session_id, bad_kw='test.ipynb') # Bad keyword
 
     def test_delete_session(self):
-        sm = SessionManager()
-        session_id1 = sm.new_session_id()
-        session_id2 = sm.new_session_id()
-        session_id3 = sm.new_session_id()
-        sm.save_session(session_id=session_id1, name='test1.ipynb', path='/path/to/1/', kernel_id='5678')
-        sm.save_session(session_id=session_id2, name='test2.ipynb', path='/path/to/2/', kernel_id='5678')
-        sm.save_session(session_id=session_id3, name='test3.ipynb', path='/path/to/3/', kernel_id='5678')
-        sm.delete_session(session_id2)
-        sessions = sm.list_sessions()
-        expected = [{'id':session_id1, 'notebook':{'name':u'test1.ipynb', 
-                    'path': u'/path/to/1/'}, 'kernel':{'id':u'5678'}},
-                    {'id':session_id3, 'notebook':{'name':u'test3.ipynb', 
-                    'path': u'/path/to/3/'}, 'kernel':{'id':u'5678'}}]
-        self.assertEqual(sessions, expected)
+        sm = SessionManager(kernel_manager=DummyMKM())
+        sessions = [
+            sm.create_session(name='test1.ipynb', path='/path/to/1/', kernel_name='python'),
+            sm.create_session(name='test2.ipynb', path='/path/to/2/', kernel_name='python'),
+            sm.create_session(name='test3.ipynb', path='/path/to/3/', kernel_name='python'),
+        ]
+        sm.delete_session(sessions[1]['id'])
+        new_sessions = sm.list_sessions()
+        expected = [{'id':sessions[0]['id'], 'notebook':{'name':u'test1.ipynb', 
+                    'path': u'/path/to/1/'}, 'kernel':{'id':u'A', 'name':'python'}},
+                    {'id':sessions[2]['id'], 'notebook':{'name':u'test3.ipynb', 
+                    'path': u'/path/to/3/'}, 'kernel':{'id':u'C', 'name':'python'}}]
+        self.assertEqual(new_sessions, expected)
 
     def test_bad_delete_session(self):
         # try to delete a session that doesn't exist ~ raise error
-        sm = SessionManager()
-        session_id = sm.new_session_id()
-        sm.save_session(session_id=session_id, name='test.ipynb', path='/path/to/', kernel_id='5678')
+        sm = SessionManager(kernel_manager=DummyMKM())
+        sm.create_session(name='test.ipynb', path='/path/to/', kernel_name='python')
         self.assertRaises(TypeError, sm.delete_session, bad_kwarg='23424') # Bad keyword
         self.assertRaises(web.HTTPError, sm.delete_session, session_id='23424') # nonexistant
 

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

@@ -37,8 +37,9 b' class SessionAPI(object):'
     def get(self, id):
         return self._req('GET', id)
 
-    def create(self, name, path):
-        body = json.dumps({'notebook': {'name':name, 'path':path}})
+    def create(self, name, path, kernel_name='python'):
+        body = json.dumps({'notebook': {'name':name, 'path':path},
+                           'kernel': {'name': kernel_name}})
         return self._req('POST', '', body)
 
     def modify(self, id, name, path):

IPython/html/static/notebook/js/notebook.js

@@ -59,6 +59,9 b' define(['
         this.keyboard_manager = options.keyboard_manager;
         this.save_widget = options.save_widget;
         this.tooltip = new tooltip.Tooltip(this.events);
+        // default_kernel_name is a temporary measure while we implement proper
+        // kernel selection and delayed start. Do not rely on it.
+        this.default_kernel_name = 'python';
         // TODO: This code smells (and the other `= this` line a couple lines down)
         // We need a better way to deal with circular instance references.
         this.keyboard_manager.notebook = this;
@@ -1495,7 +1498,12 b' define(['
             base_url: this.base_url,
             notebook_path: this.notebook_path,
             notebook_name: this.notebook_name,
+            // For now, create all sessions with the 'python' kernel, which is the
+            // default. Later, the user will be able to select kernels. This is
+            // overridden if KernelManager.kernel_cmd is specified for the server.
+            kernel_name: this.default_kernel_name,
             notebook: this});
+
         this.session.start($.proxy(this._session_started, this));
     };
 

IPython/html/static/services/kernels/js/kernel.js

@@ -15,13 +15,14 b' define(['
      * A Kernel Class to communicate with the Python kernel
      * @Class Kernel
      */
-    var Kernel = function (kernel_service_url, notebook) {
+    var Kernel = function (kernel_service_url, notebook, name) {
         this.events = notebook.events;
         this.kernel_id = null;
         this.shell_channel = null;
         this.iopub_channel = null;
         this.stdin_channel = null;
         this.kernel_service_url = kernel_service_url;
+        this.name = name;
         this.running = false;
         this.username = "username";
         this.session_id = utils.uuid();

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

@@ -15,6 +15,7 b' define(['
         this.notebook = options.notebook;
         this.name = options.notebook_name;
         this.path = options.notebook_path;
+        this.kernel_name = options.kernel_name;
         this.base_url = options.base_url;
     };
     
@@ -24,6 +25,9 b' define(['
             notebook : {
                 name : this.name,
                 path : this.path
+            },
+            kernel : {
+                name : this.kernel_name
             }
         };
         var settings = {
@@ -87,7 +91,7 b' define(['
     Session.prototype._handle_start_success = function (data, status, xhr) {
         this.id = data.id;
         var kernel_service_url = utils.url_path_join(this.base_url, "api/kernels");
-        this.kernel = new kernel.Kernel(kernel_service_url, this.notebook);
+        this.kernel = new kernel.Kernel(kernel_service_url, this.notebook, this.kernel_name);
         this.kernel._kernel_started(data.kernel);
     };
     

IPython/html/tests/notebook/save.js

@@ -80,7 +80,7 b' casper.notebook_test(function () {'
             });
             return return_this_thing;
         }, {nbname:nbname});
-        this.test.assertEquals(notebook_url == null, false, "Escaped URL in notebook list");
+        this.test.assertNotEquals(notebook_url, null, "Escaped URL in notebook list");
         // open the notebook
         this.open(notebook_url);
     });

IPython/html/tests/util.js

@@ -63,7 +63,7 b' casper.kernel_running = function() {'
 casper.shutdown_current_kernel = function () {
     // Shut down the current notebook's kernel.
     this.thenEvaluate(function() {
-        IPython.notebook.kernel.kill();
+        IPython.notebook.session.delete();
     });
     // We close the page right after this so we need to give it time to complete.
     this.wait(1000);

IPython/kernel/multikernelmanager.py

@@ -92,7 +92,7 b' class MultiKernelManager(LoggingConfigurable):'
     def __contains__(self, kernel_id):
         return kernel_id in self._kernels
 
-    def start_kernel(self, **kwargs):
+    def start_kernel(self, kernel_name='python', **kwargs):
         """Start a new kernel.
 
         The caller can pick a kernel_id by passing one in as a keyword arg,
@@ -111,7 +111,7 b' class MultiKernelManager(LoggingConfigurable):'
         # including things like its transport and ip.
         km = self.kernel_manager_factory(connection_file=os.path.join(
                     self.connection_dir, "kernel-%s.json" % kernel_id),
-                    parent=self, autorestart=True, log=self.log
+                    parent=self, autorestart=True, log=self.log, kernel_name=kernel_name,
         )
         km.start_kernel(**kwargs)
         self._kernels[kernel_id] = km