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

Support named servers #167

Merged
merged 3 commits into from
Nov 15, 2019
Merged

Support named servers #167

merged 3 commits into from
Nov 15, 2019

Conversation

rcthomas
Copy link
Contributor

Since v0.8, JupyterHub has supported "named servers." If c.JupyterHub.allow_named_servers=True in the hub config, servers can have names assigned to them. If this setting is False then there is only one server with the name '' (empty string).

For BatchSpawner to work with named servers, the port selected by batchspawner-singleuser must be routed to the right spawner. Right now the port is attached to user.spawner but this is really just an alias for user.spawners['']. Without this fix the port value lands on the empty-string spawner and that may not be what the user wanted to happen at all.

To tell the hub which spawner to attach the port information to, this PR has batchspawner-singleuser read the value of the environment variable JUPYTERHUB_SERVER_NAME and send it back to the hub as part of the API call. The API is modified to consume this value and use it to attach the port to the right spawner.

@rcthomas
Copy link
Contributor Author

With this fix, BatchSpawner appears to work at our center, including with WrapSpawner and named servers. It does not look like there is any issue with WrapSpawner.

@cmd-ntrf
Copy link
Contributor

You could identify the spawner based on the token value instead of sending the server name through the API.

This is what I did here : cmd-ntrf@5a5de66

I am not sure why it was never merged upstream, I might have forgotten to do a PR or it was lost in the merges.

@rcthomas
Copy link
Contributor Author

Ah that's even better. I'll rework.

@rcthomas
Copy link
Contributor Author

Modified to identify correct spawner based on API token. Decided to set None as the default spawner value that way if there is no matching token at all then the attempt to set the port fails. We could replace this with a more meaningful failure but I thought at least if we are checking the API token all the time, we should make it fail if there is no match.

@mbmilligan
Copy link
Member

Looks good to me, merging.

Sorry @cmd-ntrf if I missed a PR from you to fix this earlier!

@mbmilligan mbmilligan merged commit 68a1fcd into jupyterhub:master Nov 15, 2019
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.

3 participants