From 4729ae47693e65e11f4635bceed6313e75d7c76f Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 3 Jan 2018 14:12:02 +0100 Subject: [PATCH 1/3] tornado 5 fixes - ._running private attribute is removed. We don't need it anymore, since we were only using it while the application was run in a background thread. - call blocking cleanup in a thread because asyncio doesn't allow multiple loops in one thread. --- jupyterhub/app.py | 5 +---- jupyterhub/tests/mocking.py | 23 +++++++++++++++++++---- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index be3acff4..c74615d7 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1625,10 +1625,7 @@ class JupyterHub(Application): if not self.io_loop: return if self.http_server: - if self.io_loop._running: - self.io_loop.add_callback(self.http_server.stop) - else: - self.http_server.stop() + self.http_server.stop() self.io_loop.add_callback(self.io_loop.stop) @gen.coroutine diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index 63f283b9..983fec08 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -1,5 +1,6 @@ """mock utilities for testing""" +from concurrent.futures import ThreadPoolExecutor import os import sys from tempfile import NamedTemporaryFile @@ -202,11 +203,11 @@ class MockHub(JupyterHub): @default('authenticator_class') def _authenticator_class_default(self): return MockPAMAuthenticator - + @default('spawner_class') def _spawner_class_default(self): return MockSpawner - + def init_signal(self): pass @@ -229,11 +230,25 @@ class MockHub(JupyterHub): def stop(self): super().stop() - IOLoop().run_sync(self.cleanup) + + # run cleanup in a background thread + # to avoid multiple eventloops in the same thread errors from asyncio + + def cleanup(): + loop = IOLoop.current() + loop.run_sync(self.cleanup) + loop.close() + + pool = ThreadPoolExecutor(1) + f = pool.submit(cleanup) + # wait for cleanup to finish + f.result() + pool.shutdown() + # ignore the call that will fire in atexit self.cleanup = lambda : None self.db_file.close() - + @gen.coroutine def login_user(self, name): """Login a user by name, returning her cookies.""" From f87f24d9e5b36d75c99ac0ebd0ecba069dcf0574 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 3 Jan 2018 14:12:29 +0100 Subject: [PATCH 2/3] unpin tornado --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 98aa3aeb..7a71f61a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,6 @@ alembic traitlets>=4.3.2 -tornado>=4.1,<5.0 +tornado>=4.1 jinja2 pamela python-oauth2>=1.0 From 9866a0fadc3e77be4089e3283040e5446075fc22 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 3 Jan 2018 14:58:42 +0100 Subject: [PATCH 3/3] avoid raising HTTPError in get_current_user it can cause issues, e.g. with upcoming notebook releases .get_current_user may be called in set_default_headers, which doesn't catch HTTPErrors. --- jupyterhub/services/auth.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/jupyterhub/services/auth.py b/jupyterhub/services/auth.py index 93059ce3..34ffd0a0 100644 --- a/jupyterhub/services/auth.py +++ b/jupyterhub/services/auth.py @@ -778,7 +778,14 @@ class HubAuthenticated(object): except UserNotAllowed as e: # cache None, in case get_user is called again while processing the error self._hub_auth_user_cache = None - raise HTTPError(403, "{kind} {name} is not allowed.".format(**e.model)) + # Override redirect so if/when tornado @web.authenticated + # tries to redirect to login URL, 403 will be raised instead. + # This is not the best, but avoids problems that can be caused + # when get_current_user is allowed to raise. + def raise_on_redirect(*args, **kwargs): + raise HTTPError(403, "{kind} {name} is not allowed.".format(**user_model)) + self.redirect = raise_on_redirect + return except Exception: self._hub_auth_user_cache = None raise