From c409160141a8d3c80ec1dd106de31574726f9990 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Wed, 11 Mar 2020 10:06:10 -0700 Subject: [PATCH] Disable cache_ports (#790) The new jupyter_client cache_ports attribute should be disabled since this doesn't apply to remote kernels and is not compatible with the port-range support added by EG. As a result, its best to just unconditionally disable the functionality. Resolves #786 --- .../services/kernels/remotemanager.py | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/enterprise_gateway/services/kernels/remotemanager.py b/enterprise_gateway/services/kernels/remotemanager.py index d989d4f66..475a6a323 100644 --- a/enterprise_gateway/services/kernels/remotemanager.py +++ b/enterprise_gateway/services/kernels/remotemanager.py @@ -216,6 +216,14 @@ def __init__(self, **kwargs): self.user_overrides = {} self.restarting = False # need to track whether we're in a restart situation or not + # If this instance supports port caching, then disable cache_ports since we don't need this + # for remote kernels and it breaks the ability to support port ranges for local kernels (which + # is viewed as more imporant for EG). + # Note: This check MUST remain in this method since cache_ports is used immediately + # following construction. + if hasattr(self, "cache_ports"): + self.cache_ports = False + def start_kernel(self, **kwargs): """Starts a kernel in a separate process. @@ -227,13 +235,7 @@ def start_kernel(self, **kwargs): keyword arguments that are passed down to build the kernel_cmd and launching the kernel (e.g. Popen kwargs). """ - - process_proxy = get_process_proxy_config(self.kernel_spec) - process_proxy_class_name = process_proxy.get('class_name') - self.log.debug("Instantiating kernel '{}' with process proxy: {}". - format(self.kernel_spec.display_name, process_proxy_class_name)) - process_proxy_class = import_item(process_proxy_class_name) - self.process_proxy = process_proxy_class(kernel_manager=self, proxy_config=process_proxy.get('config')) + self._get_process_proxy() self._capture_user_overrides(**kwargs) super(RemoteKernelManager, self).start_kernel(**kwargs) @@ -398,7 +400,7 @@ def write_connection_file(self): """ if (isinstance(self.process_proxy, LocalProcessProxy) or not self.response_address) and not self.restarting: # However, since we *may* want to limit the selected ports, go ahead and get the ports using - # the process proxy (will be LocalPropcessProxy for default case) since the port selection will + # the process proxy (will be LocalProcessProxy for default case) since the port selection will # handle the default case when the member ports aren't set anyway. ports = self.process_proxy.select_ports(5) self.shell_port = ports[0] @@ -408,3 +410,16 @@ def write_connection_file(self): self.control_port = ports[4] return super(RemoteKernelManager, self).write_connection_file() + + def _get_process_proxy(self): + """Reads the associated kernelspec and to see if has a process proxy stanza. + If one exists, it instantiates an instance. If a process proxy is not + specified in the kernelspec, a LocalProcessProxy stanza is fabricated and + instantiated. + """ + process_proxy_cfg = get_process_proxy_config(self.kernel_spec) + process_proxy_class_name = process_proxy_cfg.get('class_name') + self.log.debug("Instantiating kernel '{}' with process proxy: {}". + format(self.kernel_spec.display_name, process_proxy_class_name)) + process_proxy_class = import_item(process_proxy_class_name) + self.process_proxy = process_proxy_class(kernel_manager=self, proxy_config=process_proxy_cfg.get('config'))