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

Add support for JupyterLab instances #133

Closed
wants to merge 3 commits into from

Conversation

jamie-arcc
Copy link

Since JupyterHub is capable of handling both JupyterLab instances as well as Notebook instances, I have added support here for launching JupyterLab instances in batchspawner-singleuser. It should not affect current configurations that are just launching notebooks by default. There is probably a better way of handling the app selection, but for now I have it so that one can choose the lab instance by passing 'lab' on the command line to batchspawner-singleuser. Doing it this way instead of with a configurable should make it so that those using wrap/profile spawners can have profiles for JupyterLab and Jupyter notebook that can be chosen on the fly.

@rkdarst
Copy link
Contributor

rkdarst commented Feb 6, 2019

Thanks! I was wondering how to do this myself. It would be nice to not have to duplicate all the logic to handle {batchspawner, non-batchspawner} × {notebook, lab}. But I guess there's not much solution now, and we should just do what we can.

I tested live with latest JH master and it works.

Test failures seem the same that everything else has, and unrelated to this.

@jamie-arcc
Copy link
Author

is there anything I need to do?

@mbmilligan
Copy link
Member

Thanks for this contribution! I agree with @rkdarst here though, I'm not wild about the compounding complexity we've introduced here.

For what it's worth, this sort of case is why I was hesitant about shipping our own version of the jupyterhub-singleuser script just to enable the port selection logic. In hindsight, I think a better approach would have been to create a wrapper script that does the port negotiation before passing control to whatever singleuser implementation the batch script invokes. Such a wrapper does risk a race condition, since it would have to allocate a port, free it, and then tell the child process to use that port, with the possibility that another process grabs that port in the interim. But that would allow us to adapt to any underlying script that takes the standard Jupyter singleuser script arguments.

At minimum, I think we should put back the random port selection in batchspawner so there is fallback logic if the other end doesn't want to use the new remote port API. (Not, I would emphasize, a failover - that would take us down a rabbit hole of timeouts and recovery scenarios. But a configurable to tell batchspawner to use the old behavior, and not expect a remote port API call.)

@rkdarst
Copy link
Contributor

rkdarst commented Jun 16, 2019

I think #141 is being used instead of this, because it operates as a wrapper and doesn't need n^2 options for everything. Check it out and let us know what you think.

@mbmilligan
Copy link
Member

#141 has been merged, closing this one.

@mbmilligan mbmilligan closed this Jun 18, 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