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

Kernel and renderer compatibility constraints #119899

Closed
connor4312 opened this issue Mar 25, 2021 · 5 comments
Closed

Kernel and renderer compatibility constraints #119899

connor4312 opened this issue Mar 25, 2021 · 5 comments
Assignees
Labels
api-proposal feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders notebook verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@connor4312
Copy link
Member

connor4312 commented Mar 25, 2021

In the initial prototype of renderers, we said that kernels would expose an interface in their preloads, and if a renderer loads in and sees that the expected interface is missing, it would be responsible for showing appropriate help to the user. However, this is not a polished experience for a variety of reasons.

In an ideal world both kernel-providing extensions and renderers statically define their compatibility "tags", but Jupyter kernels can be discovered and changed dynamically, so this is not possible. My plan is that:

  • Kernels can expose well-known "tags" at runtime in VS Code (or aznb)
  • Renderers statically define required or optional tags in their package.json.

This means that when we render an output, we can determine what and whether any installed renderers are compatible. With a marketplace integration that would let us search by tags, we also can suggest renderers that could be useful for the output. This is particularly important if the execution produces a mime type for which there is no existing renderer.

Currently kernels have a list of preload URIs, but these preloads also define the API(s) that the kernel provides. I suggest introducing a new NotebookKernelPreload interface that would be used instead:

export interface NotebookKernelPreload {
	provides: string | string[];
	uri: Uri;
}
export interface NotebookKernel {
-	preloads?: Uri[];
+	preloads?: NotebookKernelPreload[];

Then in package.jsons, renders could define a list of dependencies or optionalDependencies.

"notebookOutputRenderer": [
	{
		"id": "github-issues",
		"displayName": "Github Issues Notebook Renderer",
		"entrypoint": "./dist/renderer.js",
		"mimeTypes": [
			"x-application/github-issues"
		],
		"dependencies": [
			"gh-issue-kernel"
		]
	}
],

Dependencies is a "one of" list -- the renderer will be considered compatible if at least one of the requested dependencies is provided by the kernel. optionalDependencies can be used to hint at compatibility or kernels that can allow for better integration, but aren't required. When picking a renderer to use for some new output, VS Code will prefer renderers that have an available kernel in their dependencies over those which do not.

To be discussed: Do we need to 'pattern match' dependencies, e.g. could I optionally depend on any "jupyter dap" kernel with jupyter/*/dap versus specifically jupyter/python/dap.

Followup: we decided to not implement this initially. It can be easily added later, and we feel we can make a more informed decision once an ecosystem of renderers and kernels begins to develop.

Updated from discussion: To choose a renderer for an output, VS Code will assume 'richer-is-better' and prefer:

  • For each mime type in the output, choose:
    1. return renderer with a hard dependency on an available kernel, otherwise
    2. return renderer with an optional dependency on an available kernel, otherwise
    3. return any other renderer for this mimetype.
  • If no suitable renderer is found, show a 'fallback' for the first mime type. VS Code will suggest searching in the marketplace for an output renderer, and provide a button to do so.

When opening a new notebook with existing output, VS Code will choose the best available renderer assuming that whatever kernel dependency it needs will eventually be available. Therefore, renderers must be able to deal with a kernel loading in after the first render.

If a cell is executed and its output renderer is no longer compatible with the current kernel, the selection algorithm runs again.

cc @vijayupadya @kjcho-msft

@connor4312 connor4312 added this to the April 2021 milestone Mar 25, 2021
@connor4312 connor4312 self-assigned this Mar 25, 2021
@connor4312
Copy link
Member Author

Talked with Kai about this in my 1:1, mostly discussing the latter 'cold open' case. There are a few options to solve it. One question we had is, namely for Jupyter notebooks, does the notebook record what kernel was used for it? It seems like the answer is yes:

"metadata": {
  "kernelspec": {
    "display_name": ".NET (C#)",
    "language": "C#",
    "name": ".net-csharp"
  },

This doesn't quite map us to the "provides", but I wonder if we could add that in the metadata, at least for notebooks saved with VS Code. We could:

  • Say that renderers must be able to handle a lack of kernel support in case their output is shown from a cold open.
  • Pick the best renderer available as if the specified kernel is available
  • Re-render when the first execution happens

The pre-selection of renderer prevents the case of us switching renderers the first time the user runs a cell, which is a bad user experience (especially if it leads to a 'downgraded' output).

For renderers that don't provide a hint, we can use the best renderer that doesn't require a specific kernel, and then 'upgrade' on the first execution if there's a new renderer that works with the current kernel.


The other thing we talked about was dependency constraints. Maybe tags can also be specified as a path, like jupyter/python/xeus/v2, and then dependencies can star-match like jupyter/*/xeus/v2

@connor4312
Copy link
Member Author

Talking to @rebornix, one thing we realized is that it's going to be very common to have a renderer that both talks to a kernel and renders the same mimetype as a different renderer that talks a to a different kernel. Notably, ipywidgets have their own special mimetype.

So based on this, we can say that:

  • When opening a document, we'll always choose the 'best' renderer, assuming there's going to be a kernel available.
  • Renderers should deal with kernels not being available on their initial load / coming in late. Maybe we expose some onDidKernelLoad event for this.
  • If this guess fails and an impure renderer is executed under a kernel which it is not compatible with, then we'll re-run selection.
  • We'll keep a workspace cache of the preferred renderers for mimetypes in notebooks. This could lead to confusion if a user installed a better renderer and expects to switch automatically, but on the other hand manually switching the renderer for an output once they install it is not a large extra step.

connor4312 added a commit that referenced this issue Apr 12, 2021
See #119899

Backwards compatible, initially. The implementation should be
pretty unsurprising. Some churn from data wiring.

This does the bulk of the work. The only remaining item is caching
the last renderer used for notebooks in the workspace to avoid running
selection again if the user reopens/reloads the window.
@connor4312 connor4312 added api-proposal feature-request Request for new features or functionality labels Apr 13, 2021
@connor4312
Copy link
Member Author

Implemented in the linked commits. In this initial version we did not have strong consensus around the range of requirements for constraints. Therefore currently we only do precise string matching. Later on we can add globs if users request it.

@joaomoreno joaomoreno added the verification-needed Verification of issue is requested label Apr 28, 2021
@alexr00 alexr00 added the verification-steps-needed Steps to verify are needed for verification label Apr 29, 2021
@connor4312
Copy link
Member Author

Verifying this is kind of a pain, but possible:

  1. Start with the github issues notebook: https://github.com/microsoft/vscode-github-issue-notebooks
  2. Add a URI to its preloads that has a provides. The URI can be a path an an empty .js file on disk
  3. Add multiple renderers to its package.json that have hard or soft dependencies on what that "provides".
  4. Verify that behavior and selection works as described at the bottom of the Kernel and renderer compatibility constraints #119899 (comment)
  5. Verify that the preferred renderer, when selected and available, is cached

@connor4312 connor4312 removed the verification-steps-needed Steps to verify are needed for verification label Apr 29, 2021
@roblourens roblourens added the verified Verification succeeded label Apr 29, 2021
@github-actions github-actions bot locked and limited conversation to collaborators May 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-proposal feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders notebook verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants
@joaomoreno @roblourens @connor4312 @alexr00 and others