diff --git a/IPython/html/services/sessions/sessionmanager.py b/IPython/html/services/sessions/sessionmanager.py index 67adbb7..b105344 100644 --- a/IPython/html/services/sessions/sessionmanager.py +++ b/IPython/html/services/sessions/sessionmanager.py @@ -53,7 +53,7 @@ class SessionManager(LoggingConfigurable): """Start a database connection""" if self._connection is None: self._connection = sqlite3.connect(':memory:') - self._connection.row_factory = self.row_factory + self._connection.row_factory = sqlite3.Row return self._connection def __del__(self): @@ -141,14 +141,20 @@ class SessionManager(LoggingConfigurable): query = "SELECT * FROM session WHERE %s" % (' AND '.join(conditions)) self.cursor.execute(query, list(kwargs.values())) - model = self.cursor.fetchone() - if model is None: + try: + row = self.cursor.fetchone() + except KeyError: + # The kernel is missing, so the session just got deleted. + row = None + + if row is None: q = [] for key, value in kwargs.items(): q.append("%s=%r" % (key, value)) raise web.HTTPError(404, u'Session not found: %s' % (', '.join(q))) - return model + + return self.row_to_model(row) def update_session(self, session_id, **kwargs): """Updates the values in the session database. @@ -179,9 +185,16 @@ class SessionManager(LoggingConfigurable): query = "UPDATE session SET %s WHERE session_id=?" % (', '.join(sets)) self.cursor.execute(query, list(kwargs.values()) + [session_id]) - def row_factory(self, cursor, row): + def row_to_model(self, row): """Takes sqlite database session row and turns it into a dictionary""" - row = sqlite3.Row(cursor, row) + if row['kernel_id'] not in self.kernel_manager: + # The kernel was killed or died without deleting the session. + # We can't use delete_session here because that tries to find + # and shut down the kernel. + self.cursor.execute("DELETE FROM session WHERE session_id=?", + (row['session_id'],)) + raise KeyError + model = { 'id': row['session_id'], 'notebook': { @@ -196,7 +209,15 @@ class SessionManager(LoggingConfigurable): """Returns a list of dictionaries containing all the information from the session database""" c = self.cursor.execute("SELECT * FROM session") - return list(c.fetchall()) + result = [] + # We need to use fetchall() here, because row_to_model can delete rows, + # which messes up the cursor if we're iterating over rows. + for row in c.fetchall(): + try: + result.append(self.row_to_model(row)) + except KeyError: + pass + return result def delete_session(self, session_id): """Deletes the row in the session database with given session_id""" diff --git a/IPython/html/services/sessions/tests/test_sessionmanager.py b/IPython/html/services/sessions/tests/test_sessionmanager.py index ca080e7..6383a0b 100644 --- a/IPython/html/services/sessions/tests/test_sessionmanager.py +++ b/IPython/html/services/sessions/tests/test_sessionmanager.py @@ -47,6 +47,17 @@ class TestSessionManager(TestCase): kernel_name='foo')['id'] self.assertRaises(TypeError, sm.get_session, bad_id=session_id) # Bad keyword + def test_get_session_dead_kernel(self): + sm = SessionManager(kernel_manager=DummyMKM()) + session = sm.create_session(name='test1.ipynb', path='/path/to/1/', kernel_name='python') + # kill the kernel + sm.kernel_manager.shutdown_kernel(session['kernel']['id']) + with self.assertRaises(KeyError): + sm.get_session(session_id=session['id']) + # no sessions left + listed = sm.list_sessions() + self.assertEqual(listed, []) + def test_list_sessions(self): sm = SessionManager(kernel_manager=DummyMKM()) sessions = [ @@ -63,6 +74,30 @@ class TestSessionManager(TestCase): 'path': u'/path/to/3/'}, 'kernel':{'id':u'C', 'name':'python'}}] self.assertEqual(sessions, expected) + def test_list_sessions_dead_kernel(self): + 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'), + ] + # kill one of the kernels + sm.kernel_manager.shutdown_kernel(sessions[0]['kernel']['id']) + listed = sm.list_sessions() + expected = [ + { + 'id': sessions[1]['id'], + 'notebook': { + 'name': u'test2.ipynb', + 'path': u'/path/to/2/', + }, + 'kernel': { + 'id': u'B', + 'name':'python', + } + } + ] + self.assertEqual(listed, expected) + def test_update_session(self): sm = SessionManager(kernel_manager=DummyMKM()) session_id = sm.create_session(name='test.ipynb', path='/path/to/', diff --git a/IPython/html/tests/launchnotebook.py b/IPython/html/tests/launchnotebook.py index e49fe80..7775e28 100644 --- a/IPython/html/tests/launchnotebook.py +++ b/IPython/html/tests/launchnotebook.py @@ -95,7 +95,7 @@ def assert_http_error(status, msg=None): except requests.HTTPError as e: real_status = e.response.status_code assert real_status == status, \ - "Expected status %d, got %d" % (real_status, status) + "Expected status %d, got %d" % (status, real_status) if msg: assert msg in str(e), e else: