diff --git a/notebook/services/kernels/handlers.py b/notebook/services/kernels/handlers.py index 19bc20315..0da36714f 100644 --- a/notebook/services/kernels/handlers.py +++ b/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() diff --git a/notebook/services/kernels/kernelmanager.py b/notebook/services/kernels/kernelmanager.py index d08d2ba8d..ccf2738a7 100644 --- a/notebook/services/kernels/kernelmanager.py +++ b/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""" diff --git a/notebook/services/sessions/handlers.py b/notebook/services/sessions/handlers.py index 4c46c45df..b7a7f11de 100644 --- a/notebook/services/sessions/handlers.py +++ b/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") diff --git a/notebook/services/sessions/sessionmanager.py b/notebook/services/sessions/sessionmanager.py index b75732087..125940408 100644 --- a/notebook/services/sessions/sessionmanager.py +++ b/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 diff --git a/notebook/services/sessions/tests/test_sessionmanager.py b/notebook/services/sessions/tests/test_sessionmanager.py index d1382ed1d..0b31d97a4 100644 --- a/notebook/services/sessions/tests/test_sessionmanager.py +++ b/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 diff --git a/notebook/tests/launchnotebook.py b/notebook/tests/launchnotebook.py index b957cdf4d..e9842ca43 100644 --- a/notebook/tests/launchnotebook.py +++ b/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):