Skip to content

Commit

Permalink
Separate Kernel Specs & Python envs in kernel pick (#11926)
Browse files Browse the repository at this point in the history
* Separate Kernel Specs & Python envs in kernel pick

* oops

* oops
  • Loading branch information
DonJayamanne authored Nov 7, 2022
1 parent 60a9c28 commit d561d93
Show file tree
Hide file tree
Showing 11 changed files with 782 additions and 135 deletions.
4 changes: 4 additions & 0 deletions package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -1131,6 +1131,10 @@
"jupyter.configuration.jupyter.diagnostics.reservedPythonNames.enabled.markdownDescription": "Whether to show warnings about Python file names that could potentially [override Python packages](https://aka.ms/JupyterKernelStartFailureOverrideReservedName).",
"jupyter.configuration.jupyter.diagnostics.reservedPythonNames.exclude.markdownDescription": "Configure [glob patterns](https://code.visualstudio.com/docs/editor/codebasics#_advanced-search-options) for excluding files and folders from from being warned about [overriding Python packages](https://aka.ms/JupyterKernelStartFailureOverrideReservedName).",
"DataScience.localKernelFinderDisplayName": "Local Kernels",
"DataScience.localKernelSpecs": "Local Kernel specs",
"DataScience.pickLocalKernelSpecTitle": "Select a Local Kernel spec",
"DataScience.localPythonEnvironments": "Local Python Environments",
"DataScience.pickLocalKernelPythonEnvTitle": "Select a Local Python Environment",
"DataScience.universalRemoteKernelFinderDisplayName": "Remote - {0}",
"DataScience.remoteKernelFinderDisplayName": "Current Remote",
"DataScience.kernelPickerSelectSourceTitle": "Select Kernel Source",
Expand Down
3 changes: 2 additions & 1 deletion src/kernels/internalTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import { KernelConnectionMetadata } from './types';

export enum ContributedKernelFinderKind {
Remote = 'remote',
Local = 'local'
LocalKernelSpec = 'localKernelSpec',
LocalPythonEnvironment = 'localPythonEnvironment'
}

export interface IContributedKernelFinder<T extends KernelConnectionMetadata = KernelConnectionMetadata> {
Expand Down
7 changes: 1 addition & 6 deletions src/kernels/jupyter/finder/remoteKernelFinder.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,11 @@ import {
import { IInterpreterService } from '../../../platform/interpreter/contracts';
import { JupyterSessionManager } from '../session/jupyterSessionManager';
import { JupyterSessionManagerFactory } from '../session/jupyterSessionManagerFactory';
import { ILocalKernelFinder } from '../../raw/types';
import { ActiveKernelIdList, PreferredRemoteKernelIdProvider } from '../preferredRemoteKernelIdProvider';
import { IJupyterKernel, IJupyterRemoteCachedKernelValidator, IJupyterSessionManager } from '../types';
import { KernelFinder } from '../../kernelFinder';
import { NotebookProvider } from '../launcher/notebookProvider';
import { PythonExtensionChecker } from '../../../platform/api/pythonApi';
import { LocalKernelFinder } from '../../raw/finder/localKernelFinder.node';
import { IFileSystemNode } from '../../../platform/common/platform/types.node';
import { JupyterServerUriStorage } from '../launcher/serverUriStorage';
import { FileSystem } from '../../../platform/common/platform/fileSystem.node';
Expand All @@ -39,15 +37,14 @@ import { RemoteKernelSpecsCacheKey } from '../../common/commonFinder';
import { IKernelRankingHelper } from '../../../notebooks/controllers/types';
import { KernelRankingHelper } from '../../../notebooks/controllers/kernelRanking/kernelRankingHelper';
import { IExtensions } from '../../../platform/common/types';
import { takeTopRankKernel } from '../../raw/finder/localKernelFinder.unit.test';
import { createEventHandler, TestEventHandler } from '../../../test/common';
import { RemoteKernelFinder } from './remoteKernelFinder';
import { takeTopRankKernel } from '../../../notebooks/controllers/kernelRanking/kernelRankingHelper.unit.test';

suite(`Remote Kernel Finder`, () => {
let disposables: Disposable[] = [];
let preferredRemoteKernelIdProvider: PreferredRemoteKernelIdProvider;
let remoteKernelFinder: RemoteKernelFinder;
let localKernelFinder: ILocalKernelFinder;
let kernelFinder: KernelFinder;
let kernelRankHelper: IKernelRankingHelper;
let fs: IFileSystemNode;
Expand Down Expand Up @@ -137,8 +134,6 @@ suite(`Remote Kernel Finder`, () => {
const jupyterSessionManagerFactory = mock(JupyterSessionManagerFactory);
when(jupyterSessionManagerFactory.create(anything())).thenResolve(instance(jupyterSessionManager));
interpreterService = mock<IInterpreterService>();
localKernelFinder = mock(LocalKernelFinder);
when(localKernelFinder.kernels).thenReturn([]);
const extensionChecker = mock(PythonExtensionChecker);
when(extensionChecker.isPythonExtensionInstalled).thenReturn(true);
const notebookProvider = mock(NotebookProvider);
Expand Down

Large diffs are not rendered by default.

151 changes: 151 additions & 0 deletions src/kernels/raw/finder/contributedLocalKernelSpecFinder.node.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

'use strict';

import { inject, injectable } from 'inversify';
import { Event, EventEmitter } from 'vscode';
import { IKernelFinder, LocalKernelSpecConnectionMetadata } from '../../types';
import { LocalPythonAndRelatedNonPythonKernelSpecFinder } from './localPythonAndRelatedNonPythonKernelSpecFinder.node';
import { LocalKnownPathKernelSpecFinder } from './localKnownPathKernelSpecFinder.node';
import { traceInfo, traceDecoratorError, traceError, traceVerbose } from '../../../platform/logging';
import { IDisposableRegistry, IExtensions } from '../../../platform/common/types';
import { capturePerfTelemetry, Telemetry } from '../../../telemetry';
import { areObjectsWithUrisTheSame, noop } from '../../../platform/common/utils/misc';
import { KernelFinder } from '../../kernelFinder';
import { IExtensionSyncActivationService } from '../../../platform/activation/types';
import { DataScience } from '../../../platform/common/utils/localize';
import { IPythonExtensionChecker } from '../../../platform/api/types';
import { IInterpreterService } from '../../../platform/interpreter/contracts';
import { ContributedKernelFinderKind, IContributedKernelFinder } from '../../internalTypes';
import { PYTHON_LANGUAGE } from '../../../platform/common/constants';

// This class searches for local kernels.
// First it searches on a global persistent state, then on the installed python interpreters,
// and finally on the default locations that jupyter installs kernels on.
@injectable()
export class ContributedLocalKernelSpecFinder
implements IContributedKernelFinder<LocalKernelSpecConnectionMetadata>, IExtensionSyncActivationService
{
kind = ContributedKernelFinderKind.LocalKernelSpec;
id: string = ContributedKernelFinderKind.LocalKernelSpec;
displayName: string = DataScience.localKernelSpecs();

private _onDidChangeKernels = new EventEmitter<void>();
onDidChangeKernels: Event<void> = this._onDidChangeKernels.event;

private wasPythonInstalledWhenFetchingControllers = false;

private cache: LocalKernelSpecConnectionMetadata[] = [];

constructor(
@inject(LocalKnownPathKernelSpecFinder) private readonly nonPythonKernelFinder: LocalKnownPathKernelSpecFinder,
@inject(LocalPythonAndRelatedNonPythonKernelSpecFinder)
private readonly pythonKernelFinder: LocalPythonAndRelatedNonPythonKernelSpecFinder,
@inject(IKernelFinder) kernelFinder: KernelFinder,
@inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry,
@inject(IPythonExtensionChecker) private readonly extensionChecker: IPythonExtensionChecker,
@inject(IInterpreterService) private readonly interpreters: IInterpreterService,
@inject(IExtensions) private readonly extensions: IExtensions
) {
kernelFinder.registerKernelFinder(this);
}

activate() {
this.loadInitialState().then(noop, noop);

this.interpreters.onDidChangeInterpreters(
async () => this.updateCache().then(noop, noop),
this,
this.disposables
);
this.extensions.onDidChange(
() => {
// If we just installed the Python extension and we fetched the controllers, then fetch it again.
if (
!this.wasPythonInstalledWhenFetchingControllers &&
this.extensionChecker.isPythonExtensionInstalled
) {
this.updateCache().then(noop, noop);
}
},
this,
this.disposables
);
this.nonPythonKernelFinder.onDidChangeKernels(
() => this.updateCache().then(noop, noop),
this,
this.disposables
);
this.pythonKernelFinder.onDidChangeKernels(() => this.updateCache().then(noop, noop), this, this.disposables);
this.wasPythonInstalledWhenFetchingControllers = this.extensionChecker.isPythonExtensionInstalled;
}

private async loadInitialState() {
traceVerbose('LocalKernelFinder: load initial set of kernels');
await this.updateCache();
traceVerbose('LocalKernelFinder: loaded initial set of kernels');
}

@traceDecoratorError('List kernels failed')
@capturePerfTelemetry(Telemetry.KernelListingPerf, { kind: 'localKernelSpec' })
private async updateCache() {
try {
let kernels: LocalKernelSpecConnectionMetadata[] = [];
// Exclude python kernel specs (we'll get that from the pythonKernelFinder)
const kernelSpecs = this.nonPythonKernelFinder.kernels.filter((item) => {
if (this.extensionChecker.isPythonExtensionInstalled) {
return item.kernelSpec.language !== PYTHON_LANGUAGE;
}
return true;
});
const kernelSpecsFromPythonKernelFinder = this.pythonKernelFinder.kernels.filter(
(item) => item.kind === 'startUsingLocalKernelSpec'
) as LocalKernelSpecConnectionMetadata[];
kernels = kernels.concat(kernelSpecs).concat(kernelSpecsFromPythonKernelFinder);
await this.writeToCache(kernels);
} catch (ex) {
traceError('Exception Saving loaded kernels', ex);
}
}

public get kernels(): LocalKernelSpecConnectionMetadata[] {
return this.cache;
}
private filterKernels(kernels: LocalKernelSpecConnectionMetadata[]) {
return kernels.filter(({ kernelSpec }) => {
if (!kernelSpec) {
return true;
}
// Disable xeus python for now.
if (kernelSpec.argv[0].toLowerCase().endsWith('xpython')) {
traceInfo(`Hiding xeus kernelspec`);
return false;
}

return true;
});
}

private async writeToCache(values: LocalKernelSpecConnectionMetadata[]) {
try {
const oldValues = this.cache;
const uniqueIds = new Set<string>();
const uniqueKernels = values.filter((item) => {
if (uniqueIds.has(item.id)) {
return false;
}
uniqueIds.add(item.id);
return true;
});
this.cache = this.filterKernels(uniqueKernels);
if (areObjectsWithUrisTheSame(oldValues, this.cache)) {
return;
}

this._onDidChangeKernels.fire();
} catch (ex) {
traceError('LocalKernelFinder: Failed to write to cache', ex);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@

import { inject, injectable } from 'inversify';
import { Event, EventEmitter } from 'vscode';
import { IKernelFinder, LocalKernelConnectionMetadata } from '../../../kernels/types';
import { IKernelFinder, PythonKernelConnectionMetadata } from '../../../kernels/types';
import { LocalPythonAndRelatedNonPythonKernelSpecFinder } from './localPythonAndRelatedNonPythonKernelSpecFinder.node';
import { LocalKnownPathKernelSpecFinder } from './localKnownPathKernelSpecFinder.node';
import { traceInfo, traceDecoratorError, traceError, traceVerbose } from '../../../platform/logging';
import { traceDecoratorError, traceError, traceVerbose } from '../../../platform/logging';
import { IDisposableRegistry, IExtensions } from '../../../platform/common/types';
import { capturePerfTelemetry, Telemetry } from '../../../telemetry';
import { ILocalKernelFinder } from '../types';
import { waitForCondition } from '../../../platform/common/utils/async';
import { areObjectsWithUrisTheSame, noop } from '../../../platform/common/utils/misc';
import { KernelFinder } from '../../kernelFinder';
Expand All @@ -21,28 +19,28 @@ import * as localize from '../../../platform/common/utils/localize';
import { debounceAsync } from '../../../platform/common/utils/decorators';
import { IPythonExtensionChecker } from '../../../platform/api/types';
import { IInterpreterService } from '../../../platform/interpreter/contracts';
import { ContributedKernelFinderKind } from '../../internalTypes';
import { PYTHON_LANGUAGE } from '../../../platform/common/constants';
import { ContributedKernelFinderKind, IContributedKernelFinder } from '../../internalTypes';
import { KnownEnvironmentTypes } from '../../../platform/api/pythonApiTypes';

// This class searches for local kernels.
// First it searches on a global persistent state, then on the installed python interpreters,
// and finally on the default locations that jupyter installs kernels on.
@injectable()
export class LocalKernelFinder implements ILocalKernelFinder, IExtensionSyncActivationService {
kind = ContributedKernelFinderKind.Local;
id: string = 'local';
displayName: string = localize.DataScience.localKernelFinderDisplayName();
export class ContributedLocalPythonEnvFinder
implements IContributedKernelFinder<PythonKernelConnectionMetadata>, IExtensionSyncActivationService
{
kind = ContributedKernelFinderKind.LocalPythonEnvironment;
id: string = ContributedKernelFinderKind.LocalPythonEnvironment;
displayName: string = localize.DataScience.localPythonEnvironments();

private _onDidChangeKernels = new EventEmitter<void>();
onDidChangeKernels: Event<void> = this._onDidChangeKernels.event;

private wasPythonInstalledWhenFetchingControllers = false;

private cache: LocalKernelConnectionMetadata[] = [];
private cache: PythonKernelConnectionMetadata[] = [];

constructor(
@inject(LocalKnownPathKernelSpecFinder) private readonly nonPythonKernelFinder: LocalKnownPathKernelSpecFinder,
@inject(LocalPythonAndRelatedNonPythonKernelSpecFinder)
private readonly pythonKernelFinder: LocalPythonAndRelatedNonPythonKernelSpecFinder,
@inject(IKernelFinder) kernelFinder: KernelFinder,
Expand Down Expand Up @@ -78,11 +76,6 @@ export class LocalKernelFinder implements ILocalKernelFinder, IExtensionSyncActi
this,
this.disposables
);
this.nonPythonKernelFinder.onDidChangeKernels(
() => this.updateCache().then(noop, noop),
this,
this.disposables
);
this.pythonKernelFinder.onDidChangeKernels(() => this.updateCache().then(noop, noop), this, this.disposables);
this.wasPythonInstalledWhenFetchingControllers = this.extensionChecker.isPythonExtensionInstalled;
}
Expand All @@ -94,19 +87,13 @@ export class LocalKernelFinder implements ILocalKernelFinder, IExtensionSyncActi
}

@traceDecoratorError('List kernels failed')
@capturePerfTelemetry(Telemetry.KernelListingPerf, { kind: 'local' })
@capturePerfTelemetry(Telemetry.KernelListingPerf, { kind: 'localPython' })
private async updateCache() {
try {
let kernels: LocalKernelConnectionMetadata[] = [];
// Exclude python kernel specs (we'll get that from the pythonKernelFinder)
const nonPythonKernels = this.nonPythonKernelFinder.kernels.filter((item) => {
if (this.extensionChecker.isPythonExtensionInstalled) {
return item.kernelSpec.language !== PYTHON_LANGUAGE;
}
return true;
});
kernels = kernels.concat(nonPythonKernels).concat(this.pythonKernelFinder.kernels);
await this.writeToCache(kernels);
const pythonKernels = this.pythonKernelFinder.kernels.filter(
(item) => item.kind === 'startUsingPythonInterpreter'
) as PythonKernelConnectionMetadata[];
await this.writeToCache(pythonKernels);
} catch (ex) {
traceError('Exception Saving loaded kernels', ex);
}
Expand Down Expand Up @@ -149,25 +136,10 @@ export class LocalKernelFinder implements ILocalKernelFinder, IExtensionSyncActi
await this.updateCache();
}

public get kernels(): LocalKernelConnectionMetadata[] {
public get kernels(): PythonKernelConnectionMetadata[] {
return this.cache;
}
private filterKernels(kernels: LocalKernelConnectionMetadata[]) {
return kernels.filter(({ kernelSpec }) => {
if (!kernelSpec) {
return true;
}
// Disable xeus python for now.
if (kernelSpec.argv[0].toLowerCase().endsWith('xpython')) {
traceInfo(`Hiding xeus kernelspec`);
return false;
}

return true;
});
}

private async writeToCache(values: LocalKernelConnectionMetadata[]) {
private async writeToCache(values: PythonKernelConnectionMetadata[]) {
try {
const oldValues = this.cache;
const uniqueIds = new Set<string>();
Expand All @@ -178,7 +150,7 @@ export class LocalKernelFinder implements ILocalKernelFinder, IExtensionSyncActi
uniqueIds.add(item.id);
return true;
});
this.cache = this.filterKernels(uniqueKernels);
this.cache = uniqueKernels;
if (areObjectsWithUrisTheSame(oldValues, this.cache)) {
return;
}
Expand Down
4 changes: 0 additions & 4 deletions src/kernels/raw/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@

import { CancellationToken, Event } from 'vscode';
import { IAsyncDisposable, IDisplayOptions, IDisposable, Resource } from '../../platform/common/types';
import { IContributedKernelFinder } from '../internalTypes';
import {
IKernelConnectionSession,
KernelConnectionMetadata,
LocalKernelConnectionMetadata,
LocalKernelSpecConnectionMetadata,
PythonKernelConnectionMetadata
} from '../types';
Expand Down Expand Up @@ -58,8 +56,6 @@ export interface IKernelProcess extends IAsyncDisposable {
interrupt(): Promise<void>;
}

export interface ILocalKernelFinder extends IContributedKernelFinder<LocalKernelConnectionMetadata> {}

/**
* The daemon responsible for the Python Kernel.
*/
Expand Down
Loading

0 comments on commit d561d93

Please sign in to comment.