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

Added ability to connect to pre-existing kernel #60

Closed
wants to merge 1 commit into from

Conversation

R0bk
Copy link

@R0bk R0bk commented May 13, 2023

Hey there, this is a really neat library thanks for writing it.

I've been working on an app that needs to connect to a preexisting kernel instead of starting a new one on connect. I've made a small modification to allow a prop to pass through to your Kernal wrapper, search for pre running kernels and select them.

To be honest I'm not that happy with the implementation, as it seems your code is aligned with the standard Jupyter codebase, and this does seem to deviate from that. So other than asking to bring in this prop, I wanted to ask is there a better way to approach this?

@echarles
Copy link
Member

Thanks a lot for your contribution.

I have looked at the changes. I understand the intent, and I wonder how const runningModel = sessionManager.running().next().value; will get populated. Have you tested it and found that the sessionManager already had running kernels?

@Mako-L
Copy link

Mako-L commented May 13, 2023

Huh?

@echarles
Copy link
Member

echarles commented May 13, 2023

There is something indeed to be done as the sessionManager.running() can already be populated. This is the case e.g. when you reload the page and do not stop the server. The server keeps the previous kernel launch opened, and as we can a new random uuid as path and name when we start the kernel, we do not reuse the previous one.

This drive to interesting discussion on who own the kernel and how to identify the connection and the session...

@R0bk I am more than happy to think more about this and come with alternatives to your PR, or comment it to get that merged. I would love to get more inputs maybe on your case and what goal you are aiming. What problem have you encountered that brought this PR?

@R0bk
Copy link
Author

R0bk commented May 13, 2023

To help explain my use case. It is to assist with machine learning interpretability, which requires us to load large models, and perform slow calculations on them before we will have the ability to do research. Only once these steps are done then can we really start interacting with the models.

For this use case we are using Jupyter-UI as a client side react component in our web application. It would be very beneficial for the app if we could do two things. 1. Start the Kernel beforehand so we do not have to wait once the page is loaded. 2. Persist the kernels between reloads and rebuilds of the application.

With regards to your first question I have tested with our application and also with your create-react-app example doing the following steps.

  1. Disabling the Jupyter launch script
  2. Launching Jupyter server separately
  3. Executing a script to start a Kernel in Jupyter Server and do model preprocessing
  4. Launch our app or your cra with selectRunningKernel set to true

And it successfully loads the prior kernel without launching any new ones.

We are often only running one model per machine (and hence 1 kernel) so just selecting the first kernel has been ok for this application. But I believe a better method in general would be to select the kernel via either the IModel id or via some filter on the IModel properties, if no kernel can be found then start a new one. (This would be assuming the user provided the component with an Id or an IModel property filter of some kind).

I hope this give enough information but please ask if I can provide any more.

@echarles
Copy link
Member

echarles commented May 15, 2023

@R0bk I get your case. I have just pushed 356dd2d which is inspired by this PR. It adds useRunningKernelIndex and useRunningKernelId props. In your case, you would have to use useRunningKernelIndex={0}.

If you know the kernelId, you can use it. At some point, I want to make it easier to inspect, manage and connect to existing kernels hosted on local or remote Jupyter servers, so your case is even more supported.

Tell me what you think (and sorry, my last commit conflicts with this PR, I hope it gets the intent).

Once OK for you, I can cut a new release.

@echarles
Copy link
Member

@R0bk Just released @datalayer/jupyter-react 0.4.0 which runs on JupyterLab 4 final. Looking forward feedback on the additional props to document them, or to update to fit your use case.

@R0bk
Copy link
Author

R0bk commented May 17, 2023

@echarles This looks awesome, thank you!

I'll shift to 0.4.0 final today, give it a try and post back with results.

@echarles
Copy link
Member

Closing. Please open an new issue/PR if needed.

@echarles echarles closed this Jun 12, 2023
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.

None yet

3 participants