Merge pull request #711 from minrk/allow-async-kernel-manager

allow some KernelManager methods to be async

notebook/services/kernels/handlers.py

@@ -25,12 +25,15 @@ class MainKernelHandler(APIHandler):
 
     @web.authenticated
     @json_errors
+    @gen.coroutine
     def get(self):
         km = self.kernel_manager
-        self.finish(json.dumps(km.list_kernels()))
+        kernels = yield gen.maybe_future(km.list_kernels())
+        self.finish(json.dumps(kernels))
 
     @web.authenticated
     @json_errors
+    @gen.coroutine
     def post(self):
         km = self.kernel_manager
         model = self.get_json_body()
@@ -41,7 +44,7 @@ class MainKernelHandler(APIHandler):
         else:
             model.setdefault('name', km.default_kernel_name)
 
-        kernel_id = km.start_kernel(kernel_name=model['name'])
+        kernel_id = yield gen.maybe_future(km.start_kernel(kernel_name=model['name']))
         model = km.kernel_model(kernel_id)
         location = url_path_join(self.base_url, 'api', 'kernels', url_escape(kernel_id))
         self.set_header('Location', location)
@@ -61,9 +64,10 @@ class KernelHandler(APIHandler):
 
     @web.authenticated
     @json_errors
+    @gen.coroutine
     def delete(self, kernel_id):
         km = self.kernel_manager
-        km.shutdown_kernel(kernel_id)
+        yield gen.maybe_future(km.shutdown_kernel(kernel_id))
         self.set_status(204)
         self.finish()
 

notebook/services/kernels/kernelmanager.py

@@ -62,7 +62,8 @@ class MappingKernelManager(MultiKernelManager):
         while not os.path.isdir(os_path) and os_path != self.root_dir:
             os_path = os.path.dirname(os_path)
         return os_path
-
+    
+    @gen.coroutine
     def start_kernel(self, kernel_id=None, path=None, **kwargs):
         """Start a kernel for a session and return its kernel_id.
 
@@ -82,8 +83,9 @@ class MappingKernelManager(MultiKernelManager):
         if kernel_id is None:
             if path is not None:
                 kwargs['cwd'] = self.cwd_for_path(path)
-            kernel_id = super(MappingKernelManager, self).start_kernel(
-                                            **kwargs)
+            kernel_id = yield gen.maybe_future(
+                super(MappingKernelManager, self).start_kernel(**kwargs)
+            )
             self.log.info("Kernel started: %s" % kernel_id)
             self.log.debug("Kernel args: %r" % kwargs)
             # register callback for failed auto-restart
@@ -94,12 +96,13 @@ class MappingKernelManager(MultiKernelManager):
         else:
             self._check_kernel_id(kernel_id)
             self.log.info("Using existing kernel: %s" % kernel_id)
-        return kernel_id
+        # py2-compat
+        raise gen.Return(kernel_id)
 
     def shutdown_kernel(self, kernel_id, now=False):
         """Shutdown a kernel by kernel_id"""
         self._check_kernel_id(kernel_id)
-        super(MappingKernelManager, self).shutdown_kernel(kernel_id, now=now)
+        return super(MappingKernelManager, self).shutdown_kernel(kernel_id, now=now)
 
     def restart_kernel(self, kernel_id):
         """Restart a kernel by kernel_id"""

notebook/services/sessions/handlers.py

@@ -8,7 +8,7 @@ Preliminary documentation at https://github.com/ipython/ipython/wiki/IPEP-16%3A-
 
 import json
 
-from tornado import web
+from tornado import gen, web
 
 from ...base.handlers import APIHandler, json_errors
 from jupyter_client.jsonutil import date_default
@@ -20,20 +20,20 @@ class SessionRootHandler(APIHandler):
 
     @web.authenticated
     @json_errors
+    @gen.coroutine
     def get(self):
         # Return a list of running sessions
         sm = self.session_manager
-        sessions = sm.list_sessions()
+        sessions = yield gen.maybe_future(sm.list_sessions())
         self.finish(json.dumps(sessions, default=date_default))
 
     @web.authenticated
     @json_errors
+    @gen.coroutine
     def post(self):
         # Creates a new session
         #(unless a session already exists for the named nb)
         sm = self.session_manager
-        cm = self.contents_manager
-        km = self.kernel_manager
 
         model = self.get_json_body()
         if model is None:
@@ -49,11 +49,13 @@ class SessionRootHandler(APIHandler):
             kernel_name = None
 
         # Check to see if session exists
-        if sm.session_exists(path=path):
-            model = sm.get_session(path=path)
+        exists = yield gen.maybe_future(sm.session_exists(path=path))
+        if exists:
+            model = yield gen.maybe_future(sm.get_session(path=path))
         else:
             try:
-                model = sm.create_session(path=path, kernel_name=kernel_name)
+                model = yield gen.maybe_future(
+                    sm.create_session(path=path, kernel_name=kernel_name))
             except NoSuchKernel:
                 msg = ("The '%s' kernel is not available. Please pick another "
                        "suitable kernel instead, or install that kernel." % kernel_name)
@@ -73,14 +75,16 @@ class SessionHandler(APIHandler):
 
     @web.authenticated
     @json_errors
+    @gen.coroutine
     def get(self, session_id):
         # Returns the JSON model for a single session
         sm = self.session_manager
-        model = sm.get_session(session_id=session_id)
+        model = yield gen.maybe_future(sm.get_session(session_id=session_id))
         self.finish(json.dumps(model, default=date_default))
 
     @web.authenticated
     @json_errors
+    @gen.coroutine
     def patch(self, session_id):
         # Currently, this handler is strictly for renaming notebooks
         sm = self.session_manager
@@ -93,17 +97,18 @@ class SessionHandler(APIHandler):
             if 'path' in notebook:
                 changes['path'] = notebook['path']
 
-        sm.update_session(session_id, **changes)
-        model = sm.get_session(session_id=session_id)
+        yield gen.maybe_future(sm.update_session(session_id, **changes))
+        model = yield gen.maybe_future(sm.get_session(session_id=session_id))
         self.finish(json.dumps(model, default=date_default))
 
     @web.authenticated
     @json_errors
+    @gen.coroutine
     def delete(self, session_id):
         # Deletes the session with given session_id
         sm = self.session_manager
         try:
-            sm.delete_session(session_id)
+            yield gen.maybe_future(sm.delete_session(session_id))
         except KeyError:
             # the kernel was deleted but the session wasn't!
             raise web.HTTPError(410, "Kernel deleted before session")

notebook/services/sessions/sessionmanager.py

@@ -6,7 +6,7 @@
 import uuid
 import sqlite3
 
-from tornado import web
+from tornado import gen, web
 
 from traitlets.config.configurable import LoggingConfigurable
 from ipython_genutils.py3compat import unicode_type
@@ -39,10 +39,16 @@ class SessionManager(LoggingConfigurable):
             self._connection = sqlite3.connect(':memory:')
             self._connection.row_factory = sqlite3.Row
         return self._connection
-        
+    
+    def close(self):
+        """Close the sqlite connection"""
+        if self._cursor is not None:
+            self._cursor.close()
+            self._cursor = None
+
     def __del__(self):
         """Close connection once SessionManager closes"""
-        self.cursor.close()
+        self.close()
 
     def session_exists(self, path):
         """Check to see if the session for a given notebook exists"""
@@ -56,17 +62,22 @@ class SessionManager(LoggingConfigurable):
     def new_session_id(self):
         "Create a uuid for a new session"
         return unicode_type(uuid.uuid4())
-
+    
+    @gen.coroutine
     def create_session(self, path=None, kernel_name=None):
         """Creates a session and returns its model"""
         session_id = self.new_session_id()
         # allow nbm to specify kernels cwd
         kernel_path = self.contents_manager.get_kernel_path(path=path)
-        kernel_id = self.kernel_manager.start_kernel(path=kernel_path,
-                                                     kernel_name=kernel_name)
-        return self.save_session(session_id, path=path,
-                                 kernel_id=kernel_id)
-
+        kernel_id = yield gen.maybe_future(
+            self.kernel_manager.start_kernel(path=kernel_path, kernel_name=kernel_name)
+        )
+        result = yield gen.maybe_future(
+            self.save_session(session_id, path=path, kernel_id=kernel_id)
+        )
+        # py2-compat
+        raise gen.Return(result)
+    
     def save_session(self, session_id, path=None, kernel_id=None):
         """Saves the items for the session with the given session_id
         

notebook/services/sessions/tests/test_sessionmanager.py

@@ -2,7 +2,8 @@
 
 from unittest import TestCase
 
-from tornado import web
+from tornado import gen, web
+from tornado.ioloop import IOLoop
 
 from ..sessionmanager import SessionManager
 from notebook.services.kernels.kernelmanager import MappingKernelManager
