From b537531d189d37920ca95820c5739cbd434b28be 2014-07-11 21:58:31 From: Min RK Date: 2014-07-11 21:58:31 Subject: [PATCH] Merge pull request #6026 from takluyver/kernelspec-rest-launching Kernelspecs in REST API for kernels and sessions --- diff --git a/IPython/html/notebookapp.py b/IPython/html/notebookapp.py index 2c8324d..2e5a1b9 100644 --- a/IPython/html/notebookapp.py +++ b/IPython/html/notebookapp.py @@ -658,7 +658,9 @@ 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() diff --git a/IPython/html/services/kernels/handlers.py b/IPython/html/services/kernels/handlers.py index 58c2355..3d4337f 100644 --- a/IPython/html/services/kernels/handlers.py +++ b/IPython/html/services/kernels/handlers.py @@ -27,8 +27,16 @@ 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)) diff --git a/IPython/html/services/kernels/kernelmanager.py b/IPython/html/services/kernels/kernelmanager.py index ff27b59..3132efb 100644 --- a/IPython/html/services/kernels/kernelmanager.py +++ b/IPython/html/services/kernels/kernelmanager.py @@ -72,8 +72,8 @@ 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 @@ 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 @@ 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): diff --git a/IPython/html/services/kernels/tests/test_kernels_api.py b/IPython/html/services/kernels/tests/test_kernels_api.py index 5e624a7..6c4ef9b 100644 --- a/IPython/html/services/kernels/tests/test_kernels_api.py +++ b/IPython/html/services/kernels/tests/test_kernels_api.py @@ -1,6 +1,6 @@ """Test the kernels service API.""" - +import json import requests from IPython.html.utils import url_path_join @@ -30,8 +30,9 @@ 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 @@ 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 @@ 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 diff --git a/IPython/html/services/sessions/handlers.py b/IPython/html/services/sessions/handlers.py index 7ed47f6..cd84dc4 100644 --- a/IPython/html/services/sessions/handlers.py +++ b/IPython/html/services/sessions/handlers.py @@ -45,27 +45,28 @@ 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 @@ 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() diff --git a/IPython/html/services/sessions/sessionmanager.py b/IPython/html/services/sessions/sessionmanager.py index ec96778..cc68028 100644 --- a/IPython/html/services/sessions/sessionmanager.py +++ b/IPython/html/services/sessions/sessionmanager.py @@ -23,12 +23,16 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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,)) diff --git a/IPython/html/services/sessions/tests/test_sessionmanager.py b/IPython/html/services/sessions/tests/test_sessionmanager.py index d40aa23..ca080e7 100644 --- a/IPython/html/services/sessions/tests/test_sessionmanager.py +++ b/IPython/html/services/sessions/tests/test_sessionmanager.py @@ -5,79 +5,101 @@ 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 diff --git a/IPython/html/services/sessions/tests/test_sessions_api.py b/IPython/html/services/sessions/tests/test_sessions_api.py index 896f4cd..9623415 100644 --- a/IPython/html/services/sessions/tests/test_sessions_api.py +++ b/IPython/html/services/sessions/tests/test_sessions_api.py @@ -37,8 +37,9 @@ 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): diff --git a/IPython/html/static/notebook/js/notebook.js b/IPython/html/static/notebook/js/notebook.js index 7a899f9..8586eb6 100644 --- a/IPython/html/static/notebook/js/notebook.js +++ b/IPython/html/static/notebook/js/notebook.js @@ -59,6 +59,9 @@ 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 @@ 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)); }; diff --git a/IPython/html/static/services/kernels/js/kernel.js b/IPython/html/static/services/kernels/js/kernel.js index 8004c15..6d41ca5 100644 --- a/IPython/html/static/services/kernels/js/kernel.js +++ b/IPython/html/static/services/kernels/js/kernel.js @@ -15,13 +15,14 @@ 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(); diff --git a/IPython/html/static/services/sessions/js/session.js b/IPython/html/static/services/sessions/js/session.js index 1cfb1a9..2283c85 100644 --- a/IPython/html/static/services/sessions/js/session.js +++ b/IPython/html/static/services/sessions/js/session.js @@ -15,6 +15,7 @@ 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 @@ define([ notebook : { name : this.name, path : this.path + }, + kernel : { + name : this.kernel_name } }; var settings = { @@ -87,7 +91,7 @@ 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); }; diff --git a/IPython/html/tests/notebook/save.js b/IPython/html/tests/notebook/save.js index 30c670d..d325b8f 100644 --- a/IPython/html/tests/notebook/save.js +++ b/IPython/html/tests/notebook/save.js @@ -80,7 +80,7 @@ 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); }); diff --git a/IPython/html/tests/util.js b/IPython/html/tests/util.js index 666f94a..bc9ce6d 100644 --- a/IPython/html/tests/util.js +++ b/IPython/html/tests/util.js @@ -63,7 +63,7 @@ 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); diff --git a/IPython/kernel/multikernelmanager.py b/IPython/kernel/multikernelmanager.py index 8c46197..28f7ab2 100644 --- a/IPython/kernel/multikernelmanager.py +++ b/IPython/kernel/multikernelmanager.py @@ -92,7 +92,7 @@ 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 @@ 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