From 41696f48e4cdd5404df860efdfc0ccf126dc4ab6 Mon Sep 17 00:00:00 2001 From: Ian Huff Date: Wed, 16 Sep 2020 16:07:43 -0700 Subject: [PATCH 1/5] basic install working --- .../jupyter/kernels/kernelSelector.ts | 9 ++++++ .../kernel-launcher/kernelFinder.ts | 30 +++++++++++++++++-- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/client/datascience/jupyter/kernels/kernelSelector.ts b/src/client/datascience/jupyter/kernels/kernelSelector.ts index d2623d2df616..c90fbbaf9898 100644 --- a/src/client/datascience/jupyter/kernels/kernelSelector.ts +++ b/src/client/datascience/jupyter/kernels/kernelSelector.ts @@ -504,6 +504,10 @@ export class KernelSelector implements IKernelSelectionUsage { if (!kernelSpec && !activeInterpreter) { return; } else if (!kernelSpec && activeInterpreter) { + if (!ignoreDependencyCheck) { + await this.kernelDependencyService.installMissingDependencies(activeInterpreter, cancelToken); + } + // Return current interpreter. return { kind: 'startUsingPythonInterpreter', @@ -512,6 +516,11 @@ export class KernelSelector implements IKernelSelectionUsage { } else if (kernelSpec) { // Locate the interpreter that matches our kernelspec const interpreter = await this.kernelService.findMatchingInterpreter(kernelSpec, cancelToken); + + if (!ignoreDependencyCheck && interpreter) { + await this.kernelDependencyService.installMissingDependencies(interpreter, cancelToken); + } + return { kind: 'startUsingKernelSpec', kernelSpec, interpreter }; } } diff --git a/src/client/datascience/kernel-launcher/kernelFinder.ts b/src/client/datascience/kernel-launcher/kernelFinder.ts index 4958ea42216d..4185e30c9723 100644 --- a/src/client/datascience/kernel-launcher/kernelFinder.ts +++ b/src/client/datascience/kernel-launcher/kernelFinder.ts @@ -14,6 +14,7 @@ import { IPythonExecutionFactory } from '../../common/process/types'; import { IExtensionContext, IInstaller, InstallerResponse, IPathUtils, Product, Resource } from '../../common/types'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; import { IInterpreterLocatorService, IInterpreterService, KNOWN_PATH_SERVICE } from '../../interpreter/contracts'; +import { PythonEnvironment } from '../../pythonEnvironments/info'; import { captureTelemetry } from '../../telemetry'; import { getRealPath } from '../common'; import { Telemetry } from '../constants'; @@ -109,7 +110,8 @@ export class KernelFinder implements IKernelFinder { this.writeCache().ignoreErrors(); // Verify that ipykernel is installed into the given kernelspec interpreter - return ignoreDependencyCheck || !foundKernel ? foundKernel : this.verifyIpyKernel(foundKernel, cancelToken); + //return ignoreDependencyCheck || !foundKernel ? foundKernel : this.verifyIpyKernel(foundKernel, cancelToken); + return foundKernel; } // Search all our local file system locations for installed kernel specs and return them @@ -325,8 +327,30 @@ export class KernelFinder implements IKernelFinder { ): Promise { const interpreter = await getKernelInterpreter(kernelSpec, this.interpreterService); + await this.verifyIpyKernelInterpreter(interpreter, cancelToken); + + return kernelSpec; + + //if (await this.installer.isInstalled(Product.ipykernel, interpreter)) { + //return kernelSpec; + //} else { + //const token = new CancellationTokenSource(); + //const response = await this.installer.promptToInstall( + //Product.ipykernel, + //interpreter, + //wrapCancellationTokens(cancelToken, token.token) + //); + //if (response === InstallerResponse.Installed) { + //return kernelSpec; + //} + //} + + //throw new Error(`IPyKernel not installed into interpreter ${interpreter.displayName}`); + } + + private async verifyIpyKernelInterpreter(interpreter: PythonEnvironment, cancelToken?: CancellationToken) { if (await this.installer.isInstalled(Product.ipykernel, interpreter)) { - return kernelSpec; + return; } else { const token = new CancellationTokenSource(); const response = await this.installer.promptToInstall( @@ -335,7 +359,7 @@ export class KernelFinder implements IKernelFinder { wrapCancellationTokens(cancelToken, token.token) ); if (response === InstallerResponse.Installed) { - return kernelSpec; + return; } } From da26df70eeb6d4fc62fb48ffa6c62bd66594f4f9 Mon Sep 17 00:00:00 2001 From: Ian Huff Date: Wed, 16 Sep 2020 16:23:28 -0700 Subject: [PATCH 2/5] install dependencies in kernelSelector not finder --- .../jupyter/kernels/kernelSelector.ts | 26 +++++++--- .../kernel-launcher/kernelFinder.ts | 52 +------------------ 2 files changed, 21 insertions(+), 57 deletions(-) diff --git a/src/client/datascience/jupyter/kernels/kernelSelector.ts b/src/client/datascience/jupyter/kernels/kernelSelector.ts index c90fbbaf9898..37f7f7297857 100644 --- a/src/client/datascience/jupyter/kernels/kernelSelector.ts +++ b/src/client/datascience/jupyter/kernels/kernelSelector.ts @@ -27,7 +27,8 @@ import { IJupyterSessionManagerFactory, IKernelDependencyService, INotebookMetadataLive, - INotebookProviderConnection + INotebookProviderConnection, + KernelInterpreterDependencyResponse } from '../../types'; import { createDefaultKernelSpec, getDisplayNameOrNameOfKernelConnection } from './helpers'; import { KernelSelectionProvider } from './kernelSelections'; @@ -504,9 +505,7 @@ export class KernelSelector implements IKernelSelectionUsage { if (!kernelSpec && !activeInterpreter) { return; } else if (!kernelSpec && activeInterpreter) { - if (!ignoreDependencyCheck) { - await this.kernelDependencyService.installMissingDependencies(activeInterpreter, cancelToken); - } + await this.installDependenciesIntoInterpreter(activeInterpreter, ignoreDependencyCheck, cancelToken); // Return current interpreter. return { @@ -517,8 +516,8 @@ export class KernelSelector implements IKernelSelectionUsage { // Locate the interpreter that matches our kernelspec const interpreter = await this.kernelService.findMatchingInterpreter(kernelSpec, cancelToken); - if (!ignoreDependencyCheck && interpreter) { - await this.kernelDependencyService.installMissingDependencies(interpreter, cancelToken); + if (interpreter) { + await this.installDependenciesIntoInterpreter(interpreter, ignoreDependencyCheck, cancelToken); } return { kind: 'startUsingKernelSpec', kernelSpec, interpreter }; @@ -554,6 +553,21 @@ export class KernelSelector implements IKernelSelectionUsage { return { kernelSpec, interpreter, kind: 'startUsingPythonInterpreter' }; } + private async installDependenciesIntoInterpreter( + interpreter: PythonEnvironment, + ignoreDependencyCheck?: boolean, + cancelToken?: CancellationToken + ) { + if (!ignoreDependencyCheck) { + if ( + (await this.kernelDependencyService.installMissingDependencies(interpreter, cancelToken)) !== + KernelInterpreterDependencyResponse.ok + ) { + throw new Error(`IPyKernel not installed into interpreter ${interpreter.displayName}'`); + } + } + } + /** * Use the provided interpreter as a kernel. * If `displayNameOfKernelNotFound` is provided, then display a message indicating we're using the `current interpreter`. diff --git a/src/client/datascience/kernel-launcher/kernelFinder.ts b/src/client/datascience/kernel-launcher/kernelFinder.ts index 4185e30c9723..263de7ccf896 100644 --- a/src/client/datascience/kernel-launcher/kernelFinder.ts +++ b/src/client/datascience/kernel-launcher/kernelFinder.ts @@ -66,9 +66,7 @@ export class KernelFinder implements IKernelFinder { @captureTelemetry(Telemetry.KernelFinderPerf) public async findKernelSpec( resource: Resource, - kernelSpecMetadata?: nbformat.IKernelspecMetadata, - cancelToken?: CancellationToken, - ignoreDependencyCheck?: boolean + kernelSpecMetadata?: nbformat.IKernelspecMetadata ): Promise { await this.readCache(); let foundKernel: IJupyterKernelSpec | undefined; @@ -109,8 +107,6 @@ export class KernelFinder implements IKernelFinder { this.writeCache().ignoreErrors(); - // Verify that ipykernel is installed into the given kernelspec interpreter - //return ignoreDependencyCheck || !foundKernel ? foundKernel : this.verifyIpyKernel(foundKernel, cancelToken); return foundKernel; } @@ -320,52 +316,6 @@ export class KernelFinder implements IKernelFinder { return flatten(fullPathResults); } - // For the given kernelspec return back the kernelspec with ipykernel installed into it or error - private async verifyIpyKernel( - kernelSpec: IJupyterKernelSpec, - cancelToken?: CancellationToken - ): Promise { - const interpreter = await getKernelInterpreter(kernelSpec, this.interpreterService); - - await this.verifyIpyKernelInterpreter(interpreter, cancelToken); - - return kernelSpec; - - //if (await this.installer.isInstalled(Product.ipykernel, interpreter)) { - //return kernelSpec; - //} else { - //const token = new CancellationTokenSource(); - //const response = await this.installer.promptToInstall( - //Product.ipykernel, - //interpreter, - //wrapCancellationTokens(cancelToken, token.token) - //); - //if (response === InstallerResponse.Installed) { - //return kernelSpec; - //} - //} - - //throw new Error(`IPyKernel not installed into interpreter ${interpreter.displayName}`); - } - - private async verifyIpyKernelInterpreter(interpreter: PythonEnvironment, cancelToken?: CancellationToken) { - if (await this.installer.isInstalled(Product.ipykernel, interpreter)) { - return; - } else { - const token = new CancellationTokenSource(); - const response = await this.installer.promptToInstall( - Product.ipykernel, - interpreter, - wrapCancellationTokens(cancelToken, token.token) - ); - if (response === InstallerResponse.Installed) { - return; - } - } - - throw new Error(`IPyKernel not installed into interpreter ${interpreter.displayName}`); - } - private async getKernelSpecFromActiveInterpreter( kernelName: string, resource: Resource From 7e54e20340e2cb2c9c1e3de227086a9ca94fc367 Mon Sep 17 00:00:00 2001 From: Ian Huff Date: Wed, 16 Sep 2020 16:47:12 -0700 Subject: [PATCH 3/5] cleanup --- package.nls.json | 3 ++- src/client/common/utils/localize.ts | 4 ++++ src/client/datascience/jupyter/kernels/kernelSelector.ts | 6 +++++- src/client/datascience/kernel-launcher/kernelFinder.ts | 8 ++------ src/test/datascience/kernelFinder.unit.test.ts | 8 +------- 5 files changed, 14 insertions(+), 15 deletions(-) diff --git a/package.nls.json b/package.nls.json index ec6378814166..16a537b3a669 100644 --- a/package.nls.json +++ b/package.nls.json @@ -595,5 +595,6 @@ "DataScience.interactiveWindowModeBannerTitle": "Do you want to open a new Python Interactive window for this file? [More Information](command:workbench.action.openSettings?%5B%22python.dataScience.interactiveWindowMode%22%5D).", "DataScience.interactiveWindowModeBannerSwitchYes": "Yes", "DataScience.interactiveWindowModeBannerSwitchAlways": "Always", - "DataScience.interactiveWindowModeBannerSwitchNo": "No" + "DataScience.interactiveWindowModeBannerSwitchNo": "No", + "DataScience.ipykernelNotInstalled": "IPyKernel not installed into interpreter {0}" } diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 8207de65015f..4a4981f8ae63 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -1116,6 +1116,10 @@ export namespace DataScience { ); export const connected = localize('DataScience.connected', 'Connected'); export const disconnected = localize('DataScience.disconnected', 'Disconnected'); + export const ipykernelNotInstalled = localize( + 'DataScience.ipykernelNotInstalled', + 'IPyKernel not installed into interpreter {0}' + ); } export namespace StartPage { diff --git a/src/client/datascience/jupyter/kernels/kernelSelector.ts b/src/client/datascience/jupyter/kernels/kernelSelector.ts index 37f7f7297857..e398ded46d5f 100644 --- a/src/client/datascience/jupyter/kernels/kernelSelector.ts +++ b/src/client/datascience/jupyter/kernels/kernelSelector.ts @@ -553,6 +553,8 @@ export class KernelSelector implements IKernelSelectionUsage { return { kernelSpec, interpreter, kind: 'startUsingPythonInterpreter' }; } + // If we need to install our dependencies now (for non-native scenarios) + // then install ipykernel into the interpreter or throw error private async installDependenciesIntoInterpreter( interpreter: PythonEnvironment, ignoreDependencyCheck?: boolean, @@ -563,7 +565,9 @@ export class KernelSelector implements IKernelSelectionUsage { (await this.kernelDependencyService.installMissingDependencies(interpreter, cancelToken)) !== KernelInterpreterDependencyResponse.ok ) { - throw new Error(`IPyKernel not installed into interpreter ${interpreter.displayName}'`); + throw new Error( + localize.DataScience.ipykernelNotInstalled().format(interpreter.displayName || interpreter.path) + ); } } } diff --git a/src/client/datascience/kernel-launcher/kernelFinder.ts b/src/client/datascience/kernel-launcher/kernelFinder.ts index 263de7ccf896..5315c09caf05 100644 --- a/src/client/datascience/kernel-launcher/kernelFinder.ts +++ b/src/client/datascience/kernel-launcher/kernelFinder.ts @@ -5,23 +5,20 @@ import type { nbformat } from '@jupyterlab/coreutils'; import { inject, injectable, named } from 'inversify'; import * as path from 'path'; -import { CancellationToken, CancellationTokenSource } from 'vscode'; +import { CancellationToken } from 'vscode'; import { IWorkspaceService } from '../../common/application/types'; -import { wrapCancellationTokens } from '../../common/cancellation'; import { traceError, traceInfo } from '../../common/logger'; import { IPlatformService } from '../../common/platform/types'; import { IPythonExecutionFactory } from '../../common/process/types'; -import { IExtensionContext, IInstaller, InstallerResponse, IPathUtils, Product, Resource } from '../../common/types'; +import { IExtensionContext, IPathUtils, Resource } from '../../common/types'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; import { IInterpreterLocatorService, IInterpreterService, KNOWN_PATH_SERVICE } from '../../interpreter/contracts'; -import { PythonEnvironment } from '../../pythonEnvironments/info'; import { captureTelemetry } from '../../telemetry'; import { getRealPath } from '../common'; import { Telemetry } from '../constants'; import { defaultKernelSpecName } from '../jupyter/kernels/helpers'; import { JupyterKernelSpec } from '../jupyter/kernels/jupyterKernelSpec'; import { IDataScienceFileSystem, IJupyterKernelSpec } from '../types'; -import { getKernelInterpreter } from './helpers'; import { IKernelFinder } from './types'; // tslint:disable-next-line:no-require-imports no-var-requires const flatten = require('lodash/flatten') as typeof import('lodash/flatten'); @@ -57,7 +54,6 @@ export class KernelFinder implements IKernelFinder { @inject(IPlatformService) private platformService: IPlatformService, @inject(IDataScienceFileSystem) private fs: IDataScienceFileSystem, @inject(IPathUtils) private readonly pathUtils: IPathUtils, - @inject(IInstaller) private installer: IInstaller, @inject(IExtensionContext) private readonly context: IExtensionContext, @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, @inject(IPythonExecutionFactory) private readonly exeFactory: IPythonExecutionFactory, diff --git a/src/test/datascience/kernelFinder.unit.test.ts b/src/test/datascience/kernelFinder.unit.test.ts index 696864406153..bf37540a375c 100644 --- a/src/test/datascience/kernelFinder.unit.test.ts +++ b/src/test/datascience/kernelFinder.unit.test.ts @@ -11,7 +11,7 @@ import { Uri } from 'vscode'; import { IWorkspaceService } from '../../client/common/application/types'; import { IPlatformService } from '../../client/common/platform/types'; import { PythonExecutionFactory } from '../../client/common/process/pythonExecutionFactory'; -import { IExtensionContext, IInstaller, IPathUtils, Resource } from '../../client/common/types'; +import { IExtensionContext, IPathUtils, Resource } from '../../client/common/types'; import { Architecture } from '../../client/common/utils/platform'; import { IEnvironmentVariablesProvider } from '../../client/common/variables/types'; import { defaultKernelSpecName } from '../../client/datascience/jupyter/kernels/helpers'; @@ -30,7 +30,6 @@ suite('Kernel Finder', () => { let pathUtils: typemoq.IMock; let context: typemoq.IMock; let envVarsProvider: typemoq.IMock; - let installer: IInstaller; let workspaceService: IWorkspaceService; let kernelFinder: IKernelFinder; let activeInterpreter: PythonEnvironment; @@ -83,9 +82,6 @@ suite('Kernel Finder', () => { context.setup((c) => c.globalStoragePath).returns(() => './'); fileSystem = typemoq.Mock.ofType(); - installer = mock(); - when(installer.isInstalled(anything(), anything())).thenResolve(true); - platformService = typemoq.Mock.ofType(); platformService.setup((ps) => ps.isWindows).returns(() => true); platformService.setup((ps) => ps.isMac).returns(() => true); @@ -325,7 +321,6 @@ suite('Kernel Finder', () => { platformService.object, fileSystem.object, pathUtils.object, - instance(installer), context.object, instance(workspaceService), instance(executionFactory), @@ -408,7 +403,6 @@ suite('Kernel Finder', () => { platformService.object, fileSystem.object, pathUtils.object, - instance(installer), context.object, instance(workspaceService), instance(executionFactory), From e34605dcadf626363488d09389b4481efb4350ee Mon Sep 17 00:00:00 2001 From: Ian Huff Date: Wed, 16 Sep 2020 17:07:59 -0700 Subject: [PATCH 4/5] update unit tests --- .../datascience/jupyter/kernels/kernelSelector.unit.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/datascience/jupyter/kernels/kernelSelector.unit.test.ts b/src/test/datascience/jupyter/kernels/kernelSelector.unit.test.ts index 515e6c4984de..2b15157b520b 100644 --- a/src/test/datascience/jupyter/kernels/kernelSelector.unit.test.ts +++ b/src/test/datascience/jupyter/kernels/kernelSelector.unit.test.ts @@ -31,7 +31,7 @@ import { LiveKernelModel } from '../../../../client/datascience/jupyter/kernels/types'; import { IKernelFinder } from '../../../../client/datascience/kernel-launcher/types'; -import { IJupyterSessionManager } from '../../../../client/datascience/types'; +import { IJupyterSessionManager, KernelInterpreterDependencyResponse } from '../../../../client/datascience/types'; import { IInterpreterService } from '../../../../client/interpreter/contracts'; import { InterpreterService } from '../../../../client/interpreter/interpreterService'; import { EnvironmentType, PythonEnvironment } from '../../../../client/pythonEnvironments/info'; @@ -72,6 +72,9 @@ suite('DataScience - KernelSelector', () => { kernelSelectionProvider = mock(KernelSelectionProvider); appShell = mock(ApplicationShell); dependencyService = mock(KernelDependencyService); + when(dependencyService.installMissingDependencies(anything(), anything())).thenResolve( + KernelInterpreterDependencyResponse.ok + ); interpreterService = mock(InterpreterService); kernelFinder = mock(); const jupyterSessionManagerFactory = mock(JupyterSessionManagerFactory); From 2cd2f991323babd02b653c61c1355e883ea7240e Mon Sep 17 00:00:00 2001 From: Ian Huff Date: Wed, 16 Sep 2020 17:10:24 -0700 Subject: [PATCH 5/5] add news --- news/2 Fixes/13956.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/2 Fixes/13956.md diff --git a/news/2 Fixes/13956.md b/news/2 Fixes/13956.md new file mode 100644 index 000000000000..a0621c5ee7d1 --- /dev/null +++ b/news/2 Fixes/13956.md @@ -0,0 +1 @@ +Correctly install ipykernel when launching from an interpreter. \ No newline at end of file