From 1ae66783606b34b450f41c4938f1aa182245f805 Mon Sep 17 00:00:00 2001 From: Tom Kelley Date: Mon, 22 Apr 2019 11:51:51 -0700 Subject: [PATCH 1/3] Refactor Logout Handler AS A developer of a Logout handler I WANT to be able to call a function to kill spawners and do other backend logout stuff and a separate function to forward the user along the logout chain. I believe this PR adds (moderately private) methods to the Logout Handler to do just that. --- jupyterhub/handlers/login.py | 42 +++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/jupyterhub/handlers/login.py b/jupyterhub/handlers/login.py index a2f6577d..d78cd637 100644 --- a/jupyterhub/handlers/login.py +++ b/jupyterhub/handlers/login.py @@ -18,27 +18,35 @@ class LogoutHandler(BaseHandler): def shutdown_on_logout(self): return self.settings.get('shutdown_on_logout', False) - async def get(self): + async def _shutdown_servers(self, user): + active_servers = [ + name + for (name, spawner) in user.spawners.items() + if spawner.active and not spawner.pending + ] + if active_servers: + self.log.info("Shutting down %s's servers", user.name) + futures = [] + for server_name in active_servers: + futures.append(maybe_future(self.stop_single_user(user, server_name))) + await asyncio.gather(*futures) + + def _backend_logout_cleanup(self, name): + self.log.info("User logged out: %s", name) + self.clear_login_cookie() + self.statsd.incr('logout') + + async def _shutdown_spawners_and_backend_cleanup(self): user = self.current_user if user: if self.shutdown_on_logout: - active_servers = [ - name - for (name, spawner) in user.spawners.items() - if spawner.active and not spawner.pending - ] - if active_servers: - self.log.info("Shutting down %s's servers", user.name) - futures = [] - for server_name in active_servers: - futures.append( - maybe_future(self.stop_single_user(user, server_name)) - ) - await asyncio.gather(*futures) + await self._shutdown_servers(user) + + self._backend_logout_cleanup(user.name) + + async def get(self): + await self._shutdown_spawners_and_backend_cleanup() - self.log.info("User logged out: %s", user.name) - self.clear_login_cookie() - self.statsd.incr('logout') if self.authenticator.auto_login: html = self.render_template('logout.html') self.finish(html) From 77d4c1f23dfab070f5a78cd56a0525f4e8ba4609 Mon Sep 17 00:00:00 2001 From: Tom Kelley Date: Tue, 23 Apr 2019 09:59:56 -0700 Subject: [PATCH 2/3] Changes after CR Comments Big thanks to Erik, Tim, and Min for the great comments! Change names to be more clear, add function doc comments, change scoping on some functions, add handle_logout to let people take custom logout actions, extract render_logout_page from get method, add TODO. --- jupyterhub/handlers/login.py | 40 +++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/jupyterhub/handlers/login.py b/jupyterhub/handlers/login.py index d78cd637..b674155e 100644 --- a/jupyterhub/handlers/login.py +++ b/jupyterhub/handlers/login.py @@ -19,6 +19,10 @@ class LogoutHandler(BaseHandler): return self.settings.get('shutdown_on_logout', False) async def _shutdown_servers(self, user): + """Shutdown servers for logout + + Get all active servers for the provided user, stop them. + """ active_servers = [ name for (name, spawner) in user.spawners.items() @@ -32,11 +36,21 @@ class LogoutHandler(BaseHandler): await asyncio.gather(*futures) def _backend_logout_cleanup(self, name): + """Default backend logout actions + + Send a log message, clear some cookies, increment the logout counter. + """ self.log.info("User logged out: %s", name) self.clear_login_cookie() self.statsd.incr('logout') - async def _shutdown_spawners_and_backend_cleanup(self): + async def default_handle_logout(self): + """The default logout action + + Optionally cleans up servers, clears cookies, increments logout counter + Cleaning up servers can be prevented by setting shutdown_on_logout to + False. + """ user = self.current_user if user: if self.shutdown_on_logout: @@ -44,15 +58,35 @@ class LogoutHandler(BaseHandler): self._backend_logout_cleanup(user.name) - async def get(self): - await self._shutdown_spawners_and_backend_cleanup() + async def handle_logout(self): + """Custom user action during logout + By default a no-op, this function should be overridden in subclasses + to have JupyterHub take a custom action on logout. + """ + return + + async def render_logout_page(self): + """Render the logout page, if any + + Override this function to set a custom logout page. + """ if self.authenticator.auto_login: html = self.render_template('logout.html') self.finish(html) else: self.redirect(self.settings['login_url'], permanent=False) + async def get(self): + """Log the user out, call the custom action, forward the user + to the logout page + """ + # TODO: when these can all be done concurrently, do all of them + # concurrently? + await self.default_handle_logout() + await self.handle_logout() + await self.render_logout_page() + class LoginHandler(BaseHandler): """Render the login page.""" From 40db4edc6deb0feff5aad07945ea4f59b4cc147e Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 25 Apr 2019 13:51:27 +0200 Subject: [PATCH 3/3] remove todo order should be preserved between multiple steps --- jupyterhub/handlers/login.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/jupyterhub/handlers/login.py b/jupyterhub/handlers/login.py index b674155e..f1bd7c45 100644 --- a/jupyterhub/handlers/login.py +++ b/jupyterhub/handlers/login.py @@ -81,8 +81,6 @@ class LogoutHandler(BaseHandler): """Log the user out, call the custom action, forward the user to the logout page """ - # TODO: when these can all be done concurrently, do all of them - # concurrently? await self.default_handle_logout() await self.handle_logout() await self.render_logout_page()