From 118fa9e4801779526f92728d8cc54375e9b6219c Mon Sep 17 00:00:00 2001 From: Tim Kreuzer Date: Wed, 17 May 2023 14:50:35 +0200 Subject: [PATCH 1/4] move ready event to Spawner class. Implemented as hook like pre_spawn_hook --- jupyterhub/apihandlers/users.py | 30 ++++++++++++++++++++++-------- jupyterhub/spawner.py | 22 ++++++++++++++++++++++ 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/jupyterhub/apihandlers/users.py b/jupyterhub/apihandlers/users.py index 01356eb0..4423ecdc 100644 --- a/jupyterhub/apihandlers/users.py +++ b/jupyterhub/apihandlers/users.py @@ -2,6 +2,7 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. import asyncio +import inspect import json from datetime import datetime, timedelta, timezone @@ -710,19 +711,31 @@ class SpawnProgressAPIHandler(APIHandler): # - spawner not running at all # - spawner failed # - spawner pending start (what we expect) - url = url_path_join(user.url, url_escape_path(server_name), '/') - ready_event = { - 'progress': 100, - 'ready': True, - 'message': f"Server ready at {url}", - 'html_message': 'Server ready at {0}'.format(url), - 'url': url, - } failed_event = {'progress': 100, 'failed': True, 'message': "Spawn failed"} + async def get_ready_event(): + url = url_path_join(user.url, url_escape_path(server_name), '/') + ready_event = original_ready_event = { + 'progress': 100, + 'ready': True, + 'message': f"Server ready at {url}", + 'html_message': 'Server ready at {0}'.format(url), + 'url': url, + } + if spawner.progress_ready_hook: + try: + ready_event = spawner.progress_ready_hook(spawner, ready_event) + if inspect.isawaitable(ready_event): + ready_event = await ready_event + except Exception as e: + self.log.exception(f"Error in ready_event hook: {e}") + ready_event = original_ready_event + return ready_event + if spawner.ready: # spawner already ready. Trigger progress-completion immediately self.log.info("Server %s is already started", spawner._log_name) + ready_event = await get_ready_event() await self.send_event(ready_event) return @@ -766,6 +779,7 @@ class SpawnProgressAPIHandler(APIHandler): if spawner.ready: # spawner is ready, signal completion and redirect self.log.info("Server %s is ready", spawner._log_name) + ready_event = await get_ready_event() await self.send_event(ready_event) else: # what happened? Maybe spawn failed? diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 584f5c45..85b4b2a1 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -840,6 +840,28 @@ class Spawner(LoggingConfigurable): """, ).tag(config=True) + progress_ready_hook = Any( + help=""" + An optional hook function that you can implement to modify the + ready event, which will be shown to the user once a service + is ready. + + This can be set independent of any concrete spawner implementation. + + This maybe a coroutine. + + Example:: + + from subprocess import check_call + async def my_ready_hook(spawner, ready_event): + ready_event["html_message"] = f"Server {spawner.name} is ready for {spawner.user.name}" + return ready_event + + c.Spawner.progress_ready_hook = my_ready_hook + + """ + ).tag(config=True) + pre_spawn_hook = Any( help=""" An optional hook function that you can implement to do some From e477756f2726668388bdf42f15ecc9166b2afbb6 Mon Sep 17 00:00:00 2001 From: Tim Kreuzer Date: Mon, 22 May 2023 16:41:15 +0200 Subject: [PATCH 2/4] use deepcopy, to use the original_ready_event in case of an exception. Otherwise changes made before the exception would not have been reverted --- jupyterhub/apihandlers/users.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jupyterhub/apihandlers/users.py b/jupyterhub/apihandlers/users.py index 4423ecdc..89f3fff0 100644 --- a/jupyterhub/apihandlers/users.py +++ b/jupyterhub/apihandlers/users.py @@ -4,6 +4,7 @@ import asyncio import inspect import json +from copy import deepcopy from datetime import datetime, timedelta, timezone from async_generator import aclosing @@ -715,13 +716,14 @@ class SpawnProgressAPIHandler(APIHandler): async def get_ready_event(): url = url_path_join(user.url, url_escape_path(server_name), '/') - ready_event = original_ready_event = { + ready_event = { 'progress': 100, 'ready': True, 'message': f"Server ready at {url}", 'html_message': 'Server ready at {0}'.format(url), 'url': url, } + original_ready_event = deepcopy(ready_event) if spawner.progress_ready_hook: try: ready_event = spawner.progress_ready_hook(spawner, ready_event) From 52d070835ffcf2a3f3bd560f164ae4a41b8ca54d Mon Sep 17 00:00:00 2001 From: Tim Kreuzer Date: Mon, 22 May 2023 16:41:30 +0200 Subject: [PATCH 3/4] add tests for Spawner.progress_ready_hook --- jupyterhub/tests/test_api.py | 86 ++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index 00336b81..d3aa28e2 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -1110,6 +1110,92 @@ async def test_progress_ready(request, app): assert evt['url'] == app_user.url +async def test_progress_ready_hook_async_func(request, app): + """Test progress ready hook in Spawner class with an async function""" + db = app.db + name = 'saga' + app_user = add_user(db, app=app, name=name) + html_message = 'customized html message' + spawner = app_user.spawner + + async def custom_progress_ready_hook(spawner, ready_event): + ready_event['html_message'] = html_message + return ready_event + + spawner.progress_ready_hook = custom_progress_ready_hook + r = await api_request(app, 'users', name, 'server', method='post') + r.raise_for_status() + r = await api_request(app, 'users', name, 'server/progress', stream=True) + r.raise_for_status() + request.addfinalizer(r.close) + assert r.headers['content-type'] == 'text/event-stream' + ex = async_requests.executor + line_iter = iter(r.iter_lines(decode_unicode=True)) + evt = await ex.submit(next_event, line_iter) + assert evt['progress'] == 100 + assert evt['ready'] + assert evt['url'] == app_user.url + assert evt['html_message'] == html_message + + +async def test_progress_ready_hook_sync_func(request, app): + """Test progress ready hook in Spawner class with a sync function""" + db = app.db + name = 'saga' + app_user = add_user(db, app=app, name=name) + html_message = 'customized html message' + spawner = app_user.spawner + + def custom_progress_ready_hook(spawner, ready_event): + ready_event['html_message'] = html_message + return ready_event + + spawner.progress_ready_hook = custom_progress_ready_hook + r = await api_request(app, 'users', name, 'server', method='post') + r.raise_for_status() + r = await api_request(app, 'users', name, 'server/progress', stream=True) + r.raise_for_status() + request.addfinalizer(r.close) + assert r.headers['content-type'] == 'text/event-stream' + ex = async_requests.executor + line_iter = iter(r.iter_lines(decode_unicode=True)) + evt = await ex.submit(next_event, line_iter) + assert evt['progress'] == 100 + assert evt['ready'] + assert evt['url'] == app_user.url + assert evt['html_message'] == html_message + + +async def test_progress_ready_hook_async_func_exception(request, app): + """Test progress ready hook in Spawner class with an exception in + an async function + """ + db = app.db + name = 'saga' + app_user = add_user(db, app=app, name=name) + html_message = 'Server ready at {0}'.format(app_user.url) + spawner = app_user.spawner + + async def custom_progress_ready_hook(spawner, ready_event): + ready_event["html_message"] = "." + raise Exception() + + spawner.progress_ready_hook = custom_progress_ready_hook + r = await api_request(app, 'users', name, 'server', method='post') + r.raise_for_status() + r = await api_request(app, 'users', name, 'server/progress', stream=True) + r.raise_for_status() + request.addfinalizer(r.close) + assert r.headers['content-type'] == 'text/event-stream' + ex = async_requests.executor + line_iter = iter(r.iter_lines(decode_unicode=True)) + evt = await ex.submit(next_event, line_iter) + assert evt['progress'] == 100 + assert evt['ready'] + assert evt['url'] == app_user.url + assert evt['html_message'] == html_message + + async def test_progress_bad(request, app, bad_spawn): """Test progress API when spawner has already failed""" db = app.db From 262a831af8e8c557628c6370d42e55e26aa0ed99 Mon Sep 17 00:00:00 2001 From: Tim Kreuzer Date: Tue, 13 Jun 2023 08:44:50 +0200 Subject: [PATCH 4/4] use dict.copy() instead of deepcopy, improve docstrings --- jupyterhub/apihandlers/users.py | 3 +-- jupyterhub/spawner.py | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/jupyterhub/apihandlers/users.py b/jupyterhub/apihandlers/users.py index 89f3fff0..eb35a13f 100644 --- a/jupyterhub/apihandlers/users.py +++ b/jupyterhub/apihandlers/users.py @@ -4,7 +4,6 @@ import asyncio import inspect import json -from copy import deepcopy from datetime import datetime, timedelta, timezone from async_generator import aclosing @@ -723,7 +722,7 @@ class SpawnProgressAPIHandler(APIHandler): 'html_message': 'Server ready at {0}'.format(url), 'url': url, } - original_ready_event = deepcopy(ready_event) + original_ready_event = ready_event.copy() if spawner.progress_ready_hook: try: ready_event = spawner.progress_ready_hook(spawner, ready_event) diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 85b4b2a1..99faf081 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -843,7 +843,7 @@ class Spawner(LoggingConfigurable): progress_ready_hook = Any( help=""" An optional hook function that you can implement to modify the - ready event, which will be shown to the user once a service + ready event, which will be shown to the user on the spawn progress page when their server is ready. This can be set independent of any concrete spawner implementation. @@ -852,7 +852,6 @@ class Spawner(LoggingConfigurable): Example:: - from subprocess import check_call async def my_ready_hook(spawner, ready_event): ready_event["html_message"] = f"Server {spawner.name} is ready for {spawner.user.name}" return ready_event