Skip to content

Commit 2ce96b0

Browse files
IanMatthewHuffIanMatthewHuff
authored
Use python execution service, not general process execution service for non-daemon raw start. (#12846)
1 parent 412f5f9 commit 2ce96b0

4 files changed

Lines changed: 43 additions & 21 deletions

File tree

‎news/2 Fixes/12821.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Use the given interpreter for launching the non-daemon python

‎src/client/datascience/kernel-launcher/kernelLauncherDaemon.ts‎

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import { ChildProcess } from 'child_process';
77
import { inject, injectable } from 'inversify';
88
import { IDisposable } from 'monaco-editor';
9-
import { ObservableExecutionResult } from '../../common/process/types';
9+
import { IPythonExecutionService, ObservableExecutionResult } from '../../common/process/types';
1010
import { Resource } from '../../common/types';
1111
import { noop } from '../../common/utils/misc';
1212
import { PythonInterpreter } from '../../pythonEnvironments/info';
@@ -27,15 +27,10 @@ export class PythonKernelLauncherDaemon implements IDisposable {
2727
resource: Resource,
2828
kernelSpec: IJupyterKernelSpec,
2929
interpreter?: PythonInterpreter
30-
): Promise<{ observableOutput: ObservableExecutionResult<string>; daemon: IPythonKernelDaemon } | undefined> {
30+
): Promise<{ observableOutput: ObservableExecutionResult<string>; daemon: IPythonKernelDaemon | undefined }> {
3131
const daemon = await this.daemonPool.get(resource, kernelSpec, interpreter);
3232

33-
// The daemon pool can return back a non-IPythonKernelDaemon if daemon service is not supported or for Python 2.
34-
// Use a check for the daemon.start function here before we call it.
35-
if (!daemon.start) {
36-
return undefined;
37-
}
38-
33+
// Check to see if we have the type of kernelspec that we expect
3934
const args = kernelSpec.argv.slice();
4035
const modulePrefixIndex = args.findIndex((item) => item === '-m');
4136
if (modulePrefixIndex === -1) {
@@ -49,11 +44,26 @@ export class PythonKernelLauncherDaemon implements IDisposable {
4944
const moduleArgs = args.slice(modulePrefixIndex + 2);
5045
const env = kernelSpec.env && Object.keys(kernelSpec.env).length > 0 ? kernelSpec.env : undefined;
5146

52-
const observableOutput = await daemon.start(moduleName, moduleArgs, { env });
53-
if (observableOutput.proc) {
54-
this.processesToDispose.push(observableOutput.proc);
47+
// The daemon pool can return back a non-IPythonKernelDaemon if daemon service is not supported or for Python 2.
48+
// Use a check for the daemon.start function here before we call it.
49+
if (!daemon.start) {
50+
// If we don't have a KernelDaemon here then we have an execution service and should use that to launch
51+
// Typing is a bit funk here, as createDaemon can return an execution service instead of the requested
52+
// daemon class
53+
// tslint:disable-next-line:no-any
54+
const executionService = (daemon as any) as IPythonExecutionService;
55+
56+
const observableOutput = executionService.execModuleObservable(moduleName, moduleArgs, { env });
57+
58+
return { observableOutput, daemon: undefined };
59+
} else {
60+
// In the case that we do have a kernel deamon, just return it
61+
const observableOutput = await daemon.start(moduleName, moduleArgs, { env });
62+
if (observableOutput.proc) {
63+
this.processesToDispose.push(observableOutput.proc);
64+
}
65+
return { observableOutput, daemon };
5566
}
56-
return { observableOutput, daemon };
5767
}
5868
public dispose() {
5969
while (this.processesToDispose.length) {

‎src/client/datascience/kernel-launcher/kernelProcess.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -224,13 +224,11 @@ export class KernelProcess implements IKernelProcess {
224224
this.interpreter
225225
);
226226

227-
if (kernelDaemonLaunch) {
228-
this.kernelDaemon = kernelDaemonLaunch.daemon;
229-
exeObs = kernelDaemonLaunch.observableOutput;
230-
}
227+
this.kernelDaemon = kernelDaemonLaunch.daemon;
228+
exeObs = kernelDaemonLaunch.observableOutput;
231229
}
232230

233-
// If we are not python or if we failed with our daemon launch just use the ProcessExecutionFactory
231+
// If we are not python just use the ProcessExecutionFactory
234232
if (!exeObs) {
235233
// First part of argument is always the executable.
236234
const executable = this._kernelSpec.argv[0];

‎src/test/datascience/kernel-launcher/kernelLauncherDaemon.unit.test.ts‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
import { assert } from 'chai';
55
import { anything, deepEqual, instance, mock, when } from 'ts-mockito';
6-
import { ObservableExecutionResult } from '../../../client/common/process/types';
6+
import { IPythonExecutionService, ObservableExecutionResult } from '../../../client/common/process/types';
77
import { ReadWrite } from '../../../client/common/types';
88
import { KernelDaemonPool } from '../../../client/datascience/kernel-launcher/kernelDaemonPool';
99
import { PythonKernelLauncherDaemon } from '../../../client/datascience/kernel-launcher/kernelLauncherDaemon';
@@ -61,10 +61,23 @@ suite('Data Science - Kernel Launcher Daemon', () => {
6161
assert.equal(daemonCreationOutput.daemon, instance(kernelDaemon));
6262
}
6363
});
64-
test('If our daemon pool return does not support start, return undefined', async () => {
65-
when(daemonPool.get(anything(), anything(), anything())).thenResolve({} as any);
64+
test('If our daemon pool returns an execution service, then use it and return the daemon as undefined', async () => {
65+
const executionService = mock<IPythonExecutionService>();
66+
when(
67+
executionService.execModuleObservable(
68+
'ipkernel_launcher',
69+
deepEqual(['-f', 'file.json']),
70+
deepEqual({ env: kernelSpec.env })
71+
)
72+
).thenReturn(instance(observableOutputForDaemon));
73+
// Make sure that it doesn't have a start function. Normally the proxy will pretend to have a start function, which we are checking for non-existance
74+
when((executionService as any).start).thenReturn(false);
75+
// Else ts-mockit doesn't allow us to return an instance of a mock as a return value from an async function.
76+
(instance(executionService) as any).then = undefined;
77+
when(daemonPool.get(anything(), anything(), anything())).thenResolve(instance(executionService) as any);
6678
const daemonCreationOutput = await launcher.launch(undefined, kernelSpec, interpreter);
6779

68-
assert.isUndefined(daemonCreationOutput);
80+
assert.equal(daemonCreationOutput.observableOutput, instance(observableOutputForDaemon));
81+
assert.isUndefined(daemonCreationOutput.daemon);
6982
});
7083
});

0 commit comments

Comments
 (0)