Rework manager based on PR comments

Do not instantiate manager in the file anymore. This will be done
during extension loading

Rework monitoring for proxy apps natively using asyncio. This will be
added as callback to ServerApp IO loop during extension loading

Manager spits out logs in debug mode when there is proxy app is
added/removed from it.

Added uni_socket to server proxy app dataclass

Remove unnecessary async methods

Properly handle the get_server_proxy_{app,proc} methods when server
proxy is not found in the manager. We return a default ServerProxyApp
tuple in this case with empty values

Author: mahendrapaipuri

jupyter_server_proxy/config.py

@@ -11,9 +11,11 @@
     from importlib.metadata import entry_points
 
 from jupyter_server.utils import url_path_join as ujoin
-from traitlets import Dict, List, Tuple, Union, default, observe
+from traitlets import Dict, List, Tuple, Union, Int, default, observe
 from traitlets.config import Configurable
 
+from jupyter_server.utils import url_path_join as ujoin
+
 from .handlers import AddSlashHandler, NamedLocalProxyHandler, SuperviseAndProxyHandler
 
 try:
@@ -102,15 +104,15 @@ def get_entrypoint_server_processes(serverproxy_config):
     return sps
 
 
-def make_handlers(base_url, server_processes):
+def make_handlers(base_url, manager, server_processes):
     """
     Get tornado handlers for registered server_processes
     """
     handlers = []
     for sp in server_processes:
         if sp.command:
             handler = _make_supervisedproxy_handler(sp)
-            kwargs = dict(state={})
+            kwargs = dict(state={}, manager=manager)
         else:
             if not (sp.port or isinstance(sp.unix_socket, str)):
                 warn(
@@ -341,3 +343,13 @@ def _host_whitelist_deprecated(self, change):
                 )
             )
             self.host_allowlist = change.new
+
+    monitor_interval = Int(
+        10,
+        help="""
+        Proxy polling interval in seconds. The server proxy manager will keep
+        polling the status of running servers with a frequency set by this
+        interval.
+        """,
+        config=True
+    )

jupyter_server_proxy/manager.py

@@ -1,48 +1,57 @@
 """Manager for jupyter server proxy"""
 
-from collections import namedtuple
-
-from jupyter_server.utils import url_path_join as ujoin
-from tornado.ioloop import PeriodicCallback
-
-from .utils import check_pid
-
-ServerProxy = namedtuple("ServerProxy", ["name", "url", "cmd", "port", "managed"])
-ServerProxyProc = namedtuple("ServerProxyProc", ["name", "proc"])
+import asyncio
 
+from collections import namedtuple
 
-async def monitor_server_proxy_procs():
-    """Perodically monitor the server proxy processes. If user terminates
-    the process outside of jupyter-server-proxy, we should be able to
-    capture that and remove proxy app from manager"""
-    # Get current active apps
-    procs = manager._list_server_proxy_procs()
+from traitlets import List, Int
+from traitlets.config import LoggingConfigurable
 
-    # Check if all pids are alive
-    for proc in procs:
-        exists = check_pid(proc.proc.pid)
-        if not exists:
-            await manager.del_server_proxy_app(proc.name)
+from jupyter_server.utils import url_path_join as ujoin
 
 
-class ServerProxyAppManager:
+ServerProxy = namedtuple(
+    "ServerProxy",
+    [
+        "name",
+        "url",
+        "cmd",
+        "port",
+        "managed",
+        "unix_socket"
+    ],
+    defaults=[""] * 6
+)
+ServerProxyProc = namedtuple(
+    "ServerProxyProc",
+    [
+        "name",
+        "proc"
+    ],
+    defaults=[""] * 2
+)
+
+
+class ServerProxyAppManager(LoggingConfigurable):
     """
     A class for listing and stopping server proxies that are started
     by jupyter server proxy.
     """
 
-    def __init__(self):
-        """Initialize the server proxy manager"""
-        # List of server proxy apps
-        self.server_proxy_apps = []
+    server_proxy_apps = List(
+        help="List of server proxy apps"
+    )
 
