-
Notifications
You must be signed in to change notification settings - Fork 29k
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
Notebook kernel source menu contribution #150146
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jrieken
reviewed
May 23, 2022
src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts
Outdated
Show resolved
Hide resolved
jrieken
reviewed
May 23, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good.Left a nit-comment. CI is failing, no approval because of that
roblourens
previously approved these changes
May 23, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet
roblourens
approved these changes
May 24, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is an API experiment for #146942 based on offline discussions with @jrieken . The idea is allowing extensions to contribute commands to the kernel picker list through menu contribution. This gives extensions the full capabilities of implementing proxy kernels, kernel servers, etc. The menu contribution can be replaced by real APIs if necessary (especially if we have quite some interops with other models in the notebook).
The code in this PR is large, but it's basically:
ProxyKernel
concept in the core or API.notebook/kernelSource
notebook/kernelSource
In addition to above, here are two typical workflows enabled:
Single source command
When there is only one single extension contributing a command to menu
notebook/kernelSource
, users will see that command in the kernel status bar item, as shown belowWhen users attempt to run cells, the core will run the command. When the command finishes execution (promise resolves), we will detect if there are notebook controllers created for current notebook, if there is a preferred one, pass the pending cell execution request to the controller, finish the cell execution.
Mixed notebook kernels and kernel source commands
When we have multiple kernels and commands contributed by extensions, the kernel status bar item behaves the same as before, it will show the suggested/preferred kernel name if there is any.
When users click the kernel status item, we will show the kernel picker, which lists all available kernels and commands contributed to menu
notebook/kernelSource
. If users pick a command/action fromnotebook/kernelSource
, the core will run the command. At the end, we might get more kernels/controllers contributed as a side effect of running the command. Users can then run cells as usual.