define Spawner.delete_forever on base Spawner

instead of on the test class

and fix the logic for when it is called a bit:

- call on *all* Spawners, not just the default
- call on named server deletion when remove=True

jupyterhub/apihandlers/users.py

@@ -242,11 +242,7 @@ class UserAPIHandler(APIHandler):
 
         await maybe_future(self.authenticator.delete_user(user))
 
-        # allow the spawner to cleanup any persistent resources associated with the user
-        try:
-            await user.spawner.delete_forever()
-        except Exception as e:
-            self.log.error("Error cleaning up persistent resources: %s" % e)
+        await user.delete_spawners()
 
         # remove from registry
         self.users.delete(user)
@@ -488,10 +484,18 @@ class UserServerAPIHandler(APIHandler):
         options = self.get_json_body()
         remove = (options or {}).get('remove', False)
 
-        def _remove_spawner(f=None):
-            if f and f.exception():
-                return
+        async def _remove_spawner(f=None):
+            """Remove the spawner object
+
+            only called after it stops successfully
+            """
+            if f:
+                # await f, stop on error,
+                # leaving resources in the db in case of failure to stop
+                await f
             self.log.info("Deleting spawner %s", spawner._log_name)
+            await maybe_future(user._delete_spawner(spawner))
+
             self.db.delete(spawner.orm_spawner)
             user.spawners.pop(server_name, None)
             self.db.commit()
@@ -512,7 +516,8 @@ class UserServerAPIHandler(APIHandler):
             self.set_header('Content-Type', 'text/plain')
             self.set_status(202)
             if remove:
-                spawner._stop_future.add_done_callback(_remove_spawner)
+                # schedule remove when stop completes
+                asyncio.ensure_future(_remove_spawner(spawner._stop_future))
             return
 
         if spawner.pending:
@@ -530,9 +535,10 @@ class UserServerAPIHandler(APIHandler):
 
         if remove:
             if stop_future:
-                stop_future.add_done_callback(_remove_spawner)
+                # schedule remove when stop completes
+                asyncio.ensure_future(_remove_spawner(spawner._stop_future))
             else:
-                _remove_spawner()
+                await _remove_spawner()
 
         status = 202 if spawner._stop_pending else 204
         self.set_header('Content-Type', 'text/plain')

jupyterhub/spawner.py

@@ -1141,6 +1141,18 @@ class Spawner(LoggingConfigurable):
         """
         raise NotImplementedError("Override in subclass. Must be a coroutine.")
 
+    def delete_forever(self):
+        """Called when a user or server is deleted.
+
+        This can do things like request removal of resources such as persistent storage.
+        Only called on stopped spawners, and is usually the last action ever taken for the user.
+
+        Will only be called once on each Spawner, immediately prior to removal.
+
+        Stopping a server does *not* call this method.
+        """
+        pass
+
     def add_poll_callback(self, callback, *args, **kwargs):
         """Add a callback to fire when the single-user server stops"""
         if args or kwargs:

jupyterhub/tests/mocking.py

@@ -93,16 +93,6 @@ class MockSpawner(SimpleLocalProcessSpawner):
     def _cmd_default(self):
         return [sys.executable, '-m', 'jupyterhub.tests.mocksu']
 
-    async def delete_forever(self):
-        """Called when a user is deleted.
-
-        This can do things like request removal of resources such as persistent storage.
-        Only called on stopped spawners, and is likely the last action ever taken for the user.
-
-        Will only be called once on the user's default Spawner.
-        """
-        pass
-
     use_this_api_token = None
 
     def start(self):

jupyterhub/user.py

@@ -252,6 +252,35 @@ class User:
                 await self.save_auth_state(auth_state)
         return auth_state
 
+    async def delete_spawners(self):
+        """Call spawner cleanup methods
+
+        Allows the spawner to cleanup persistent resources
+        """
+        for name in self.orm_user.orm_spawners.keys():
+            await self._delete_spawner(name)
+
+    async def _delete_spawner(self, name_or_spawner):
+        """Delete a single spawner"""
+        # always ensure full Spawner
+        # this may instantiate the Spawner if it wasn't already running,
+        # just to delete it
+        if isinstance(name_or_spawner, str):
+            spawner = self.spawners[name_or_spawner]
+        else:
+            spawner = name_or_spawner
+
+        if spawner.active:
+            raise RuntimeError(
+                f"Spawner {spawner._log_name} is active and cannot be deleted."
+            )
+        try:
+            await maybe_future(spawner.delete_forever())
+        except Exception as e:
+            self.log.exception(
+                f"Error cleaning up persistent resources on {spawner._log_name}"
+            )
+
     def all_spawners(self, include_default=True):
         """Generator yielding all my spawners