-        # List of server proxy app proc objects. For internal use only
-        self._server_proxy_procs = []
+    _server_proxy_procs = List(
+        help="List of server proxy app proc objects"
+    )
 
-        # Total number of currently running proxy apps
-        self.num_active_server_proxy_apps = 0
+    num_active_server_proxy_apps = Int(
+        0,
+        help="Total number of currently running proxy apps"
+    )
 
-    async def add_server_proxy_app(self, name, base_url, cmd, port, proc):
+    def add_server_proxy_app(self, name, base_url, cmd, port, proc, unix_socket):
         """Add a launched proxy server to list"""
         self.num_active_server_proxy_apps += 1
 
@@ -54,6 +63,7 @@ async def add_server_proxy_app(self, name, base_url, cmd, port, proc):
                 cmd=" ".join(cmd),
                 port=port,
                 managed=True if proc else False,
+                unix_socket=unix_socket if unix_socket is not None else ''
             )
         )
 
@@ -65,24 +75,25 @@ async def add_server_proxy_app(self, name, base_url, cmd, port, proc):
                 proc=proc,
             )
         )
+        self.log.debug("Server proxy %s added to server proxy manager" % name)
 
-    async def del_server_proxy_app(self, name):
+    def del_server_proxy_app(self, name):
         """Remove a launched proxy server from list"""
         self.server_proxy_apps = [
             app for app in self.server_proxy_apps if app.name != name
         ]
         self._server_proxy_procs = [
             app for app in self._server_proxy_procs if app.name != name
         ]
-        self.num_active_server_proxy_apps -= 1
+        self.num_active_server_proxy_apps = len(self.server_proxy_apps)
 
     def get_server_proxy_app(self, name):
         """Get a given server proxy app"""
-        return next((app for app in self.server_proxy_apps if app.name == name), {})
+        return next((app for app in self.server_proxy_apps if app.name == name), ServerProxy())
 
     def _get_server_proxy_proc(self, name):
         """Get a given server proxy app"""
-        return next((app for app in self._server_proxy_procs if app.name == name), {})
+        return next((app for app in self._server_proxy_procs if app.name == name), ServerProxyProc())
 
     def list_server_proxy_apps(self):
         """List all active server proxy apps"""
@@ -107,21 +118,32 @@ async def terminate_server_proxy_app(self, name):
             await app.proc.terminate()
 
             # Remove proxy app from list
-            await self.del_server_proxy_app(name)
+            self.del_server_proxy_app(name)
+
+            self.log.debug("Server proxy %s removed from server proxy manager" % name)
 
             return True
         except (KeyError, AttributeError):
+            self.log.warning("Server proxy %s not found in server proxy manager" % name)
             return None
 
     async def terminate_all(self):
         """Close all server proxy and cleanup"""
         for app in self.server_proxy_apps:
             await self.terminate_server_proxy_app(app)
 
-
-# Create a default manager to keep track of server proxy apps.
-manager = ServerProxyAppManager()
-
-# Create a Periodic call back function to check the status of processes
-pc = PeriodicCallback(monitor_server_proxy_procs, 1e4)
-pc.start()
+    async def monitor(self, monitor_interval):
+        while True:
+            procs = self._list_server_proxy_procs()
+
+            # Check if processes are running
+            for proc in procs:
+                running = proc.proc.running
+                if not running:
+                    self.log.warning(
+                        "Server proxy %s is not running anymore. "
+                        "Removing from server proxy manager" % proc.name
+                    )
+                    self.del_server_proxy_app(proc.name)
+
+            await asyncio.sleep(monitor_interval)

jupyter_server_proxy/utils.py

@@ -51,13 +51,3 @@ def validate(self, obj, value):
             return value
         else:
             self.error(obj, value)
-
-
-def check_pid(pid):
-    """Check For the existence of a unix pid"""
-    try:
-        os.kill(pid, 0)
-    except OSError:
-        return False
-    else:
-        return True