Skip to content

Commit d0158c3

Browse files
Don't save active interpreter in kernel finder (#11531)
1 parent 262c14f commit d0158c3

File tree

3 files changed

+51
-20
lines changed

3 files changed

+51
-20
lines changed

news/2 Fixes/11530.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
For direct kernel launch correctly detect if interpreter has changed since last launch.

src/client/datascience/kernel-launcher/kernelFinder.ts

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ export function findIndexOfConnectionFile(kernelSpec: Readonly<IJupyterKernelSpe
4646
// Before returning the IJupyterKernelSpec it makes sure that ipykernel is installed into the kernel spec interpreter
4747
@injectable()
4848
export class KernelFinder implements IKernelFinder {
49-
private activeInterpreter: PythonInterpreter | undefined;
5049
private cache: string[] = [];
5150

5251
constructor(
@@ -69,6 +68,7 @@ export class KernelFinder implements IKernelFinder {
6968
): Promise<IJupyterKernelSpec> {
7069
this.cache = await this.readCache();
7170
let foundKernel: IJupyterKernelSpec | undefined;
71+
const activeInterpreter = await this.interpreterService.getActiveInterpreter(resource);
7272

7373
if (kernelName && !kernelName.includes(defaultSpecName)) {
7474
let kernelSpec = await this.searchCache(kernelName);
@@ -77,7 +77,10 @@ export class KernelFinder implements IKernelFinder {
7777
return kernelSpec;
7878
}
7979

80-
kernelSpec = await this.getKernelSpecFromActiveInterpreter(resource, kernelName);
80+
// Check in active interpreter first
81+
if (activeInterpreter) {
82+
kernelSpec = await this.getKernelSpecFromActiveInterpreter(kernelName, activeInterpreter);
83+
}
8184

8285
if (kernelSpec) {
8386
this.writeCache(this.cache).ignoreErrors();
@@ -98,9 +101,9 @@ export class KernelFinder implements IKernelFinder {
98101
result = both[0] ? both[0] : both[1];
99102
}
100103

101-
foundKernel = result ? result : await this.getDefaultKernelSpec(resource);
104+
foundKernel = result ? result : await this.getDefaultKernelSpec(activeInterpreter);
102105
} else {
103-
foundKernel = await this.getDefaultKernelSpec(resource);
106+
foundKernel = await this.getDefaultKernelSpec(activeInterpreter);
104107
}
105108

106109
this.writeCache(this.cache).ignoreErrors();
@@ -139,17 +142,13 @@ export class KernelFinder implements IKernelFinder {
139142
}
140143

141144
private async getKernelSpecFromActiveInterpreter(
142-
resource: Resource,
143-
kernelName: string
145+
kernelName: string,
146+
activeInterpreter: PythonInterpreter
144147
): Promise<IJupyterKernelSpec | undefined> {
145-
this.activeInterpreter = await this.interpreterService.getActiveInterpreter(resource);
146-
147-
if (this.activeInterpreter) {
148-
return this.getKernelSpecFromDisk(
149-
[path.join(this.activeInterpreter.sysPrefix, 'share', 'jupyter', 'kernels')],
150-
kernelName
151-
);
152-
}
148+
return this.getKernelSpecFromDisk(
149+
[path.join(activeInterpreter.sysPrefix, 'share', 'jupyter', 'kernels')],
150+
kernelName
151+
);
153152
}
154153

155154
private async findInterpreterPath(
@@ -206,17 +205,13 @@ export class KernelFinder implements IKernelFinder {
206205
return this.searchCache(kernelName);
207206
}
208207

209-
private async getDefaultKernelSpec(resource: Resource): Promise<IJupyterKernelSpec> {
210-
if (!this.activeInterpreter) {
211-
this.activeInterpreter = await this.interpreterService.getActiveInterpreter(resource);
212-
}
213-
208+
private async getDefaultKernelSpec(activeInterpreter?: PythonInterpreter): Promise<IJupyterKernelSpec> {
214209
// This creates a default kernel spec. When launched, 'python' argument will map to using the interpreter
215210
// associated with the current resource for launching.
216211
const defaultSpec: Kernel.ISpecModel = {
217212
name: defaultSpecName + Date.now().toString(),
218213
language: 'python',
219-
display_name: this.activeInterpreter?.displayName ? this.activeInterpreter.displayName : 'Python 3',
214+
display_name: activeInterpreter?.displayName ? activeInterpreter.displayName : 'Python 3',
220215
metadata: {},
221216
argv: ['python', '-m', 'ipykernel_launcher', '-f', connectionFilePlaceholder],
222217
env: {},

src/test/datascience/kernelFinder.unit.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ suite('Kernel Finder', () => {
9393

9494
activeInterpreter = {
9595
path: context.object.globalStoragePath,
96+
displayName: 'activeInterpreter',
9697
sysPrefix: '1',
9798
envName: '1',
9899
sysVersion: '3.1.1.1',
@@ -153,6 +154,40 @@ suite('Kernel Finder', () => {
153154
fileSystem.reset();
154155
});
155156

157+
test('No kernel name given. Default spec returned should match the interpreter selected.', async () => {
158+
setupFileSystem();
159+
160+
// Create a second active interpreter to return on the second call
161+
const activeInterpreter2 = {
162+
path: context.object.globalStoragePath,
163+
displayName: 'activeInterpreter2',
164+
sysPrefix: '1',
165+
envName: '1',
166+
sysVersion: '3.1.1.1',
167+
architecture: Architecture.x64,
168+
type: InterpreterType.Unknown
169+
};
170+
// Record a second call to getActiveInterpreter, will play after the first
171+
interpreterService
172+
.setup((is) => is.getActiveInterpreter(typemoq.It.isAny()))
173+
.returns(() => Promise.resolve(activeInterpreter2));
174+
175+
fileSystem
176+
.setup((fs) => fs.readFile(typemoq.It.isAnyString()))
177+
.returns((pathParam: string) => {
178+
if (pathParam.includes(cacheFile)) {
179+
return Promise.resolve('[]');
180+
}
181+
return Promise.resolve(JSON.stringify(kernel));
182+
});
183+
let spec = await kernelFinder.findKernelSpec(resource);
184+
expect(spec.display_name).to.equal(activeInterpreter.displayName);
185+
186+
spec = await kernelFinder.findKernelSpec(resource);
187+
expect(spec.display_name).to.equal(activeInterpreter2.displayName);
188+
fileSystem.reset();
189+
});
190+
156191
test('KernelSpec is in the interpreters', async () => {
157192
setupFileSystem();
158193
fileSystem

0 commit comments

Comments
 (0)