From c1223d48738c366a2084234ae70c7005e04f1dbd 2014-07-11 04:50:00 From: Thomas Kluyver Date: 2014-07-11 04:50:00 Subject: [PATCH] Add kernel name to sessions REST API Also, some refactoring so that the relationship between session, kernels and notebooks is managed in the SessionManager, not in the HTTP handlers. --- 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/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):