Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure kernelspec is readonly #16090

Merged
merged 2 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions src/kernels/raw/finder/localKernelSpecFinderBase.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,18 +367,22 @@ export async function loadKernelSpec(
kernelJson.name = `${kernelJson.name}.${argv.join('#')}`;
}
}
kernelJson.metadata = kernelJson.metadata || {};
kernelJson.metadata.vscode = kernelJson.metadata.vscode || {};
if (!kernelJson.metadata.vscode.originalSpecFile) {
kernelJson.metadata.vscode.originalSpecFile = specPath.fsPath;
const metadata = (kernelJson.metadata as ReadWrite<typeof kernelJson.metadata>) || {};
metadata.vscode = metadata.vscode || {};
if (!metadata.vscode.originalSpecFile) {
metadata.vscode.originalSpecFile = specPath.fsPath;
}
if (!kernelJson.metadata.vscode.originalDisplayName) {
kernelJson.metadata.vscode.originalDisplayName = kernelJson.display_name;
if (!metadata.vscode.originalDisplayName) {
metadata.vscode.originalDisplayName = kernelJson.display_name;
}
if (kernelJson.metadata.originalSpecFile) {
kernelJson.metadata.vscode.originalSpecFile = kernelJson.metadata.originalSpecFile;
delete kernelJson.metadata.originalSpecFile;
if (metadata.originalSpecFile) {
metadata.vscode.originalSpecFile = metadata.originalSpecFile;
delete metadata.originalSpecFile;
}
kernelJson.metadata = metadata;

// Some registered kernel specs do not have a name, in this case use the last part of the path
kernelJson.name = kernelJson?.name || path.basename(path.dirname(specPath.fsPath));

const kernelSpec: IJupyterKernelSpec = new JupyterKernelSpec(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand All @@ -389,9 +393,6 @@ export async function loadKernelSpec(
getKernelRegistrationInfo(kernelJson)
);

// Some registered kernel specs do not have a name, in this case use the last part of the path
kernelSpec.name = kernelJson?.name || path.basename(path.dirname(specPath.fsPath));

// Possible user deleted the underlying interpreter.
const interpreterPath = interpreter?.uri.fsPath || kernelJson?.metadata?.interpreter?.path;
const isEmptyCondaEnv = isCondaEnvironmentWithoutPython(interpreter);
Expand Down
7 changes: 5 additions & 2 deletions src/kernels/raw/launcher/kernelEnvVarsService.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { inject, injectable } from 'inversify';
import { logger } from '../../../platform/logging';
import { getDisplayPath } from '../../../platform/common/platform/fs-paths';
import { IConfigurationService, Resource } from '../../../platform/common/types';
import { IConfigurationService, Resource, type ReadWrite } from '../../../platform/common/types';
import { noop } from '../../../platform/common/utils/misc';
import {
IEnvironmentVariablesService,
Expand Down Expand Up @@ -51,7 +51,10 @@ export class KernelEnvironmentVariablesService {
kernelSpec: IJupyterKernelSpec,
token?: CancellationToken
) {
let kernelEnv = kernelSpec.env && Object.keys(kernelSpec.env).length > 0 ? kernelSpec.env : undefined;
let kernelEnv =
kernelSpec.env && Object.keys(kernelSpec.env).length > 0
? (Object.assign({}, kernelSpec.env) as ReadWrite<NodeJS.ProcessEnv>)
: undefined;
const isPythonKernel = (kernelSpec.language || '').toLowerCase() === PYTHON_LANGUAGE;
// If an interpreter was not explicitly passed in, check for an interpreter path in the kernelspec to use
if (!interpreter && kernelSpec.interpreterPath) {
Expand Down
4 changes: 2 additions & 2 deletions src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { anything, instance, mock, when } from 'ts-mockito';
import { KernelEnvironmentVariablesService } from './kernelEnvVarsService.node';
import { IJupyterKernelSpec } from '../../types';
import { Uri } from 'vscode';
import { IConfigurationService, IWatchableJupyterSettings } from '../../../platform/common/types';
import { IConfigurationService, IWatchableJupyterSettings, type ReadWrite } from '../../../platform/common/types';
import { JupyterSettings } from '../../../platform/common/configSettings';

use(chaiAsPromised);
Expand All @@ -34,7 +34,7 @@ suite('Kernel Environment Variables Service', () => {
uri: pathFile,
id: pathFile.fsPath
};
let kernelSpec: IJupyterKernelSpec;
let kernelSpec: ReadWrite<IJupyterKernelSpec>;
let processEnv: NodeJS.ProcessEnv;
const originalEnvVars = Object.assign({}, process.env);
let processPath: string | undefined;
Expand Down
5 changes: 3 additions & 2 deletions src/kernels/raw/launcher/kernelProcess.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ import {
IOutputChannel,
IJupyterSettings,
IExperimentService,
Experiments
Experiments,
type ReadWrite
} from '../../../platform/common/types';
import { createDeferred, raceTimeout } from '../../../platform/common/utils/async';
import { DataScience } from '../../../platform/common/utils/localize';
Expand Down Expand Up @@ -117,7 +118,7 @@ export class KernelProcess extends ObservableDisposable implements IKernelProces
private exitEvent = new EventEmitter<{ exitCode?: number; reason?: string; stderr: string }>();
private launchedOnce?: boolean;
private connectionFile?: Uri;
private _launchKernelSpec?: IJupyterKernelSpec;
private _launchKernelSpec?: ReadWrite<IJupyterKernelSpec>;
private interrupter?: Interrupter;
private exitEventFired = false;
private readonly _kernelConnectionMetadata: Readonly<
Expand Down
58 changes: 30 additions & 28 deletions src/kernels/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -596,11 +596,11 @@ export interface IJupyterKernelSpec {
* @type {string}
* @memberof IJupyterKernel
*/
id?: string;
name: string;
language?: string;
executable: string; // argv[0] of the kernelspec.json
env?: NodeJS.ProcessEnv | undefined;
readonly id?: string;
readonly name: string;
readonly language?: string;
readonly executable: string; // argv[0] of the kernelspec.json
readonly env?: Readonly<NodeJS.ProcessEnv> | undefined;
/**
* Kernel display name.
*
Expand All @@ -612,45 +612,47 @@ export interface IJupyterKernelSpec {
* A dictionary of additional attributes about this kernel; used by clients to aid in kernel selection.
* Optionally storing the interpreter information in the metadata (helping extension search for kernels that match an interpereter).
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
readonly metadata?: Record<string, any> & {
vscode?: {
readonly metadata?: Readonly<
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Record<string, any> & {
vscode?: {
/**
* Optionally where the original user-created kernel spec json is located on the local FS.
* Remember when using non-raw we create kernelspecs from the original spec.
*/
originalSpecFile?: string;
/**
* E.g. assume we're loading a kernlespec for a default Python kernel, the name would be `python3`
* However we give this a completely different name, and at that point its not possible to determine
* whether this is a default kernel or not.
* Hence keep track of the original name in the metadata.
*/
originalDisplayName?: string;
};
interpreter?: Partial<PythonEnvironment_PythonApi>; // read from disk so has to follow old format
/**
* Optionally where the original user-created kernel spec json is located on the local FS.
* Remember when using non-raw we create kernelspecs from the original spec.
* @deprecated (use metadata.jupyter.originalSpecFile)
*/
originalSpecFile?: string;
/**
* E.g. assume we're loading a kernlespec for a default Python kernel, the name would be `python3`
* However we give this a completely different name, and at that point its not possible to determine
* whether this is a default kernel or not.
* Hence keep track of the original name in the metadata.
* Whether the kernels supports the debugger Protocol.
*/
originalDisplayName?: string;
};
interpreter?: Partial<PythonEnvironment_PythonApi>; // read from disk so has to follow old format
/**
* @deprecated (use metadata.jupyter.originalSpecFile)
*/
originalSpecFile?: string;
/**
* Whether the kernels supports the debugger Protocol.
*/
debugger?: boolean;
};
debugger?: boolean;
}
>;
readonly argv: string[];
/**
* Optionally where this kernel spec json is located on the local FS.
*/
specFile?: string;
readonly specFile?: string;
/**
* Optionally the Interpreter this kernel spec belongs to.
* You can have kernel specs that are scoped to an interpreter.
* E.g. if you have Python in `c:\Python\Python3.8`
* Then you could have kernels in `<sys.prefix folder for this interpreter>\share\jupyter\kernels`
* Plenty of conda packages ship kernels in this manner (beakerx, etc).
*/
interpreterPath?: string; // Has to be a string as old kernelspecs wrote it this way
readonly interpreterPath?: string; // Has to be a string as old kernelspecs wrote it this way
readonly interrupt_mode?: 'message' | 'signal';
/**
* Whether the kernelspec is registered by VS Code
Expand Down
Loading