@@ -37,11 +38,27 @@ class TestSessionManager(TestCase):
             kernel_manager=DummyMKM(),
             contents_manager=ContentsManager(),
         )
-
+        self.loop = IOLoop()
+    
+    def tearDown(self):
+        self.loop.close(all_fds=True)
+    
+    def create_sessions(self, *kwarg_list):
+        @gen.coroutine
+        def co_add():
+            sessions = []
+            for kwargs in kwarg_list:
+                session = yield self.sm.create_session(**kwargs)
+                sessions.append(session)
+            raise gen.Return(sessions)
+        return self.loop.run_sync(co_add)
+    
+    def create_session(self, **kwargs):
+        return self.create_sessions(kwargs)[0]
+    
     def test_get_session(self):
         sm = self.sm
-        session_id = sm.create_session(path='/path/to/test.ipynb',
-                                       kernel_name='bar')['id']
+        session_id = self.create_session(path='/path/to/test.ipynb', kernel_name='bar')['id']
         model = sm.get_session(session_id=session_id)
         expected = {'id':session_id,
                     'notebook':{'path': u'/path/to/test.ipynb'},
@@ -51,13 +68,13 @@ class TestSessionManager(TestCase):
     def test_bad_get_session(self):
         # Should raise error if a bad key is passed to the database.
         sm = self.sm
-        session_id = sm.create_session(path='/path/to/test.ipynb',
+        session_id = self.create_session(path='/path/to/test.ipynb',
                                        kernel_name='foo')['id']
         self.assertRaises(TypeError, sm.get_session, bad_id=session_id) # Bad keyword
 
     def test_get_session_dead_kernel(self):
         sm = self.sm
-        session = sm.create_session(path='/path/to/1/test1.ipynb', kernel_name='python')
+        session = self.create_session(path='/path/to/1/test1.ipynb', kernel_name='python')
         # kill the kernel
         sm.kernel_manager.shutdown_kernel(session['kernel']['id'])
         with self.assertRaises(KeyError):
@@ -68,11 +85,12 @@ class TestSessionManager(TestCase):
 
     def test_list_sessions(self):
         sm = self.sm
-        sessions = [
-            sm.create_session(path='/path/to/1/test1.ipynb', kernel_name='python'),
-            sm.create_session(path='/path/to/2/test2.ipynb', kernel_name='python'),
-            sm.create_session(path='/path/to/3/test3.ipynb', kernel_name='python'),
-        ]
+        sessions = self.create_sessions(
+            dict(path='/path/to/1/test1.ipynb', kernel_name='python'),
+            dict(path='/path/to/2/test2.ipynb', kernel_name='python'),
+            dict(path='/path/to/3/test3.ipynb', kernel_name='python'),
+        )
+        
         sessions = sm.list_sessions()
         expected = [
             {
@@ -93,10 +111,10 @@ class TestSessionManager(TestCase):
 
     def test_list_sessions_dead_kernel(self):
         sm = self.sm
-        sessions = [
-            sm.create_session(path='/path/to/1/test1.ipynb', kernel_name='python'),
-            sm.create_session(path='/path/to/2/test2.ipynb', kernel_name='python'),
-        ]
+        sessions = self.create_sessions(
+            dict(path='/path/to/1/test1.ipynb', kernel_name='python'),
+            dict(path='/path/to/2/test2.ipynb', kernel_name='python'),
+        )
         # kill one of the kernels
         sm.kernel_manager.shutdown_kernel(sessions[0]['kernel']['id'])
         listed = sm.list_sessions()
@@ -116,7 +134,7 @@ class TestSessionManager(TestCase):
 
     def test_update_session(self):
         sm = self.sm
-        session_id = sm.create_session(path='/path/to/test.ipynb',
+        session_id = self.create_session(path='/path/to/test.ipynb',
                                        kernel_name='julia')['id']
         sm.update_session(session_id, path='/path/to/new_name.ipynb')
         model = sm.get_session(session_id=session_id)
@@ -128,17 +146,17 @@ class TestSessionManager(TestCase):
     def test_bad_update_session(self):
         # try to update a session with a bad keyword ~ raise error
         sm = self.sm
-        session_id = sm.create_session(path='/path/to/test.ipynb',
+        session_id = self.create_session(path='/path/to/test.ipynb',
                                        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 = self.sm
-        sessions = [
-            sm.create_session(path='/path/to/1/test1.ipynb', kernel_name='python'),
-            sm.create_session(path='/path/to/2/test2.ipynb', kernel_name='python'),
-            sm.create_session(path='/path/to/3/test3.ipynb', kernel_name='python'),
-        ]
+        sessions = self.create_sessions(
+            dict(path='/path/to/1/test1.ipynb', kernel_name='python'),
+            dict(path='/path/to/2/test2.ipynb', kernel_name='python'),
+            dict(path='/path/to/3/test3.ipynb', kernel_name='python'),
+        )
         sm.delete_session(sessions[1]['id'])
         new_sessions = sm.list_sessions()
         expected = [{
@@ -156,7 +174,7 @@ class TestSessionManager(TestCase):
     def test_bad_delete_session(self):
         # try to delete a session that doesn't exist ~ raise error
         sm = self.sm
-        sm.create_session(path='/path/to/test.ipynb', kernel_name='python')
+        self.create_session(path='/path/to/test.ipynb', 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
 

notebook/tests/launchnotebook.py

@@ -19,6 +19,7 @@ except ImportError:
 
 from tornado.ioloop import IOLoop
 
+import jupyter_core.paths
 from ..notebookapp import NotebookApp
 from ipython_genutils.tempdir import TemporaryDirectory
 
@@ -51,9 +52,8 @@ class NotebookTestBase(TestCase):
             try:
                 requests.get(url)
             except Exception as e:
-                if cls.notebook.poll() is not None:
-                    raise RuntimeError("The notebook server exited with status %s" \
-                                        % cls.notebook.poll())
+                if not cls.notebook_thread.is_alive():
+                    raise RuntimeError("The notebook server failed to start")
                 time.sleep(POLL_INTERVAL)
             else:
                 return
@@ -77,31 +77,35 @@ class NotebookTestBase(TestCase):
             'JUPYTER_DATA_DIR' : data_dir.name
         })
         cls.env_patch.start()
+        cls.path_patch = patch.object(jupyter_core.paths, 'SYSTEM_JUPYTER_PATH', [])
+        cls.path_patch.start()
         cls.config_dir = TemporaryDirectory()
         cls.data_dir = data_dir
         cls.runtime_dir = TemporaryDirectory()
         cls.notebook_dir = TemporaryDirectory()
-        app = cls.notebook = NotebookApp(
-            port=cls.port,
-            port_retries=0,
-            open_browser=False,
-            config_dir=cls.config_dir.name,
-            data_dir=cls.data_dir.name,
-            runtime_dir=cls.runtime_dir.name,
-            notebook_dir=cls.notebook_dir.name,
-            base_url=cls.url_prefix,
-            config=cls.config,
-        )
         
-        # clear log handlers and propagate to root for nose to capture it
-        # needs to be redone after initialize, which reconfigures logging
-        app.log.propagate = True
-        app.log.handlers = []
-        app.initialize(argv=[])
-        app.log.propagate = True
-        app.log.handlers = []
         started = Event()
         def start_thread():
+            app = cls.notebook = NotebookApp(
+                port=cls.port,
+                port_retries=0,
+                open_browser=False,
+                config_dir=cls.config_dir.name,
+                data_dir=cls.data_dir.name,
+                runtime_dir=cls.runtime_dir.name,
+                notebook_dir=cls.notebook_dir.name,
+                base_url=cls.url_prefix,
+                config=cls.config,
+            )
+            # don't register signal handler during tests
+            app.init_signal = lambda : None
+            # clear log handlers and propagate to root for nose to capture it
+            # needs to be redone after initialize, which reconfigures logging
+            app.log.propagate = True
+            app.log.handlers = []
+            app.initialize(argv=[])
+            app.log.propagate = True
+            app.log.handlers = []
             loop = IOLoop.current()
             loop.add_callback(started.set)
             try:
@@ -109,6 +113,7 @@ class NotebookTestBase(TestCase):
             finally:
                 # set the event, so failure to start doesn't cause a hang
                 started.set()
+                app.session_manager.close()
         cls.notebook_thread = Thread(target=start_thread)
         cls.notebook_thread.start()
         started.wait()
@@ -118,12 +123,13 @@ class NotebookTestBase(TestCase):
     def teardown_class(cls):
         cls.notebook.stop()
         cls.wait_until_dead()
-        cls.env_patch.start()
         cls.home_dir.cleanup()
         cls.config_dir.cleanup()
         cls.data_dir.cleanup()
         cls.runtime_dir.cleanup()
         cls.notebook_dir.cleanup()
+        cls.env_patch.stop()
+        cls.path_patch.stop()
 
     @classmethod
     def base_url(cls):