Skip to content

Commit 00c2229

Browse files
rchiodorchiodo
authored
Update kernel to allow conda environments (#11318)
* Working raw kernel with preliminary test * Fix up test * Add news entry * Fix up after merge * Get rid of extra vars in the execution factory * Add some more output to the timeout message Fixup some tests * One last unit test
1 parent c3c4777 commit 00c2229

20 files changed

Lines changed: 325 additions & 66 deletions

‎news/2 Fixes/11306.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Conda environments working with raw kernels.

‎src/client/datascience/interactive-common/interactiveBase.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1419,7 +1419,8 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
14191419
name: kernel.name,
14201420
language: kernel.language ?? 'python',
14211421
display_name: kernel.display_name ?? kernel.name,
1422-
argv: kernel.argv ?? []
1422+
argv: kernel.argv ?? [],
1423+
env: kernel.env
14231424
};
14241425
return this.updateNotebookOptions(kernelSpec, this._notebook?.getMatchingInterpreter());
14251426
}

‎src/client/datascience/jupyter/kernels/jupyterKernelSpec.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Licensed under the MIT License.
33
'use strict';
44
import type { Kernel } from '@jupyterlab/services';
5-
import type { JSONObject } from '@phosphor/coreutils';
65
import * as path from 'path';
76
import { CancellationToken } from 'vscode';
87
import { createPromiseFromCancellation } from '../../../common/cancellation';
@@ -16,9 +15,9 @@ export class JupyterKernelSpec implements IJupyterKernelSpec {
1615
public language: string;
1716
public path: string;
1817
public specFile: string | undefined;
18+
public readonly env: NodeJS.ProcessEnv | undefined;
1919
public display_name: string;
2020
public argv: string[];
21-
public readonly env?: JSONObject;
2221

2322
// tslint:disable-next-line: no-any
2423
public metadata?: Record<string, any> & { interpreter?: Partial<PythonInterpreter> };
@@ -31,7 +30,7 @@ export class JupyterKernelSpec implements IJupyterKernelSpec {
3130
this.display_name = specModel.display_name;
3231
this.metadata = specModel.metadata;
3332
// tslint:disable-next-line: no-any
34-
this.env = specModel.env as any;
33+
this.env = specModel.env as any; // JSONObject, but should match
3534
}
3635
}
3736

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ export class KernelFinder implements IKernelFinder {
207207
'-f',
208208
connectionFilePlaceholder
209209
],
210+
env: {},
210211
resources: {}
211212
};
212213
const kernelSpec = new JupyterKernelSpec(defaultSpec);

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

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { IPythonExecutionFactory } from '../../common/process/types';
1515
import { createDeferred, Deferred } from '../../common/utils/async';
1616
import * as localize from '../../common/utils/localize';
1717
import { noop } from '../../common/utils/misc';
18+
import { IInterpreterService } from '../../interpreter/contracts';
1819
import { IJupyterKernelSpec } from '../types';
1920
import { findIndexOfConnectionFile } from './kernelFinder';
2021
import { IKernelConnection, IKernelFinder, IKernelLauncher, IKernelProcess } from './types';
@@ -46,6 +47,7 @@ class KernelProcess implements IKernelProcess {
4647

4748
constructor(
4849
private executionFactory: IPythonExecutionFactory,
50+
private interpreterService: IInterpreterService,
4951
private file: IFileSystem,
5052
private _connection: IKernelConnection,
5153
private _kernelSpec: IJupyterKernelSpec
@@ -71,11 +73,17 @@ class KernelProcess implements IKernelProcess {
7173
const pythonPath = this._kernelSpec.metadata?.interpreter?.path || args[0];
7274
args.shift();
7375

74-
const executionService = await this.executionFactory.create({
76+
// Use that to find the matching interpeter.
77+
const matchingInterpreter = await this.interpreterService.getInterpreterDetails(pythonPath);
78+
79+
// Use that to create an execution service with the correct environment.
80+
const executionService = await this.executionFactory.createActivatedEnvironment({
7581
resource: undefined,
76-
pythonPath
82+
interpreter: matchingInterpreter
7783
});
78-
const exeObs = executionService.execObservable(args, {});
84+
85+
// Then launch that process, also merging in the environment in the kernelspec
86+
const exeObs = executionService.execObservable(args, { extraVariables: this._kernelSpec.env });
7987

8088
if (exeObs.proc) {
8189
exeObs.proc!.on('exit', (exitCode) => {
@@ -124,6 +132,7 @@ export class KernelLauncher implements IKernelLauncher {
124132
constructor(
125133
@inject(IKernelFinder) private kernelFinder: IKernelFinder,
126134
@inject(IPythonExecutionFactory) private executionFactory: IPythonExecutionFactory,
135+
@inject(IInterpreterService) private interpreterService: IInterpreterService,
127136
@inject(IFileSystem) private file: IFileSystem
128137
) {}
129138

@@ -141,7 +150,13 @@ export class KernelLauncher implements IKernelLauncher {
141150
}
142151

143152
const connection = await this.getKernelConnection();
144-
const kernelProcess = new KernelProcess(this.executionFactory, this.file, connection, kernelSpec);
153+
const kernelProcess = new KernelProcess(
154+
this.executionFactory,
155+
this.interpreterService,
156+
this.file,
157+
connection,
158+
kernelSpec
159+
);
145160
await kernelProcess.launch();
146161
return kernelProcess;
147162
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ export interface IKernelProcess extends IDisposable {
3131
readonly kernelSpec: Readonly<IJupyterKernelSpec>;
3232
exited: Event<number | null>;
3333
dispose(): void;
34-
launch(interpreter: InterpreterUri, kernelSpec: IJupyterKernelSpec): Promise<void>;
3534
}
3635

3736
export const IKernelFinder = Symbol('IKernelFinder');

‎src/client/datascience/types.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,7 @@ export interface IJupyterKernelSpec {
390390
name: string;
391391
language: string;
392392
path: string;
393+
env: NodeJS.ProcessEnv | undefined;
393394
/**
394395
* Kernel display name.
395396
*
@@ -404,10 +405,6 @@ export interface IJupyterKernelSpec {
404405
// tslint:disable-next-line: no-any
405406
readonly metadata?: Record<string, any> & { interpreter?: Partial<PythonInterpreter> };
406407
readonly argv: string[];
407-
/**
408-
* A dictionary of environment variables to set for the kernel.
409-
*/
410-
readonly env?: JSONObject;
411408
}
412409

413410
export const INotebookImporter = Symbol('INotebookImporter');

‎src/datascience-ui/ipywidgets/container.tsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,13 @@ export class WidgetManagerComponent extends React.Component<Props> {
219219
setTimeout(() => {
220220
// tslint:disable-next-line: no-console
221221
console.error(`Timeout waiting to get widget source for ${moduleName}, ${moduleVersion}`);
222-
this.handleLoadError('<class>', moduleName, moduleVersion, new Error('Timeout'), true).ignoreErrors();
222+
this.handleLoadError(
223+
'<class>',
224+
moduleName,
225+
moduleVersion,
226+
new Error(`Timeout getting ${moduleName}:${moduleVersion}`),
227+
true
228+
).ignoreErrors();
223229
if (deferred) {
224230
deferred.resolve();
225231
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -957,7 +957,8 @@ suite('Jupyter Execution', async () => {
957957
display_name: 'hello',
958958
language: PYTHON_LANGUAGE,
959959
name: 'hello',
960-
path: ''
960+
path: '',
961+
env: undefined
961962
};
962963
when(
963964
kernelSelector.getKernelForLocalConnection(anything(), anything(), anything(), anything(), anything())

‎src/test/datascience/ipywidgets/localWidgetScriptSourceProvider.unit.test.ts‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,10 @@ suite('Data Science - ipywidget - Local Widget Script Source', () => {
112112
metadata: { interpreter: { sysPrefix, path: 'pythonPath' } }
113113
} as any);
114114
when(fs.search(anything(), anything())).thenResolve([
115-
path.join('widget1', 'index.js'),
116-
path.join('widget2', 'index.js'),
117-
path.join('widget3', 'index.js')
115+
// In order to match the real behavior, don't use join here
116+
'widget1/index.js',
117+
'widget2/index.js',
118+
'widget3/index.js'
118119
]);
119120

120121
const value = await scriptSourceProvider.getWidgetScriptSource('widget2', '1');
@@ -138,9 +139,10 @@ suite('Data Science - ipywidget - Local Widget Script Source', () => {
138139
metadata: { interpreter: { sysPrefix, path: 'pythonPath' } }
139140
} as any);
140141
when(fs.search(anything(), anything())).thenResolve([
141-
path.join('widget1', 'index.js'),
142-
path.join('widget2', 'index.js'),
143-
path.join('widget3', 'index.js')
142+
// In order to match the real behavior, don't use join here
143+
'widget1/index.js',
144+
'widget2/index.js',
145+
'widget3/index.js'
144146
]);
145147

146148
const value = await scriptSourceProvider.getWidgetScriptSource('widget1', '1');
@@ -162,9 +164,10 @@ suite('Data Science - ipywidget - Local Widget Script Source', () => {
162164
metadata: { interpreter: { sysPrefix, path: 'pythonPath' } }
163165
} as any);
164166
when(fs.search(anything(), anything())).thenResolve([
165-
path.join('widget1', 'index.js'),
166-
path.join('widget2', 'index.js'),
167-
path.join('widget3', 'index.js')
167+
// In order to match the real behavior, don't use join here
168+
'widget1/index.js',
169+
'widget2/index.js',
170+
'widget3/index.js'
168171
]);
169172

170173
const value = await scriptSourceProvider.getWidgetScriptSource('widgetNotFound', '1');

0 commit comments

Comments
 (0)