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

Cherry pick changes from main branch into release #6174

Merged
merged 8 commits into from
Jun 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,6 @@ module.exports = {
'src/client/datascience/themeFinder.ts',
'src/client/datascience/multiplexingDebugService.ts',
'src/client/datascience/interactive-window/identity.ts',
'src/client/datascience/interactive-window/interactiveWindow.ts',
'src/client/datascience/datascience.ts',
'src/client/datascience/liveshare/liveshare.ts',
'src/client/datascience/liveshare/serviceProxy.ts',
Expand Down
71 changes: 0 additions & 71 deletions .github/workflows/publish-insiders.yml

This file was deleted.

86 changes: 0 additions & 86 deletions .github/workflows/publish-release.yml

This file was deleted.

1 change: 1 addition & 0 deletions news/2 Fixes/6164.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hide kernels belonging to deleted Python environments from kernel picker.
19 changes: 12 additions & 7 deletions src/client/datascience/commands/notebookCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@
import { inject, injectable } from 'inversify';
import { Uri } from 'vscode';
import { ICommandManager } from '../../common/application/types';
import { IDisposable } from '../../common/types';
import { IConfigurationService, IDisposable } from '../../common/types';
import { Commands } from '../constants';
import {
getDisplayNameOrNameOfKernelConnection,
isLocalLaunch,
kernelConnectionMetadataHasKernelModel
} from '../jupyter/kernels/helpers';
import { KernelSelector } from '../jupyter/kernels/kernelSelector';
Expand All @@ -26,7 +27,8 @@ export class NotebookCommands implements IDisposable {
@inject(IInteractiveWindowProvider) private interactiveWindowProvider: IInteractiveWindowProvider,
@inject(INotebookProvider) private readonly notebookProvider: INotebookProvider,
@inject(KernelSelector) private readonly kernelSelector: KernelSelector,
@inject(KernelSwitcher) private readonly kernelSwitcher: KernelSwitcher
@inject(KernelSwitcher) private readonly kernelSwitcher: KernelSwitcher,
@inject(IConfigurationService) private readonly configService: IConfigurationService
) {}
public register() {
this.disposables.push(
Expand Down Expand Up @@ -95,12 +97,15 @@ export class NotebookCommands implements IDisposable {
};
}
if (options.identity) {
const isLocal = isLocalLaunch(this.configService);
// Make sure we have a connection or we can't get remote kernels.
const connection = await this.notebookProvider.connect({
getOnly: false,
disableUI: false,
resource: options.resource
});
const connection = isLocal
? undefined
: await this.notebookProvider.connect({
getOnly: false,
disableUI: false,
resource: options.resource
});

// Select a new kernel using the connection information
const kernel = await this.kernelSelector.selectJupyterKernel(
Expand Down
3 changes: 3 additions & 0 deletions src/client/datascience/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -709,3 +709,6 @@ export namespace LiveShareCommands {
export const VSCodeNotebookProvider = 'VSCodeNotebookProvider';
export const OurNotebookProvider = 'OurNotebookProvider';
export const DataScienceStartupTime = Symbol('DataScienceStartupTime');

// Default for notebook version (major & minor) used when creating notebooks.
export const defaultNotebookFormat = { major: 4, minor: 2 };
35 changes: 19 additions & 16 deletions src/client/datascience/interactive-window/interactiveWindow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ import {
} from '../../common/types';
import { createDeferred, Deferred } from '../../common/utils/async';
import * as localize from '../../common/utils/localize';
import { noop } from '../../common/utils/misc';
import { EXTENSION_ROOT_DIR } from '../../constants';
import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
import { Commands, EditorContexts, Identifiers, Telemetry } from '../constants';
import { Commands, defaultNotebookFormat, EditorContexts, Identifiers, Telemetry } from '../constants';
import { IDataViewerFactory } from '../data-viewing/types';
import { ExportFormat, IExportDialog } from '../export/types';
import { InteractiveBase } from '../interactive-common/interactiveBase';
Expand Down Expand Up @@ -411,18 +412,18 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi
// extension contexts
if (this.commandManager && this.commandManager.executeCommand) {
const interactiveContext = new ContextKey(EditorContexts.HaveInteractive, this.commandManager);
interactiveContext.set(!this.isDisposed).catch();
interactiveContext.set(!this.isDisposed).catch(noop);
const interactiveCellsContext = new ContextKey(EditorContexts.HaveInteractiveCells, this.commandManager);
const redoableContext = new ContextKey(EditorContexts.HaveRedoableCells, this.commandManager);
const hasCellSelectedContext = new ContextKey(EditorContexts.HaveCellSelected, this.commandManager);
if (info) {
interactiveCellsContext.set(info.cellCount > 0).catch();
redoableContext.set(info.redoCount > 0).catch();
hasCellSelectedContext.set(info.selectedCell ? true : false).catch();
interactiveCellsContext.set(info.cellCount > 0).catch(noop);
redoableContext.set(info.redoCount > 0).catch(noop);
hasCellSelectedContext.set(info.selectedCell ? true : false).catch(noop);
} else {
interactiveCellsContext.set(false).catch();
redoableContext.set(false).catch();
hasCellSelectedContext.set(false).catch();
interactiveCellsContext.set(false).catch(noop);
redoableContext.set(false).catch(noop);
hasCellSelectedContext.set(false).catch(noop);
}
}
}
Expand Down Expand Up @@ -533,7 +534,7 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi
}

// Pull out the metadata from our active notebook
const metadata: nbformat.INotebookMetadata = { orig_nbformat: 3 };
const metadata: nbformat.INotebookMetadata = { orig_nbformat: defaultNotebookFormat.major };
if (this.notebook) {
updateNotebookMetadata(metadata, this.notebook?.getKernelConnection());
}
Expand All @@ -551,13 +552,15 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi
}

// Then run the export command with these contents
this.commandManager.executeCommand(
Commands.Export,
contents,
this.owningResource,
defaultFileName,
this.notebook?.getMatchingInterpreter()
);
this.commandManager
.executeCommand(
Commands.Export,
contents,
this.owningResource,
defaultFileName,
this.notebook?.getMatchingInterpreter()
)
.then(noop, noop);
}

private handleModelChange(update: NotebookModelChange) {
Expand Down
8 changes: 4 additions & 4 deletions src/client/datascience/jupyter/jupyterExporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import * as localize from '../../common/utils/localize';
import { noop } from '../../common/utils/misc';
import { CellMatcher } from '../cellMatcher';
import { pruneCell } from '../common';
import { CodeSnippets, Identifiers } from '../constants';
import { CodeSnippets, defaultNotebookFormat, Identifiers } from '../constants';
import {
CellState,
ICell,
Expand Down Expand Up @@ -104,7 +104,7 @@ export class JupyterExporter implements INotebookExporter {
pygments_lexer: `ipython${pythonNumber}`,
version: pythonNumber
},
orig_nbformat: 2,
orig_nbformat: defaultNotebookFormat.major,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
kernelspec: kernelSpec as any
};
Expand All @@ -115,8 +115,8 @@ export class JupyterExporter implements INotebookExporter {
// Combine this into a JSON object
return {
cells: this.pruneCells(cells, matcher),
nbformat: 4,
nbformat_minor: 2,
nbformat: defaultNotebookFormat.major,
nbformat_minor: defaultNotebookFormat.minor,
metadata: metadata
};
}
Expand Down
32 changes: 24 additions & 8 deletions src/client/datascience/jupyter/kernels/kernelDependencyService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export class KernelDependencyService implements IKernelDependencyService {
// Cache the install run
let promise = this.installPromises.get(interpreter.path);
if (!promise) {
promise = this.runInstaller(interpreter, token, disableUI);
promise = this.runInstaller(interpreter, token, disableUI, resource);
this.installPromises.set(interpreter.path, promise);
}

Expand All @@ -89,11 +89,17 @@ export class KernelDependencyService implements IKernelDependencyService {
return;
}
if (response === KernelInterpreterDependencyResponse.selectDifferentKernel) {
const cmd =
getResourceType(resource) === 'notebook' && this.useNativeNb
? 'notebook.selectKernel'
: Commands.SwitchJupyterKernel;
this.commandManager.executeCommand(cmd).then(noop, noop);
if (getResourceType(resource) === 'notebook' && this.useNativeNb) {
this.commandManager.executeCommand('notebook.selectKernel').then(noop, noop);
} else {
this.commandManager
.executeCommand(Commands.SwitchJupyterKernel, {
currentKernelDisplayName: interpreter.displayName,
identity: resource,
resource
})
.then(noop, noop);
}
}
throw new IpyKernelNotInstalledError(
DataScience.ipykernelNotInstalled().format(
Expand All @@ -105,7 +111,8 @@ export class KernelDependencyService implements IKernelDependencyService {
private async runInstaller(
interpreter: PythonEnvironment,
token?: CancellationToken,
disableUI?: boolean
disableUI?: boolean,
resource?: Resource
): Promise<KernelInterpreterDependencyResponse> {
// If there's no UI, then cancel installation.
if (disableUI) {
Expand All @@ -131,10 +138,19 @@ export class KernelDependencyService implements IKernelDependencyService {
});
const installPrompt = isModulePresent ? Common.reInstall() : Common.install();
const selectKernel = DataScience.selectKernel();
// Due to a bug in our code, if we don't have a resource, don't display the option to change kernels.
// https://github.com/microsoft/vscode-jupyter/issues/6135
const options = resource ? [installPrompt, selectKernel] : [installPrompt];
// In the case of interactive window, due to the current code flow we get this code executed twice,
// hence we get two messages about ipykernel not being installed.
// THat's a very poor ux, one could end up with two modal dialog boxes (one after the other for interactive).
// hence disabling modal dialog for interactive window for now.
// Again to be resolved in https://github.com/microsoft/vscode-jupyter/issues/6135
const modal = getResourceType(resource) === 'notebook';
const selection = this.isCodeSpace
? installPrompt
: await Promise.race([
this.appShell.showErrorMessage(message, { modal: true }, installPrompt, selectKernel),
this.appShell.showErrorMessage(message, { modal }, ...options),
promptCancellationPromise
]);
if (installerToken.isCancellationRequested) {
Expand Down
Loading