Skip to content

Commit 18d6b65

Browse files
authored
Add getMatchingInterpreter API (#8796)
* Add getMatchingInterpreter as the API to go between kernel spec and interpreter * Fix functional tests * Linter error
1 parent 7cbd1c2 commit 18d6b65

File tree

8 files changed

+56
-19
lines changed

8 files changed

+56
-19
lines changed

src/client/datascience/jupyter/jupyterNotebook.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
3-
'use strict';
43
import '../../common/extensions';
54

65
// tslint:disable-next-line: no-require-imports
@@ -23,6 +22,7 @@ import { createDeferred, Deferred, waitForPromise } from '../../common/utils/asy
2322
import * as localize from '../../common/utils/localize';
2423
import { noop } from '../../common/utils/misc';
2524
import { StopWatch } from '../../common/utils/stopWatch';
25+
import { IInterpreterService, PythonInterpreter } from '../../interpreter/contracts';
2626
import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
2727
import { generateCells } from '../cellFactory';
2828
import { CellMatcher } from '../cellMatcher';
@@ -145,6 +145,7 @@ export class JupyterNotebookBase implements INotebook {
145145
private _disposed: boolean = false;
146146
private _workingDirectory: string | undefined;
147147
private _loggers: INotebookExecutionLogger[] = [];
148+
private interpreter: PythonInterpreter | undefined;
148149

149150
constructor(
150151
_liveShare: ILiveShareApi, // This is so the liveshare mixin works
@@ -157,11 +158,15 @@ export class JupyterNotebookBase implements INotebook {
157158
resource: Uri,
158159
private getDisposedError: () => Error,
159160
private workspace: IWorkspaceService,
160-
private applicationService: IApplicationShell
161+
private applicationService: IApplicationShell,
162+
private interpreterService: IInterpreterService
161163
) {
162164
this.sessionStartTime = Date.now();
163165
this._resource = resource;
164166
this._loggers = [...loggers];
167+
// Save our interpreter and don't change it. Later on when kernel changes
168+
// are possible, recompute it.
169+
this.interpreterService.getActiveInterpreter(resource).then(i => this.interpreter = i).ignoreErrors();
165170
}
166171

167172
public get server(): INotebookServer {
@@ -438,7 +443,7 @@ export class JupyterNotebookBase implements INotebook {
438443
cursor_pos: offsetInCode
439444
}), cancelToken);
440445
if (result && result.content) {
441-
if ('matches' in result.content){
446+
if ('matches' in result.content) {
442447
return {
443448
matches: result.content.matches,
444449
cursor: {
@@ -450,7 +455,7 @@ export class JupyterNotebookBase implements INotebook {
450455
} else {
451456
return {
452457
matches: [],
453-
cursor : {start: 0, end: 0},
458+
cursor: { start: 0, end: 0 },
454459
metadata: []
455460
};
456461
}
@@ -461,6 +466,10 @@ export class JupyterNotebookBase implements INotebook {
461466
throw new Error(localize.DataScience.sessionDisposed());
462467
}
463468

469+
public async getMatchingInterpreter(): Promise<PythonInterpreter | undefined> {
470+
return this.interpreter;
471+
}
472+
464473
private finishUncompletedCells() {
465474
const copyPending = [...this.pendingCellSubscriptions];
466475
copyPending.forEach(c => c.cancel());
@@ -734,7 +743,7 @@ export class JupyterNotebookBase implements INotebook {
734743
.catch(e => {
735744
// @jupyterlab/services throws a `Canceled` error when the kernel is interrupted.
736745
// Such an error must be ignored.
737-
if (e && e instanceof Error && e.message === 'Canceled'){
746+
if (e && e instanceof Error && e.message === 'Canceled') {
738747
subscriber.complete(this.sessionStartTime);
739748
} else {
740749
subscriber.error(this.sessionStartTime, e);

src/client/datascience/jupyter/jupyterServerFactory.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import * as vsls from 'vsls/vscode';
1111

1212
import { IApplicationShell, ILiveShareApi, IWorkspaceService } from '../../common/application/types';
1313
import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry } from '../../common/types';
14+
import { IInterpreterService } from '../../interpreter/contracts';
1415
import {
1516
IConnection,
1617
IDataScience,
@@ -38,7 +39,8 @@ type JupyterServerClassType = {
3839
sessionManager: IJupyterSessionManagerFactory,
3940
workspaceService: IWorkspaceService,
4041
loggers: INotebookExecutionLogger[],
41-
appShell: IApplicationShell
42+
appShell: IApplicationShell,
43+
interpreterService: IInterpreterService
4244
): IJupyterServerInterface;
4345
};
4446
// tslint:enable:callable-types
@@ -59,7 +61,8 @@ export class JupyterServerFactory implements INotebookServer, ILiveShareHasRole
5961
@inject(IJupyterSessionManagerFactory) sessionManager: IJupyterSessionManagerFactory,
6062
@inject(IWorkspaceService) workspaceService: IWorkspaceService,
6163
@multiInject(INotebookExecutionLogger) @optional() loggers: INotebookExecutionLogger[] | undefined,
62-
@inject(IApplicationShell) appShell: IApplicationShell) {
64+
@inject(IApplicationShell) appShell: IApplicationShell,
65+
@inject(IInterpreterService) interpreterService: IInterpreterService) {
6366
this.serverFactory = new RoleBasedFactory<IJupyterServerInterface, JupyterServerClassType>(
6467
liveShare,
6568
HostJupyterServer,
@@ -72,7 +75,8 @@ export class JupyterServerFactory implements INotebookServer, ILiveShareHasRole
7275
sessionManager,
7376
workspaceService,
7477
loggers ? loggers : [],
75-
appShell
78+
appShell,
79+
interpreterService
7680
);
7781
}
7882

src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,16 @@ import { IConfigurationService, IDisposableRegistry } from '../../../common/type
1313
import { createDeferred } from '../../../common/utils/async';
1414
import * as localize from '../../../common/utils/localize';
1515
import { noop } from '../../../common/utils/misc';
16+
import { PythonInterpreter } from '../../../interpreter/contracts';
1617
import { LiveShare, LiveShareCommands } from '../../constants';
17-
import { ICell, INotebook, INotebookCompletion, INotebookExecutionLogger, INotebookServer, InterruptResult } from '../../types';
18+
import {
19+
ICell,
20+
INotebook,
21+
INotebookCompletion,
22+
INotebookExecutionLogger,
23+
INotebookServer,
24+
InterruptResult
25+
} from '../../types';
1826
import { LiveShareParticipantDefault, LiveShareParticipantGuest } from './liveShareParticipantMixin';
1927
import { ResponseQueue } from './responseQueue';
2028
import { IExecuteObservableResponse, ILiveShareParticipant, IServerResponse } from './types';
@@ -172,6 +180,10 @@ export class GuestJupyterNotebook
172180
}
173181
}
174182

183+
public getMatchingInterpreter(): Promise<PythonInterpreter | undefined> {
184+
return Promise.resolve(undefined);
185+
}
186+
175187
private onServerResponse = (args: Object) => {
176188
const er = args as IExecuteObservableResponse;
177189
traceInfo(`Guest serverResponse ${er.pos} ${er.id}`);

src/client/datascience/jupyter/liveshare/hostJupyterNotebook.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { IApplicationShell, ILiveShareApi, IWorkspaceService } from '../../../co
1414
import { traceError } from '../../../common/logger';
1515
import { IConfigurationService, IDisposableRegistry } from '../../../common/types';
1616
import { createDeferred } from '../../../common/utils/async';
17+
import { IInterpreterService } from '../../../interpreter/contracts';
1718
import { Identifiers, LiveShare, LiveShareCommands } from '../../constants';
1819
import { IExecuteInfo } from '../../interactive-common/interactiveWindowTypes';
1920
import {
@@ -52,9 +53,10 @@ export class HostJupyterNotebook
5253
resource: vscode.Uri,
5354
getDisposedError: () => Error,
5455
workspace: IWorkspaceService,
55-
appService: IApplicationShell
56+
appService: IApplicationShell,
57+
interpreterService: IInterpreterService
5658
) {
57-
super(liveShare, session, configService, disposableRegistry, owner, launchInfo, loggers, resource, getDisposedError, workspace, appService);
59+
super(liveShare, session, configService, disposableRegistry, owner, launchInfo, loggers, resource, getDisposedError, workspace, appService, interpreterService);
5860
}
5961

6062
public dispose = async (): Promise<void> => {

src/client/datascience/jupyter/liveshare/hostJupyterServer.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { traceInfo } from '../../../common/logger';
1313
import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry } from '../../../common/types';
1414
import * as localize from '../../../common/utils/localize';
1515
import { StopWatch } from '../../../common/utils/stopWatch';
16+
import { IInterpreterService } from '../../../interpreter/contracts';
1617
import { sendTelemetryEvent } from '../../../telemetry';
1718
import { Identifiers, LiveShare, LiveShareCommands, RegExpValues, Telemetry } from '../../constants';
1819
import {
@@ -48,7 +49,9 @@ export class HostJupyterServer
4849
sessionManager: IJupyterSessionManagerFactory,
4950
private workspaceService: IWorkspaceService,
5051
loggers: INotebookExecutionLogger[],
51-
private appService: IApplicationShell) {
52+
private appService: IApplicationShell,
53+
private interpreterService: IInterpreterService
54+
) {
5255
super(liveShare, asyncRegistry, disposableRegistry, configService, sessionManager, loggers);
5356
}
5457

@@ -177,7 +180,8 @@ export class HostJupyterServer
177180
resource,
178181
this.getDisposedError.bind(this),
179182
this.workspaceService,
180-
this.appService);
183+
this.appService,
184+
this.interpreterService);
181185

182186
// Wait for it to be ready
183187
traceInfo(`Waiting for idle (session) ${this.id}`);

src/client/datascience/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ export interface INotebook extends IAsyncDisposable {
100100
getSysInfo(): Promise<ICell | undefined>;
101101
setMatplotLibStyle(useDark: boolean): Promise<void>;
102102
addLogger(logger: INotebookExecutionLogger): void;
103+
getMatchingInterpreter(): Promise<PythonInterpreter | undefined>;
103104
}
104105

105106
export interface INotebookServerOptions {

src/test/datascience/execution.unit.test.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ class MockJupyterNotebook implements INotebook {
123123
public async dispose(): Promise<void> {
124124
return Promise.resolve();
125125
}
126+
127+
public getMatchingInterpreter(): Promise<PythonInterpreter | undefined> {
128+
return Promise.resolve(undefined);
129+
}
126130
}
127131

128132
// tslint:disable:no-any no-http-string no-multiline-string max-func-body-length
@@ -538,7 +542,7 @@ suite('Jupyter Execution', async () => {
538542
function createExecution(activeInterpreter: PythonInterpreter, notebookStdErr?: string[], skipSearch?: boolean): JupyterExecutionFactory {
539543
return createExecutionAndReturnProcessService(activeInterpreter, notebookStdErr, skipSearch).jupyterExecutionFactory;
540544
}
541-
function createExecutionAndReturnProcessService(activeInterpreter: PythonInterpreter, notebookStdErr?: string[], skipSearch?: boolean, runInDocker?: boolean): {workingPythonExecutionService: TypeMoq.IMock<IPythonExecutionService>; jupyterExecutionFactory: JupyterExecutionFactory} {
545+
function createExecutionAndReturnProcessService(activeInterpreter: PythonInterpreter, notebookStdErr?: string[], skipSearch?: boolean, runInDocker?: boolean): { workingPythonExecutionService: TypeMoq.IMock<IPythonExecutionService>; jupyterExecutionFactory: JupyterExecutionFactory } {
542546
// Setup defaults
543547
when(interpreterService.onDidChangeInterpreter).thenReturn(dummyEvent.event);
544548
when(interpreterService.getActiveInterpreter()).thenResolve(activeInterpreter);
@@ -547,7 +551,7 @@ suite('Jupyter Execution', async () => {
547551
when(interpreterService.getInterpreterDetails(match('/foo/baz/python.exe'))).thenResolve(missingKernelPython);
548552
when(interpreterService.getInterpreterDetails(match('/bar/baz/python.exe'))).thenResolve(missingNotebookPython);
549553
when(interpreterService.getInterpreterDetails(argThat(o => !o.includes || !o.includes('python')))).thenReject('Unknown interpreter');
550-
if (runInDocker){
554+
if (runInDocker) {
551555
when(fileSystem.readFile('/proc/self/cgroup')).thenResolve('hello docker world');
552556
}
553557
// Create our working python and process service.
@@ -692,7 +696,7 @@ suite('Jupyter Execution', async () => {
692696
}
693697

694698
test('Working notebook and commands found', async () => {
695-
const { workingPythonExecutionService, jupyterExecutionFactory} = createExecutionAndReturnProcessService(workingPython);
699+
const { workingPythonExecutionService, jupyterExecutionFactory } = createExecutionAndReturnProcessService(workingPython);
696700
when(executionFactory.createDaemon(deepEqual({ daemonModule: PythonDaemonModule, pythonPath: workingPython.path }))).thenResolve(workingPythonExecutionService.object);
697701

698702
await assert.eventually.equal(jupyterExecutionFactory.isNotebookSupported(), true, 'Notebook not supported');
@@ -705,7 +709,7 @@ suite('Jupyter Execution', async () => {
705709
}).timeout(10000);
706710

707711
test('Includes correct args for running in docker', async () => {
708-
const { workingPythonExecutionService, jupyterExecutionFactory} = createExecutionAndReturnProcessService(workingPython, undefined, undefined, true);
712+
const { workingPythonExecutionService, jupyterExecutionFactory } = createExecutionAndReturnProcessService(workingPython, undefined, undefined, true);
709713
when(executionFactory.createDaemon(deepEqual({ daemonModule: PythonDaemonModule, pythonPath: workingPython.path }))).thenResolve(workingPythonExecutionService.object);
710714

711715
await assert.eventually.equal(jupyterExecutionFactory.isNotebookSupported(), true, 'Notebook not supported');
@@ -732,7 +736,7 @@ suite('Jupyter Execution', async () => {
732736
test('Slow notebook startups throws exception', async () => {
733737
const daemonService = mock(PythonDaemonExecutionService);
734738
const stdErr = 'Failure';
735-
const proc = {on: noop} as any as ChildProcess;
739+
const proc = { on: noop } as any as ChildProcess;
736740
const out = new Observable<Output<string>>(s => s.next({ source: 'stderr', out: stdErr }));
737741
when(daemonService.execModuleObservable(anything(), anything(), anything())).thenReturn({ dispose: noop, proc: proc, out });
738742
when(executionFactory.createDaemon(deepEqual({ daemonModule: PythonDaemonModule, pythonPath: workingPython.path }))).thenResolve(instance(daemonService));
@@ -856,7 +860,7 @@ suite('Jupyter Execution', async () => {
856860
}).timeout(20_000);
857861

858862
test('Kernelspec is deleted on exit', async () => {
859-
const { workingPythonExecutionService, jupyterExecutionFactory} = createExecutionAndReturnProcessService(missingKernelPython);
863+
const { workingPythonExecutionService, jupyterExecutionFactory } = createExecutionAndReturnProcessService(missingKernelPython);
860864
when(interpreterService.getActiveInterpreter(undefined)).thenResolve(missingKernelPython);
861865
when(executionFactory.createDaemon(deepEqual({ daemonModule: PythonDaemonModule, pythonPath: missingKernelPython.path }))).thenResolve(workingPythonExecutionService.object);
862866

src/test/datascience/mockJupyterManager.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ export class MockJupyterManager implements IJupyterSessionManager {
7777
// Setup our interpreter service
7878
this.interpreterService.setup(i => i.onDidChangeInterpreter).returns(() => this.changedInterpreterEvent.event);
7979
this.interpreterService.setup(i => i.getActiveInterpreter()).returns(() => Promise.resolve(this.activeInterpreter));
80+
this.interpreterService.setup(i => i.getActiveInterpreter(TypeMoq.It.isAny())).returns(() => Promise.resolve(this.activeInterpreter));
8081
this.interpreterService.setup(i => i.getInterpreters()).returns(() => Promise.resolve(this.installedInterpreters));
8182
this.interpreterService.setup(i => i.getInterpreterDetails(TypeMoq.It.isAnyString())).returns((p) => {
8283
const found = this.installedInterpreters.find(i => i.path === p);

0 commit comments

Comments
 (0)