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

Does 'registerKernelSourceActionProvider' work for extensions other than 'ms-toolsai.jupyter' #197621

Open
tomqwpl opened this issue Nov 7, 2023 · 2 comments
Assignees
Labels
notebook-kernel under-discussion Issue is under discussion for relevance, priority, approach

Comments

@tomqwpl
Copy link
Contributor

tomqwpl commented Nov 7, 2023

With reference to API proposal finalization for notebookKernelSource API in endgame plan at #197438

I'm trying to make use of this API. I'm using 1.85.0-insiders.

There is code in notebookKernelQuickPickStrategy.ts that smells to me:

			if (isKernelSourceQuickPickItem(selectedKernelPickItem)) {
				try {
					const selectedKernelId = await this._executeCommand<string>(notebook, selectedKernelPickItem.command);
					if (selectedKernelId) {
						const { all } = await this._getMatchingResult(notebook);
						const kernel = all.find(kernel => kernel.id === `ms-toolsai.jupyter/${selectedKernelId}`);
						if (kernel) {
							await this._selecteKernel(notebook, kernel);
							return true;
						}
						return true;
					} else {
						return this.displaySelectAnotherQuickPick(editor, false);
					}
				} catch (ex) {
					return false;
				}

I currently don't have things set up to actually run VSCode in the debugger and debug the code, so this is by code inspection. This code would appear to only work if your extension ID is ms-toolsai.jupyter? If I look at all of the other ways of selecting a kernel, they all ultimately seem to end with await this._selecteKernel(notebook, kernel);. This code would would appear to only do that in a special case. If the command returns a kernel ID, but it doesn't match that ID pattern, true is returned, despite no kernel apparently being selected?

I'm have difficulty actually selecting a kernel using this registerKernelSourceActionProvider mechanism, and looking at this code, it would seem that this might be why?

@gjsjohnmurray
Copy link
Contributor

/assign @DonJayamanne

@tomqwpl
Copy link
Contributor Author

tomqwpl commented Nov 7, 2023

There is also this:

	private async _calculdateKernelSources(editor: IActiveNotebookEditor) {
		const notebook: NotebookTextModel = editor.textModel;

		const sourceActionCommands = this._notebookKernelService.getSourceActions(notebook, editor.scopedContextKeyService);
		const actions = await this._notebookKernelService.getKernelSourceActions2(notebook);
		const matchResult = this._getMatchingResult(notebook);

		if (sourceActionCommands.length === 0 && matchResult.all.length === 0 && actions.length === 0) {
			return await this._getKernelRecommendationsQuickPickItems(notebook, this._extensionWorkbenchService) ?? [];
		}

		const others = matchResult.all.filter(item => item.extension.value !== JUPYTER_EXTENSION_ID);
		const quickPickItems: QuickPickInput<KernelQuickPickItem>[] = [];

So here, it removes registered kernels where the extension ID is the jupter extension, presumably on the basis that it knows that the jupyter extension then uses the "notebookKernelSource" API to show its own prompts to select a kernel. So it looks like this whole notebookKernelSource API can only be made use of by the jupyter extension, its something that is specifically put in place to support a thing that that one extension wanted to do (basically it seems to want to present the list of kernels in a slightly different way). However, as a third party extension developer, the result will be that my kernels would be listed twice. The command I register with registerKernelSourceActionProvider would be listed as a source, and presumably that would then list kernels in a particular way. However, any kernels of mine that are already registered are then also listed under my extension. This is likely to produce a confusing result.

Tracing through the code I'm not really sure I even understand what the general point of the proposed API actually is. I had assumed that the point was that it allowed me to not "pre-register" the kernels by calling createNotebookController. However that doesn't seem to be the case. The command that is called by the registerKernelSourceActionProvider mechanism has to return a kernel ID, and it appears that there has to already be a kernel of that ID existing, that is, something for which createNotebookController had already been called. I had hoped to use this mechanism to show a list of existing "connection profiles" that I have, plus the option of creating a new one, however it looks like that's not going to be possible. Maybe, I can call createNotebookController as part of that command, but at the moment it's hard to tell as I can't get the facility working anyway.

@DonJayamanne DonJayamanne assigned rebornix and unassigned DonJayamanne Nov 7, 2023
@rebornix rebornix added under-discussion Issue is under discussion for relevance, priority, approach notebook labels Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notebook-kernel under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

5 participants