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

UI: Notebook controller/kernel detection progress #165959

Closed
rebornix opened this issue Nov 9, 2022 · 1 comment
Closed

UI: Notebook controller/kernel detection progress #165959

rebornix opened this issue Nov 9, 2022 · 1 comment

Comments

@rebornix
Copy link
Member

rebornix commented Nov 9, 2022

The Jupyter extension already adopts the push model for creating controllers (instead of waiting for all kernels to be detected and then creating them in one batch operation). Thus users usually see

  • Notebook open, Jupyter activates and then register local kernel specs.
    • Local kernel specs can come reasonably fast, e.g., hundreds of ms.
  • Users open the kernel picker but didn't find the kernel they want to use (because Jupyter is still discovering local Python kernels or remote Jupyter kernels).
  • Both the kernel picker and kernel status are static now.
  • More controllers are created gradually, including
    • Python extension detected local python environments on the system and Jupyter creates controllers for them.
    • Jupyter extension connects to remote Jupyter server (provided by user manually or through 3rd party extension, e.g. GitHub) and finds kernel specs and sessions on the server, then create controllers for them.

The detection of Python environments and kernels from remote Jupyter servers are generally not fast and there is a limit of how much we can improve them. This leads us to the common UX complain: users open and look at kernel pickers at the top of the editor but unaware of the kernel detections in background and unclear how to proceed.


Allowing extensions to present a "kernel detection" progress to users would improve the UX. There are two options available to us

  • Extending window.withProgress. @sbatten and I looked into this options together and we found the withProgress API currently is only aware of the view/layout system. Extensions can use this API to show progress bar on a view/container/composite, along with dialog and notification, but it is not aware of any implementation details of editor groups or a specific view inside an editor composite. Adding notebook controller/kernel detection logic to it would be layer breaking, unless we introduce a generic extensibility model to the progress service.
  • A notebook kernel specific progress like API. The API can be minimal and tuned specifically for notebooks. Extensions use this API to tell VS Code there is kernel detection tasks in the background and the core embeds the progress info into the kernel status and picker.

I experimented with the later approach a bit (tracking in #165958, and it's intentional that the API is not named/designed as withNotebook/Kernel*Progress but closer to a factory api). Would love to hear feedbacks from @jrieken @mjbvz @roblourens since you engaged in the current design of the controller API. We can have offline meetings as well if we want to expand on the details.

declare module 'vscode' {
	export interface NotebookControllerDetectionTask {
		dispose(): void;
	}

	export namespace notebooks {
		export function createNotebookControllerDetectionTask(notebookType: string): NotebookControllerDetectionTask;
	}
}
Screen.Recording.2022-11-15.at.11.17.00.AM.mov
@rebornix
Copy link
Member Author

we found the withProgress API currently is only aware of the view/layout system. Extensions can use this API to show progress bar on a view/container/composite, along with dialog and notification, but it is not aware of any implementation details of editor groups or a specific view inside an editor composite

Here I'm mostly referring to the internal implementation of the progress service, but necessarily the public one. There are multiple ways to tackle this without lifting the internal progress service from its layer.

Putting the impl aside, the most natural API for this is still extending the Progress API to allow extensions to express there is a kernel related task/progress for a specific notebook type:

export function withProgress<R>(options: ProgressOptions, task: (progress: Progress<{ message?: string; increment?: number }>, token: CancellationToken) => Thenable<R>): Thenable<R>;

export interface ProgressOptions {
-	location: ProgressLocation | { viewId: string };
+	location: ProgressLocation | { viewId: string } | { location: 'KernelPicker', viewType: string };
	title?: string;
	cancellable?: boolean;
}

{ location, viewType } decides where the progress will present inside a ${viewtype} notebook editor and extensions can offer human readable info for the progress through title, with which the Progress API still only talks about location/position, but not purpose.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant