Add support for local live kernels #9854
Conversation
| @@ -68,6 +68,7 @@ export class EmptyNotebookCellLanguageService implements IExtensionSingleActivat | |||
| break; | |||
| } | |||
| case 'startUsingRemoteKernelSpec': | |||
| case 'connectToLiveLocalKernel': | |||
| @@ -268,6 +297,32 @@ export class NotebookControllerManager implements INotebookControllerManager, IE | |||
| return this.controllersPromise; | |||
| } | |||
|
|
|||
| private async addLocalLiveKernelController(kernel: IKernel) { | |||
| // For now connecting to live local kernels is limited to Insiders or the pre-release channel. | |||
| if (this.appEnv.channel === 'insiders' || (await this.isPreRelease)) { | |||
There was a problem hiding this comment.
Feature available only in pre-release or vscode insiders
| @@ -1035,6 +1035,10 @@ export namespace DataScience { | |||
| 'jupyter.kernel.category.jupyterRemoteKernel', | |||
| '(Remote) Jupyter Kernel' | |||
| ); | |||
| export const kernelCategoryForLocalLiveJupyterKernel = localize( | |||
| 'jupyter.kernel.category.jupyterLocalLiveKernel', | |||
| '(Local Live) Jupyter Kernel' | |||
There was a problem hiding this comment.
New group in kernel list
Codecov Report
@@ Coverage Diff @@
## main #9854 +/- ##
====================================
- Coverage 63% 63% -1%
====================================
Files 203 203
Lines 9826 9811 -15
Branches 1564 1556 -8
====================================
- Hits 6243 6233 -10
- Misses 3079 3080 +1
+ Partials 504 498 -6
|
DonJayamanne
force-pushed
the
localLiveKernels2
branch
from
30a6539 to
e5bfed3
Compare
| const notebook = workspace.notebookDocuments.find((nb) => nb.uri.toString() === uri.toString()); | ||
| // If we're connecting to a live local kernel, then Uri will point to the Uri of the original kernel. | ||
| if (options.metadata.kind === 'connectToLiveLocalKernel') { |
There was a problem hiding this comment.
I feel like we've pushed the ownership of handling 'live' kernels up to the kernelProvider and the controller. With remote this is all handled by the kernel finder.
Seems like we could do it the same way, we just need something in the raw kernel stuff to keep track of running kernels.
There was a problem hiding this comment.
Otherwise the controller and the kernelprovider have to special case live local kernels.
There was a problem hiding this comment.
ILocalKernelFinder could return running kernels too. In fact jupyter puts them in a known spot. We just need a way to store them for raw.
There was a problem hiding this comment.
I feel like we've pushed the ownership of handling 'live' kernels up to the kernelProvider
I think this is fine, the kernels provider is responsible for providing the real kernels and that's exactly what this does.
However agree with your comments about searching for them (see below).
ILocalKernelFinder could return running kernels too
Yes, that's possible.
In fact jupyter puts them in a known spot. We just need a way to store them for raw.
Yes, i thought about that as well, but that requires bit more work, to track the connection json and exposing that and then starting a new kernel with the kernel information provided and that messes up a lot of other things. E.g. we have 2 processes but connected to the same underlying kernel and could complicate how we handle intellisense, widgets, etc. This way we just use the same IKernel and don't really start new sesions.
E.g. even in remote case, we do'nt really start new sessions, we just connect to the exact same controller from multiple notebooks & I'd prefer to do the same here as well.
There was a problem hiding this comment.
ILocalKernelFinder could return running kernels too. In f
not going to do this, as thi will then need to know about live kernels & IKernel and the like.
I.e. this will need information about what s in KernelProvider.
Also mentioned earlier about issues with using that approach to start kernels. I think that complicates things a lot & could have more issues as well.
There was a problem hiding this comment.
IKernelProvider would not be pushed down (as it isn't for IRemoteKernelFinder). Instead the raw code would have to expose a way to get the list of processes it started. Much like the jupyter session manager does.
There was a problem hiding this comment.
Actually it's more like the list of RawKernels/RawSessions (as we can't create another RawKernel to connect to the same process), but same idea as the JupyterSessions.
There was a problem hiding this comment.
For zmq, we simply need a KernelProcessManager. It returns the list of runnin
Ok, I think I understand what yoo're suggesting, I thought you wanted me to connect to the same kernel using the same old kernel-*.json connection file (which also works, but more complex).
There was a problem hiding this comment.
as we can't create another RawKernel to connect to the same process
Actually we can, & that's what I thought you were suggesting all this time.
All we need to do is start a web socket connection for the same old kernel connection instead of starting a new process..
There was a problem hiding this comment.
Hey if that works, probably even better (so that dispose stuff doesn't get confused).
DonJayamanne
force-pushed
the
localLiveKernels2
branch
from
e5bfed3 to
8a8ad1c
Compare
Part of powertoys