We want to share renderers in azure notebooks, and even within VS Code we can have different notebook implementations. Mimetype matching is a simple and effective way for a renderer to declare and be invoked with data it can understand, but in advanced the scenarios the renderer will want to communicate back to the kernel.
At the moment, renderers just 'assume' that the kernel they're connecting to is the one they understand. However, with different kernels available and different platforms, this cannot always hold true. Particularly, a notebook might work fine in VS Code, but there might not be a custom-written kernel for the aznb case.
Therefore we want a way for renderers to know that what protocol(s) are available, and have logic to gracefully degrade.
I initially approached this from a mechanical point of view where we would make a change in the notebook renderer API to acquire a protocol, e.g.
export interface NotebookRendererApi<T> {
/**
* Returns a handle that can be used for communication with the notebook
* kernel. It returns undefined if the protocol isn't available in the
* current context. If this happens, the renderer should gracefully degrade.
*/
acquireCommunication<TReq = unknown, TRes = any>(protocol: string): NotebookMessagingPipe<TReq, TRes> | undefined;
// ...
}
export interface NotebookMessagingPipe<TReq, TRes> {
/**
* Sends a message to the renderer extension code. Can be received in
* the `onDidReceiveMessage` event in `NotebookCommunication`.
*/
postMessage(msg: TReq): void;
/**
* Called when the renderer uses `postMessage` on the NotebookCommunication
* instance for this renderer.
*/
onDidReceiveMessage: VSCodeEvent<TRes>;
}
However this is really ideal and only solves part of the issue. It's not really sufficient to have renderers only handle a lack of a protocol, we also want them to be able to indicate which renderers work with which kernels. This entails something in the package.json contributions.
In VS Code, we have a field for extensionDependencies, but this also is not sufficient: a jupyter protocol could be exposed by multiple different kernel implementations, and the idea of extension kerns is not and is not likely to ever be portable to Azure Notebooks.
So my proposed approach for this involves three changes:
In the VS Code extension API, notebook kernels optionally say that they provide a kernel in the renderer:
export interface NotebookKernel {
providersRendererKernel?: string;
// ...
The Kernel preload would then use, for provideKernel(rendereKernelName: string, object: unknown) to provide the instance.
For example, if I set providersRendererKernel = "greeterKernel", I would call provideKernel("greeterKernel", greeterObj) in my preload.
In their package.json, renderers say that they would like a kernel
"notebookOutputRenderer": [
{
"id": "az-notebook-hello-renderers",
+ "kernel": "greeterKernel"
Then we expose the requested kernel on the render API _if_ it's available, in lieu of the existing (currently not hooked up) messaging APIs:
export interface NotebookRendererApi<TState, TKernel = any> {
/**
* If the renderer requested a "kernel" in its package.json contributions
* and it's available in the current runtime, the object is injected here.
*/
kernel?: TKernel;
I like this approach for a few reasons:
cc @vijayupadya @kjcho-msft @DonJayamanne @rebornix
Thanks @connor4312
This looks like a great start, just a couple of questions/thoughts (things u might not be aware of).
When VS Code Python, we need the kernel hooked up with the renderer even before there is anyoutput.
Today we achieve this by loading the same renderer script as part of the Kernel preload scripts.
Hence for Python extension, the handshake with the renderer happens even when any output is renderer (happens in resolveKernel method of the NotebookKernelProvider.
Doesn't seem like your current model takes that into account, else I'm mistaken and don't understand how it will work with the workflow when there is no output but kernel handshake needs to take place.
The Kernel preload would then use, for provideKernel(rendereKernelName: string, object: unknown) to provide the instance.
What is the kernel preload here? Is this a script file that is loaded by the kernel preload.
More specifically is this done in the NotebookKernel.preloads[] in VS Code extension?
If this is the case, then this means, renderers will only ever work if there is a real kernel.
However there's also the situation where users can embed ipywidgets (HTML, JS & JSON) into the output of the notebook. THis way, the notebook can be opened somewhere else and they still have ipywidget fucntionality (but limited to the data that is stored in the output).
This doesn't always work with all ipywidgest, but works with most. Its a capability of Jupyter notebooks & something We (Python extensions) don't yet support.
In shrot, we need to be able to render ipywidgets (if ipywidgets are embedded in output with all informatoin) when opening an existing notebook.
For more info about this see here https://ipywidgets.readthedocs.io/en/latest/embedding.html#save-notebook-widget-state
I think if we supported the idea of optionally requiring kernels, that would work. At least for ipywidgets.
_update_: I think my comment is pretty close to what @DonJayamanne said above.
From my understanding of how ipywidget works, whether a kernel can work seamlessly with ipywidget renderers can't be determined statically. You can get a partially working ipywidget rendering without a kernel, or a kernel/server which does not support ipywidget (meaning users didn't install widgetsnbextension nb extension). Only when the widgetsnbextension is installed, the widget rendered in the UI can talk to the kernel.
Another case that a kernel is not always necessary for rendering ipywidgets is exporting notebooks to HTML, which will store all the widget states as global states.
IMHO, our API should only provide a communication for the kernel/provider and the renderer to perform the handshake other than introducing this concept in the API.
What is the kernel preload here? Is this a script file that is loaded by the kernel preload.
More specifically is this done in the NotebookKernel.preloads[] in VS Code extension?
Yes
However there's also the situation where users can embed ipywidgets (HTML, JS & JSON) into the output of the notebook. THis way, the notebook can be opened somewhere else and they still have ipywidget fucntionality (but limited to the data that is stored in the output).
I didn't call that out explicitly, in the issue -- but that is why I put kernel?: TKernel as an optional property. If the kernel isn't available, we will still render it, and assume that the renderer falls back gracefully or displays a nice error message.
IMHO, our API should only provide a communication for the kernel/provider and the renderer to perform the handshake other than introducing this concept in the API.
If I understand you correctly, you're advocating for keeping what we have today, where the kernel can create an arbitrary global in the window and renderers look for it?
If I understand you correctly, you're advocating for keeping what we have today, where the kernel can create an arbitrary global in the window and renderers look for it?
@connor4312 yes, I prefer what we have right now. However, I do see the value of kernel provider contributing kernels into the renderer. It's what jupyter frontend does in practice when support ipywidgets, by building a ipywidget comm object, and the renderer can talk to the real kernel through this comm object. This platform/frontend-specific comm object is like a kernel in the webview environment.
I'm wondering if we should then push this a bit future: allow kernel preloads to directly register a kernel in the webview if we want to standardize it. Basically a kernel preload can register a kernel by acquireVSCodeAPI().registerKernel(id, ...) and then when a renderer is activated, it can use getContributedKernel(id): IKernel | undefined to find the kernel if it exists. Conceptually it's the same as you proposed, but it does not introduce a "static dependency" between renderer and kernel (I guess in aznb, we don't have such static dependency at all).
For short, we are making renderers pure (they only live in the webview), but allowing it to talk to the real kernel behind the extension host breaks it purity, thus we allow extensions to create mirror/js kernel in the webview.
but allowing it to talk to the real kernel behind the extension host breaks it purity, thus we allow extensions to create mirror/js kernel in the webview.
Good point.
Had a call with Peng. We agreed that getContributedKernel(id): IKernel | undefined is better versus a static contribution. But we went into some more cases where the discovery of relevant contributed kernels is hard. I would really like to solve the problem of using a mismatched renderer+kernel (notifying and more importantly providing a clear path to reconciliation) but I'm not sure that's possible.
In this design for the question of the ipywidgets, every kernel that _could_ support ipywidgets would provide the renderer kernel instance, and it would be up to the rendere kernel+renderer to gracefully fail (and show an appropriate error message, e.g. steps to enable them) if the physical kernel does not have ipywidget support.
What works happen if I use a notebook, run a cell with python kernel and get ipywidgets.
Then create another cell and run that cell with a rust kernel.
Would VSCode display a message saying the kernel is wrong for one of the cell output?
Or would VSCode display a message when opening the same notebook with rust kernel? At this point we're only rendering static output, don't see why VSCode should get involved in detecting the fact that the kernel is wrong. If the renderer can display it will.
If the renderer needs to access a kennel and the kernel is undefined.
At that point the renderer should handle that and tail gracefully. Or of this what you're trying to solve in VSCode.
Or would VSCode display a message when opening the same notebook with rust kernel? At this point we're only rendering static output, don't see why VSCode should get involved in detecting the fact that the kernel is wrong. If the renderer can display it will.
IMHO the only error message VS Code should show is we can't render this mimetype, please install renderer extension xyz and it's xyz renderer's responsibility to tell users what potentially goes wrong. For example, ipywidgets renderer claims that it can handle application/vdn-ipywidgets, we delegate the rendering to ipywidgets renderer, and the renderer can draw the widget and maybe with some warning overlay, indicating that the widget is currently not interactive as there is no compatible kernel connected.
@DonJayamanne can you help us understand how now the renderer code checks if the jupyter server supports ipywidgets or not?
renderer code checks if the jupyter server supports ipywidgets or not?
Renderer & extensions communicate via the postMessage.
If we have ipywidgets in output, its assumed that jupyter server supports ipywidgets (it has been stable for a few years in Jupyter).
Note: We do not yet support rendering ipywidgets for static notebooks.
We decided to leave this for later. Thus this isn't an issue today, however the day we do support it, it will have to work even without a live kernel (basically rendering ipywidgets with the data embedded in the output).
renderer's responsibility to tell users what potentially goes wrong.
Agreed.
Thanks, Don.
The weekend brought no new great flashes of insight wrt the discovery scenario. If that is insolvable, then, the proposal would by narrowed down to only the registerKernel and getContributedKernel that Peng mentioned.
However in this scenario there the only advantage over just using a global in the window is that the TS types for getContributedKernel would (if strict null checks are enabled) force the renderer to handle the absence of the kernel. But if strict null checks aren't enabled, then there's no advantage I see.
I see two 馃憤's, so let's stick with the global for now. I will remove the (non-operational) postMessage from the renderer API, and update the docs with this guidance tomorrow.
I will remove the (non-operational) postMessage from the renderer API,
FY - Last week i found that onMessage in the renderer API doesn't work, I had to listen to global messages at the window level.
Something for a separate thread.
Yes, they're both connected to the no-longer-existent NotebookOutputRenderer in the extension host, so they would both be removed (in favor of having extensions do their kernel messaging through whatever global(s) the kernel exposes)
This is done in https://github.com/microsoft/vscode/pull/109273
Most helpful comment
Thanks, Don.
The weekend brought no new great flashes of insight wrt the discovery scenario. If that is insolvable, then, the proposal would by narrowed down to only the
registerKernelandgetContributedKernelthat Peng mentioned.However in this scenario there the only advantage over just using a global in the window is that the TS types for
getContributedKernelwould (if strict null checks are enabled) force the renderer to handle the absence of the kernel. But if strict null checks aren't enabled, then there's no advantage I see.