Skip to content

Commit

Permalink
Disable cache_ports (#790)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kevin-bates authored Mar 11, 2020
1 parent 2085894 commit c409160
Showing 1 changed file with 23 additions and 8 deletions.
31 changes: 23 additions & 8 deletions enterprise_gateway/services/kernels/remotemanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)

Expand Down Expand Up @@ -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]
Expand All @@ -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'))

0 comments on commit c409160

Please sign in to comment.