Merge pull request #4844 from minrk/allow-stop-during-start

allow stop while start is pending

jupyterhub/apihandlers/users.py

@@ -694,14 +694,22 @@ class UserServerAPIHandler(APIHandler):
                 asyncio.ensure_future(_remove_spawner(spawner._stop_future))
             return
 
-        if spawner.pending:
-            raise web.HTTPError(
-                400,
-                f"{spawner._log_name} is pending {spawner.pending}, please wait",
-            )
-
         stop_future = None
-        if spawner.ready:
+        if spawner.pending:
+            # we are interrupting a pending start
+            # hopefully nothing gets leftover
+            self.log.warning(
+                f"Interrupting spawner {spawner._log_name}, pending {spawner.pending}"
+            )
+            spawn_future = spawner._spawn_future
+            if spawn_future:
+                spawn_future.cancel()
+            # Give cancel a chance to resolve?
+            # not sure what we would wait for here,
+            await asyncio.sleep(1)
+            stop_future = await self.stop_single_user(user, server_name)
+
+        elif spawner.ready:
             # include notify, so that a server that died is noticed immediately
             status = await spawner.poll_and_notify()
             if status is None:
@@ -837,7 +845,9 @@ class SpawnProgressAPIHandler(APIHandler):
             # not pending, no progress to fetch
             # check if spawner has just failed
             f = spawn_future
-            if f and f.done() and f.exception():
+            if f and f.cancelled():
+                failed_event['message'] = "Spawn cancelled"
+            elif f and f.done() and f.exception():
                 exc = f.exception()
                 message = getattr(exc, "jupyterhub_message", str(exc))
                 failed_event['message'] = f"Spawn failed: {message}"
@@ -876,7 +886,9 @@ class SpawnProgressAPIHandler(APIHandler):
         else:
             # what happened? Maybe spawn failed?
             f = spawn_future
-            if f and f.done() and f.exception():
+            if f and f.cancelled():
+                failed_event['message'] = "Spawn cancelled"
+            elif f and f.done() and f.exception():
                 exc = f.exception()
                 message = getattr(exc, "jupyterhub_message", str(exc))
                 failed_event['message'] = f"Spawn failed: {message}"

jupyterhub/tests/test_api.py

@@ -1558,23 +1558,20 @@ async def test_start_stop_race(app, no_patience, slow_spawn):
     r = await api_request(app, 'users', user.name, 'server', method='post')
     assert r.status_code == 202
     assert spawner.pending == 'spawn'
+    spawn_future = spawner._spawn_future
     # additional spawns while spawning shouldn't trigger a new spawn
     with mock.patch.object(spawner, 'start') as m:
         r = await api_request(app, 'users', user.name, 'server', method='post')
     assert r.status_code == 202
     assert m.call_count == 0
 
-    # stop while spawning is not okay
-    r = await api_request(app, 'users', user.name, 'server', method='delete')
-    assert r.status_code == 400
-    while not spawner.ready:
-        await asyncio.sleep(0.1)
-
+    # stop while spawning is okay now
     spawner.delay = 3
-    # stop the spawner
     r = await api_request(app, 'users', user.name, 'server', method='delete')
     assert r.status_code == 202
     assert spawner.pending == 'stop'
+    assert spawn_future.cancelled()
+    assert spawner._spawn_future is None
     # make sure we get past deleting from the proxy
     await asyncio.sleep(1)
     # additional stops while stopping shouldn't trigger a new stop

jupyterhub/user.py

@@ -1,13 +1,13 @@
 # Copyright (c) Jupyter Development Team.
 # Distributed under the terms of the Modified BSD License.
+import asyncio
 import json
 import warnings
 from collections import defaultdict
-from datetime import timedelta
 from urllib.parse import quote, urlparse, urlunparse
 
 from sqlalchemy import inspect
-from tornado import gen, web
+from tornado import web
 from tornado.httputil import urlencode
 from tornado.log import app_log
 
@@ -912,9 +912,13 @@ class User:
                 spawner.cert_paths = await maybe_future(spawner.move_certs(hub_paths))
             self.log.debug("Calling Spawner.start for %s", spawner._log_name)
             f = maybe_future(spawner.start())
-            # commit any changes in spawner.start (always commit db changes before yield)
+            # commit any changes in spawner.start (always commit db changes before await)
             db.commit()
-            url = await gen.with_timeout(timedelta(seconds=spawner.start_timeout), f)
+            # gen.with_timeout protects waited-for tasks from cancellation,
+            # whereas wait_for cancels tasks that don't finish within timeout.
+            # we want this task to halt if it doesn't return in the time limit.
+            await asyncio.wait_for(f, timeout=spawner.start_timeout)
+            url = f.result()
             if url:
                 # get ip, port info from return value of start()
                 if isinstance(url, str):