GeezerVison is showing github.com — Use different method for checking if kernelspec is available (#10114) · Cetlan/vscode-python@6c0f326
File treeExpand file treeCollapse file tree Open diff view settings client/datascience/jupyter/interpreter test/datascience/jupyter/interpreter
Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboard +7Lines changed: 7 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +# Check whether kernelspec module exists. | | 2 | +import sys | | 3 | +import jupyter_client | | 4 | +import jupyter_client.kernelspec | | 5 | + | | 6 | +sys.stdout.write(jupyter_client.__version__) | | 7 | +sys.stdout.flush() |
Collapse file Copy file name to clipboardExpand all lines: src/client/datascience/jupyter/interpreter/jupyterCommand.ts +43-21Lines changed: 43 additions & 21 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -222,37 +222,59 @@ export class InterpreterJupyterKernelSpecCommand extends InterpreterJupyterComma | 222 | 222 | return output; | 223 | 223 | } | 224 | 224 |
| 225 | | - // We're only interested in `python -m jupyter kernelspec list --json` | | 225 | + const defaultAction = () => { | | 226 | + if (exception) { | | 227 | + throw exception; | | 228 | + } | | 229 | + return output; | | 230 | + }; | | 231 | + | | 232 | + // We're only interested in `python -m jupyter kernelspec` | 226 | 233 | const interpreter = await this.interpreter(); | 227 | 234 | if ( | 228 | 235 | !interpreter || | 229 | 236 | this.moduleName.toLowerCase() !== 'jupyter' || | 230 | | - this.args.join(' ').toLowerCase() !== `-m jupyter ${JupyterCommands.KernelSpecCommand}`.toLowerCase() || | 231 | | - args.join(' ').toLowerCase() !== 'list --json' | | 237 | + this.args.join(' ').toLowerCase() !== `-m jupyter ${JupyterCommands.KernelSpecCommand}`.toLowerCase() | 232 | 238 | ) { | 233 | | - if (exception) { | 234 | | - throw exception; | 235 | | - } | 236 | | - return output; | | 239 | + return defaultAction(); | 237 | 240 | } | | 241 | + | | 242 | + // Otherwise try running a script instead. | 238 | 243 | try { | 239 | | - // Try getting kernels using python script, if that fails (even if there's output in stderr) rethrow original exception. | 240 | | - const activatedEnv = await this.pythonExecutionFactory.createActivatedEnvironment({ | 241 | | - interpreter, | 242 | | - bypassCondaExecution: true | 243 | | - }); | 244 | | - return activatedEnv.exec( | 245 | | - [path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'datascience', 'getJupyterKernels.py')], | 246 | | - { ...options, throwOnStdErr: true } | 247 | | - ); | | 244 | + if (args.join(' ').toLowerCase() === 'list --json') { | | 245 | + // Try getting kernels using python script, if that fails (even if there's output in stderr) rethrow original exception. | | 246 | + return this.getKernelSpecList(interpreter, options); | | 247 | + } else if (args.join(' ').toLowerCase() === '--version') { | | 248 | + // Try getting kernelspec version using python script, if that fails (even if there's output in stderr) rethrow original exception. | | 249 | + return this.getKernelSpecVersion(interpreter, options); | | 250 | + } | 248 | 251 | } catch (innerEx) { | 249 | 252 | traceError('Failed to get a list of the kernelspec using python script', innerEx); | 250 | | - // Rethrow original exception. | 251 | | - if (exception) { | 252 | | - throw exception; | 253 | | - } | 254 | | - return output; | 255 | 253 | } | | 254 | + return defaultAction(); | | 255 | + } | | 256 | + | | 257 | + private async getKernelSpecList(interpreter: PythonInterpreter, options: SpawnOptions) { | | 258 | + // Try getting kernels using python script, if that fails (even if there's output in stderr) rethrow original exception. | | 259 | + const activatedEnv = await this.pythonExecutionFactory.createActivatedEnvironment({ | | 260 | + interpreter, | | 261 | + bypassCondaExecution: true | | 262 | + }); | | 263 | + return activatedEnv.exec( | | 264 | + [path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'datascience', 'getJupyterKernels.py')], | | 265 | + { ...options, throwOnStdErr: true } | | 266 | + ); | | 267 | + } | | 268 | + private async getKernelSpecVersion(interpreter: PythonInterpreter, options: SpawnOptions) { | | 269 | + // Try getting kernels using python script, if that fails (even if there's output in stderr) rethrow original exception. | | 270 | + const activatedEnv = await this.pythonExecutionFactory.createActivatedEnvironment({ | | 271 | + interpreter, | | 272 | + bypassCondaExecution: true | | 273 | + }); | | 274 | + return activatedEnv.exec( | | 275 | + [path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'datascience', 'getJupyterKernelspecVersion.py')], | | 276 | + { ...options, throwOnStdErr: true } | | 277 | + ); | 256 | 278 | } | 257 | 279 | } | 258 | 280 |
| |
Collapse file Copy file name to clipboardExpand all lines: src/client/datascience/jupyter/interpreter/jupyterInterpreterDependencyService.ts +14-14Lines changed: 14 additions & 14 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -8,13 +8,13 @@ import { CancellationToken } from 'vscode'; | 8 | 8 | import { IApplicationShell } from '../../../common/application/types'; | 9 | 9 | import { Cancellation, createPromiseFromCancellation, wrapCancellationTokens } from '../../../common/cancellation'; | 10 | 10 | import { ProductNames } from '../../../common/installer/productNames'; | 11 | | -import { IPythonExecutionFactory } from '../../../common/process/types'; | 12 | 11 | import { IInstaller, InstallerResponse, Product } from '../../../common/types'; | 13 | 12 | import { Common, DataScience } from '../../../common/utils/localize'; | 14 | 13 | import { noop } from '../../../common/utils/misc'; | 15 | 14 | import { PythonInterpreter } from '../../../interpreter/contracts'; | 16 | 15 | import { sendTelemetryEvent } from '../../../telemetry'; | 17 | | -import { HelpLinks, Telemetry } from '../../constants'; | | 16 | +import { HelpLinks, JupyterCommands, Telemetry } from '../../constants'; | | 17 | +import { IJupyterCommandFactory } from '../../types'; | 18 | 18 | import { JupyterInstallError } from '../jupyterInstallError'; | 19 | 19 |
| 20 | 20 | export enum JupyterInterpreterDependencyResponse { | @@ -113,7 +113,7 @@ export class JupyterInterpreterDependencyService { | 113 | 113 | constructor( | 114 | 114 | @inject(IApplicationShell) private readonly applicationShell: IApplicationShell, | 115 | 115 | @inject(IInstaller) private readonly installer: IInstaller, | 116 | | - @inject(IPythonExecutionFactory) private readonly pythonExecFactory: IPythonExecutionFactory | | 116 | + @inject(IJupyterCommandFactory) private readonly commandFactory: IJupyterCommandFactory | 117 | 117 | ) {} | 118 | 118 | /** | 119 | 119 | * Configures the python interpreter to ensure it can run Jupyter server by installing any missing dependencies. | @@ -291,17 +291,16 @@ export class JupyterInterpreterDependencyService { | 291 | 291 | * @returns {Promise<boolean>} | 292 | 292 | * @memberof JupyterInterpreterConfigurationService | 293 | 293 | */ | 294 | | - private async isKernelSpecAvailable(interpreter: PythonInterpreter, token?: CancellationToken): Promise<boolean> { | 295 | | - const execService = await this.pythonExecFactory.createActivatedEnvironment({ | | 294 | + private async isKernelSpecAvailable(interpreter: PythonInterpreter, _token?: CancellationToken): Promise<boolean> { | | 295 | + const command = this.commandFactory.createInterpreterCommand( | | 296 | + JupyterCommands.KernelSpecCommand, | | 297 | + 'jupyter', | | 298 | + ['-m', 'jupyter', 'kernelspec'], | 296 | 299 | interpreter, | 297 | | - allowEnvironmentFetchExceptions: true, | 298 | | - bypassCondaExecution: true | 299 | | - }); | 300 | | - if (Cancellation.isCanceled(token)) { | 301 | | - return false; | 302 | | - } | 303 | | - return execService | 304 | | - .execModule('jupyter', ['kernelspec', '--version'], { throwOnStdErr: true }) | | 300 | + false | | 301 | + ); | | 302 | + return command | | 303 | + .exec(['--version'], { throwOnStdErr: true }) | 305 | 304 | .then(() => true) | 306 | 305 | .catch(() => { | 307 | 306 | sendTelemetryEvent(Telemetry.KernelSpecNotFound); | @@ -326,9 +325,10 @@ export class JupyterInterpreterDependencyService { | 326 | 325 | token?: CancellationToken | 327 | 326 | ): Promise<JupyterInterpreterDependencyResponse> { | 328 | 327 | if (await this.isKernelSpecAvailable(interpreter)) { | 329 | | - sendTelemetryEvent(Telemetry.JupyterInstalledButNotKernelSpecModule); | 330 | 328 | return JupyterInterpreterDependencyResponse.ok; | 331 | 329 | } | | 330 | + // Indicate no kernel spec module. | | 331 | + sendTelemetryEvent(Telemetry.JupyterInstalledButNotKernelSpecModule); | 332 | 332 | if (Cancellation.isCanceled(token)) { | 333 | 333 | return JupyterInterpreterDependencyResponse.cancel; | 334 | 334 | } | |
Collapse file Copy file name to clipboardExpand all lines: src/test/datascience/jupyter/interpreter/jupyterInterpreterDependencyService.unit.test.ts +19-18Lines changed: 19 additions & 18 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 4 | 4 | 'use strict'; | 5 | 5 |
| 6 | 6 | import { assert } from 'chai'; | 7 | | -import { anything, deepEqual, instance, mock, verify, when } from 'ts-mockito'; | | 7 | +import { anything, instance, mock, verify, when } from 'ts-mockito'; | 8 | 8 | import { ApplicationShell } from '../../../../client/common/application/applicationShell'; | 9 | 9 | import { IApplicationShell } from '../../../../client/common/application/types'; | 10 | 10 | import { ProductInstaller } from '../../../../client/common/installer/productInstaller'; | 11 | | -import { PythonExecutionFactory } from '../../../../client/common/process/pythonExecutionFactory'; | 12 | | -import { PythonExecutionService } from '../../../../client/common/process/pythonProcess'; | 13 | | -import { IPythonExecutionService } from '../../../../client/common/process/types'; | 14 | 11 | import { IInstaller, InstallerResponse, Product } from '../../../../client/common/types'; | 15 | 12 | import { DataScience } from '../../../../client/common/utils/localize'; | 16 | 13 | import { Architecture } from '../../../../client/common/utils/platform'; | | 14 | +import { | | 15 | + InterpreterJupyterKernelSpecCommand, | | 16 | + JupyterCommandFactory | | 17 | +} from '../../../../client/datascience/jupyter/interpreter/jupyterCommand'; | 17 | 18 | import { | 18 | 19 | JupyterInterpreterDependencyResponse, | 19 | 20 | JupyterInterpreterDependencyService | 20 | 21 | } from '../../../../client/datascience/jupyter/interpreter/jupyterInterpreterDependencyService'; | | 22 | +import { IJupyterCommand, IJupyterCommandFactory } from '../../../../client/datascience/types'; | 21 | 23 | import { InterpreterType, PythonInterpreter } from '../../../../client/interpreter/contracts'; | 22 | 24 |
| 23 | | -// tslint:disable: max-func-body-length | | 25 | +// tslint:disable: max-func-body-length no-any | 24 | 26 |
| 25 | 27 | suite('Data Science - Jupyter Interpreter Configuration', () => { | 26 | 28 | let configuration: JupyterInterpreterDependencyService; | 27 | 29 | let appShell: IApplicationShell; | 28 | 30 | let installer: IInstaller; | 29 | | - let pythonExecService: IPythonExecutionService; | | 31 | + let commandFactory: IJupyterCommandFactory; | | 32 | + let command: IJupyterCommand; | 30 | 33 | const pythonInterpreter: PythonInterpreter = { | 31 | 34 | path: '', | 32 | 35 | architecture: Architecture.Unknown, | @@ -37,19 +40,19 @@ suite('Data Science - Jupyter Interpreter Configuration', () => { | 37 | 40 | setup(() => { | 38 | 41 | appShell = mock(ApplicationShell); | 39 | 42 | installer = mock(ProductInstaller); | 40 | | - pythonExecService = mock(PythonExecutionService); | 41 | | - const pythonExecFactory = mock(PythonExecutionFactory); | 42 | | - when(pythonExecFactory.createActivatedEnvironment(anything())).thenResolve(instance(pythonExecService)); | 43 | | - // tslint:disable-next-line: no-any | 44 | | - instance(pythonExecService as any).then = undefined; | 45 | | - when(pythonExecService.execModule('jupyter', deepEqual(['kernelspec', '--version']), anything())).thenResolve({ | 46 | | - stdout: '' | 47 | | - }); | | 43 | + commandFactory = mock(JupyterCommandFactory); | | 44 | + command = mock(InterpreterJupyterKernelSpecCommand); | | 45 | + instance(commandFactory as any).then = undefined; | | 46 | + instance(command as any).then = undefined; | | 47 | + when( | | 48 | + commandFactory.createInterpreterCommand(anything(), anything(), anything(), anything(), anything()) | | 49 | + ).thenReturn(instance(command)); | | 50 | + when(command.exec(anything(), anything())).thenResolve({ stdout: '' }); | 48 | 51 |
| 49 | 52 | configuration = new JupyterInterpreterDependencyService( | 50 | 53 | instance(appShell), | 51 | 54 | instance(installer), | 52 | | - instance(pythonExecFactory) | | 55 | + instance(commandFactory) | 53 | 56 | ); | 54 | 57 | }); | 55 | 58 | test('Return ok if all dependencies are installed', async () => { | @@ -91,9 +94,7 @@ suite('Data Science - Jupyter Interpreter Configuration', () => { | 91 | 94 | // tslint:disable-next-line: no-any | 92 | 95 | DataScience.jupyterInstall() as any | 93 | 96 | ); | 94 | | - when(pythonExecService.execModule('jupyter', deepEqual(['kernelspec', '--version']), anything())).thenReject( | 95 | | - new Error('Not found') | 96 | | - ); | | 97 | + when(command.exec(anything(), anything())).thenReject(new Error('Not found')); | 97 | 98 | when(installer.install(anything(), anything(), anything())).thenResolve(InstallerResponse.Installed); | 98 | 99 |
| 99 | 100 | const response = await configuration.installMissingDependencies(pythonInterpreter); | |
|