Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes with JupyterHub 2.3.1 #45

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Fixes with JupyterHub 2.3.1 #45

wants to merge 5 commits into from

Conversation

kmpaul
Copy link

@kmpaul kmpaul commented Jun 13, 2022

This PR includes changes I made in my fork to get SSHSpawner to work with JupyterHub 2.3.1 in my personal Docker-based testing environment. There are not many source-code changes, so I will describe them all:

  • Simplify since ip and port always required now: This is a simplification of the get_port.py script. Since the latest versions of JupyterHub seem to require both ip and port be specified (and in that order), I just simplified the script to always return the (ip, port) tuple. However, maybe we want backwards compatibility, in which case this should probably be reverted/modified.

  • Change default IP for all SSHSpawners: By default, JupyterHub Spawners have a default ip of 127.0.0.1, which will not work for any remote singleuser instance. According to the JupyterHub documentation, remote Spawners should use an ip value of 0.0.0.0. I cannot think of an instance where an SSHSpawner would not be remote (otherwise why not just use LocalProcessSpawner?), so I've changed the default ip value to 0.0.0.0.

  • Always set the port: SSHSpawner relies on the separate get_port.py script to determine the port on the remote system to listen on, which means the remote port number needs to be passed to the singleuser instance. So, I figure you probably always want the --port option set.

  • Wrap all env vars in quotes to ensure JSON encoding & Need single quote to wrap JSON: There are JUPYTERHUB_* environment variables sent to the spawned process that contain JSON data, and the previous setting/exporting of those environment variables in SSHSpawner's bash run script did not encase this JSON in quotes, just leading to improperly set environment variables. For safety's sake, I have wrapped all of the environment variables used in the SSHSpawner run script with single quotes. Something more sophisticated may be needed if this is not always sufficient.

In additiion to these source-code changes, I had to make one additions to the jupyterhub_config.py file to allow SSHSpawner to work. This change was:

c.SSHSpawner.hub_api_url = f"http://{c.JupyterHub.hub_connect_ip}:8081/hub/api"

I am not sure if this suggests a modification needs to be made to SSHSpawner that I have not made. It seems to me that there should be an automated was of constructing the SSHSpawner's hub_api_url from information already known. I just haven't investigated this, yet.

I completely understand if these changes are not what you want implemented in SSHSpawner. I just wanted to share them with you to demonstrate what I needed to do to get it to work with JupyterHub 2.3.1 (latest version).

@WernerFS
Copy link

Hi, I tested these changes and they work fine. I would also like to request a merge, but my changes depend on these commits. Could some one approve it, please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants