Select kernel does not put up the 'install ipykernel' message

[rchiodo]

Create a new virtual env
Open a notebook with 'Select Kernel' being shown at the bottom
Run a cell
Pick the new env
Make sure 'auto start' is enabled.

Nothing happens

It should ask you to install ipykernel.

[joyceerhl]

This is what we're expecting to see, right?

This works in webview fwiw so likely related to notebook controller changes.

[rchiodo]

Yes that should come up.

[joyceerhl]

Actually sorry this seems to be working for me following the repro steps above with a new conda environment:

Rich do you happen to have a log you can share?

[rchiodo]

errorlog.log

[joyceerhl]

So I'll confess I don't have the most context on our kernel start logic, but I went through the codebase looking for any places that we were improperly passing disableUI: true. These two spots looked potentially iffy to me:

vscode-jupyter/src/client/datascience/notebook/notebookControllerManager.ts

Lines 441 to 444 in 8d84808

	// Auto start the local kernels.
	if (newKernel && !this.configuration.getSettings(undefined).disableJupyterAutoStart && this.isLocalLaunch) {
	await newKernel.start({ disableUI: true, document }).catch(noop);
	}

vscode-jupyter/src/client/datascience/jupyter/serverPreload.ts

Lines 59 to 77 in 4a0eefa

	traceInfo(`Attempting to start a server because of preload conditions ...`);
	// Check if we are already connected
	let providerConnection = await this.notebookProvider.connect({
	getOnly: true,
	disableUI: true,
	resource: undefined
	});
	// If it didn't start, attempt for local and if allowed.
	if (!providerConnection && !this.configService.getSettings(undefined).disableJupyterAutoStart) {
	// Local case, try creating one
	providerConnection = await this.notebookProvider.connect({
	getOnly: false,
	resource: undefined,
	disableUI: true,
	localOnly: true
	});
	}

Rich's log doesn't have the log message from the second code snippet in it so I think that means the first one might be the culprit. The first one was also last touched in the kernelpush updates: #5565

[DonJayamanne]

that we were improperly passing disableUI: true. These two spots looked potentially iffy to me:

I think this is correct

I think we should change this to perhaps have a state.
If dependency installation checks has been completed, then we need to re-run them.

Basically its a race condition, auto startup says don't display UI. But when you start running a cell, if the auto start has already started, then we just use that existing promise.
We were aware of this when we did this.
I recall the discussion around the Progress indicators as well. E.g. you'll never see any of the progress indicators for starting the notebook either (if you are in the small window).

I think a quick fix would be to re-check the dependencies based on some state.
Long term fix is to ensure we have the ability to display the progress indicators when user hits run & auto start is still in progress.

[rchiodo]

I remember having something that would switch the 'disableUI' flag as soon as somebody clicked a button. But that was long ago and it was for getting a jupyter server. I believe that's how we handled this before.

[DonJayamanne]

I think we'll need an object instead of a flag, so that when progress messages are being displayed, we can see the change in the state of the object & we can then create/display the UI.

However, the first issue - checking dependencies will still need to be addressed separately.
Generally that happens first, and its possible we've missed the ship on that, and might have to perform another check.

I think the best fix is:

• Just disable auto start for native notebooks.
• fix this in june (not even via a point fix)

As its not a bug, but a mere perf issue & i don't think it will be too bad, after all, we didn't have auto start in native notebooks for a while, hence we should be ok until next release.

Also better than making a lot of major changes to the code (that's used in webview/interactive), etc at the last minute.

[joyceerhl]

My vote would be to defer fixing this to the June release if possible, as it doesn't seem to be a regression and a fix is risky (potentially impacts kernel startup and being able to run cells). I'm not sure we need to disable auto start at all for the May release. Am I missing something about this bug that would make it worth fixing sooner?

[DonJayamanne]

I agree

[joyceerhl]

Autostart is disabled in main right now. We should turn that back on as part of fixing this bug.

[IanMatthewHuff]

Quick note when fixing this. This code should be reenabled as part of the fix:

vscode-jupyter/src/client/datascience/notebook/notebookControllerManager.ts

Lines 463 to 466 in aaa6fe3

	// Auto start the local kernels.
	// if (newKernel && !this.configuration.getSettings(undefined).disableJupyterAutoStart && this.isLocalLaunch) {
	// await newKernel.start({ disableUI: true, document }).catch(noop);
	// }

[greazer]

Can wait until July.

[DonJayamanne]

Make sure 'auto start' is enabled.

Closing in favor of #7903