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.

Sign up for GitHub

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

Delete ctrl if associated kernel is deleted #12148

Merged
merged 9 commits into from
Nov 23, 2022
Merged

Delete ctrl if associated kernel is deleted #12148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
737ae42
Delete ctrl if associated active env is deleted
DonJayamanne Nov 23, 2022
99a3d72
oops
DonJayamanne Nov 23, 2022
86bbfb1
Misc
DonJayamanne Nov 23, 2022
14e17f1
More changes
DonJayamanne Nov 23, 2022
e6f7053
oops
DonJayamanne Nov 23, 2022
fc5f657
oops
DonJayamanne Nov 23, 2022
7d7cbb2
oop\s
DonJayamanne Nov 23, 2022
19ca6f9
misc
DonJayamanne Nov 23, 2022
a87ef94
remove if
DonJayamanne Nov 23, 2022
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
6 changes: 5 additions & 1 deletion src/kernels/internalTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ export interface IContributedKernelFinder<T extends KernelConnectionMetadata = K
id: string;
displayName: string;
kind: ContributedKernelFinderKind;
onDidChangeKernels: Event<void>;
onDidChangeKernels: Event<{
added?: T[];
updated?: T[];
removed?: T[];
}>;
kernels: T[];
refresh(): Promise<void>;
}
32 changes: 27 additions & 5 deletions src/kernels/jupyter/finder/remoteKernelFinder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

'use strict';

import { CancellationToken, CancellationTokenSource, Event, EventEmitter, Memento } from 'vscode';
import { CancellationToken, CancellationTokenSource, EventEmitter, Memento } from 'vscode';
import { getKernelId } from '../../helpers';
import {
BaseKernelConnectionMetadata,
Expand Down Expand Up @@ -32,7 +32,7 @@ import { computeServerId } from '../jupyterUtils';
import { createPromiseFromCancellation } from '../../../platform/common/cancellation';
import { DisplayOptions } from '../../displayOptions';
import { isArray } from '../../../platform/common/utils/sysTypes';
import { noop } from '../../../platform/common/utils/misc';
import { areObjectsWithUrisTheSame, noop } from '../../../platform/common/utils/misc';
import { IApplicationEnvironment } from '../../../platform/common/application/types';
import { KernelFinder } from '../../kernelFinder';
import { removeOldCachedItems } from '../../common/commonFinder';
Expand Down Expand Up @@ -68,8 +68,8 @@ export class RemoteKernelFinder implements IRemoteKernelFinder, IDisposable {
private _cacheUpdateCancelTokenSource: CancellationTokenSource | undefined;
private cache: RemoteKernelConnectionMetadata[] = [];

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

private readonly disposables: IDisposable[] = [];

Expand Down Expand Up @@ -396,6 +396,15 @@ export class RemoteKernelFinder implements IRemoteKernelFinder, IDisposable {
`UniversalRemoteKernelFinder: Writing ${values.length} remote kernel connection metadata to cache`
);

const oldValues = this.cache;
const oldKernels = new Map(oldValues.map((item) => [item.id, item]));
const kernels = new Map(values.map((item) => [item.id, item]));
const added = values.filter((k) => !oldKernels.has(k.id));
const updated = values.filter(
(k) => oldKernels.has(k.id) && !areObjectsWithUrisTheSame(k, oldKernels.get(k.id))
);
const removed = oldValues.filter((k) => !kernels.has(k.id));

const key = this.cacheKey;
this.cache = values;
const serialized = values.map((item) => item.toJSON());
Expand All @@ -404,7 +413,20 @@ export class RemoteKernelFinder implements IRemoteKernelFinder, IDisposable {
this.globalState.update(key, { kernels: serialized, extensionVersion: this.env.extensionVersion })
]);

this._onDidChangeKernels.fire();
if (added.length || updated.length || removed.length) {
this._onDidChangeKernels.fire({ added, updated, removed });
}
traceVerbose(
`Updating cache with Remote kernels ${values
.map((k) => `${k.kind}:'${k.id} (interpreter id = ${k.interpreter?.id})'`)
.join(', ')}\n, Added = ${added
.map((k) => `${k.kind}:'${k.id} (interpreter id = ${k.interpreter?.id})'`)
.join(', ')}\n, Updated = ${updated
.map((k) => `${k.kind}:'${k.id} (interpreter id = ${k.interpreter?.id})'`)
.join(', ')}\n, Removed = ${removed
.map((k) => `${k.kind}:'${k.id} (interpreter id = ${k.interpreter?.id})'`)
.join(', ')}`
);
} catch (ex) {
traceError('UniversalRemoteKernelFinder: Failed to write to cache', ex);
}
Expand Down
4 changes: 4 additions & 0 deletions src/kernels/raw/finder/contributedKerneFinder.node.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ import { ITrustedKernelPaths } from './types';
let kernelRankHelper: IKernelRankingHelper;
let cancelToken: CancellationTokenSource;
let onDidChangeInterpreters: EventEmitter<void>;
let onDidDeleteInterpreter: EventEmitter<{ id: string }>;
let onDidChangeInterpreter: EventEmitter<void>;
let onDidChangeInterpreterStatus: EventEmitter<void>;
let changeEventFired: TestEventHandler<void>;
Expand Down Expand Up @@ -125,9 +126,11 @@ import { ITrustedKernelPaths } from './types';
onDidChangeInterpreter = new EventEmitter<void>();
onDidChangeInterpreters = new EventEmitter<void>();
onDidChangeInterpreterStatus = new EventEmitter<void>();
onDidDeleteInterpreter = new EventEmitter<{ id: string }>();
disposables.push(onDidChangeInterpreter);
disposables.push(onDidChangeInterpreters);
disposables.push(onDidChangeInterpreterStatus);
disposables.push(onDidDeleteInterpreter);
// Ensure the active Interpreter is in the list of interpreters.
if (activeInterpreter) {
testData.interpreters = testData.interpreters || [];
Expand All @@ -145,6 +148,7 @@ import { ITrustedKernelPaths } from './types';
testData.interpreters = Array.from(distinctInterpreters);
when(interpreterService.onDidChangeInterpreter).thenReturn(onDidChangeInterpreter.event);
when(interpreterService.onDidChangeInterpreters).thenReturn(onDidChangeInterpreters.event);
when(interpreterService.onDidRemoveInterpreter).thenReturn(onDidDeleteInterpreter.event);
when(interpreterService.onDidChangeStatus).thenReturn(onDidChangeInterpreterStatus.event);
when(interpreterService.resolvedEnvironments).thenReturn(Array.from(distinctInterpreters));
when(interpreterService.getActiveInterpreter(anything())).thenResolve(activeInterpreter);
Expand Down
58 changes: 41 additions & 17 deletions src/kernels/raw/finder/contributedLocalKernelSpecFinder.node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
'use strict';

import { inject, injectable } from 'inversify';
import { Event, EventEmitter } from 'vscode';
import { EventEmitter } from 'vscode';
import { IKernelFinder, LocalKernelConnectionMetadata } from '../../types';
import { LocalPythonAndRelatedNonPythonKernelSpecFinder } from './localPythonAndRelatedNonPythonKernelSpecFinder.node';
import { LocalKnownPathKernelSpecFinder } from './localKnownPathKernelSpecFinder.node';
import { traceInfo, traceDecoratorError, traceError } from '../../../platform/logging';
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';
Expand Down Expand Up @@ -49,8 +49,12 @@ export class ContributedLocalKernelSpecFinder
id: string = ContributedKernelFinderKind.LocalKernelSpec;
displayName: string = DataScience.localKernelSpecs();

private _onDidChangeKernels = new EventEmitter<void>();
onDidChangeKernels: Event<void> = this._onDidChangeKernels.event;
private _onDidChangeKernels = new EventEmitter<{
added?: LocalKernelConnectionMetadata[];
updated?: LocalKernelConnectionMetadata[];
removed?: LocalKernelConnectionMetadata[];
}>();
onDidChangeKernels = this._onDidChangeKernels.event;

private wasPythonInstalledWhenFetchingControllers = false;

Expand Down Expand Up @@ -166,7 +170,6 @@ export class ContributedLocalKernelSpecFinder
traceError('Exception Saving loaded kernels', ex);
}
}

public get kernels(): LocalKernelConnectionMetadata[] {
const loadedKernelSpecFiles = new Set<string>();
const kernels: LocalKernelConnectionMetadata[] = [];
Expand Down Expand Up @@ -205,21 +208,42 @@ export class ContributedLocalKernelSpecFinder

private async writeToCache(values: LocalKernelConnectionMetadata[]) {
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 (oldValues.length === this.cache.length && areObjectsWithUrisTheSame(oldValues, this.cache)) {
return;
values = this.filterKernels(
values.filter((item) => {
if (uniqueIds.has(item.id)) {
return false;
}
uniqueIds.add(item.id);
return true;
})
);

const oldValues = this.cache;
const oldKernels = new Map(oldValues.map((item) => [item.id, item]));
const kernels = new Map(values.map((item) => [item.id, item]));
const added = values.filter((k) => !oldKernels.has(k.id));
const updated = values.filter(
(k) => oldKernels.has(k.id) && !areObjectsWithUrisTheSame(k, oldKernels.get(k.id))
);
const removed = oldValues.filter((k) => !kernels.has(k.id));

this.cache = values;
if (added.length || updated.length || removed.length) {
this._onDidChangeKernels.fire({ added, updated, removed });
}

this._onDidChangeKernels.fire();
traceVerbose(
`Updating cache with Local kernels ${values
.map((k) => `${k.kind}:'${k.id} (interpreter id = ${k.interpreter?.id})'`)
.join(', ')}\n, Added = ${added
.map((k) => `${k.kind}:'${k.id} (interpreter id = ${k.interpreter?.id})'`)
.join(', ')}\n, Updated = ${updated
.map((k) => `${k.kind}:'${k.id} (interpreter id = ${k.interpreter?.id})'`)
.join(', ')}\n, Removed = ${removed
.map((k) => `${k.kind}:'${k.id} (interpreter id = ${k.interpreter?.id})'`)
.join(', ')}`
);
} catch (ex) {
traceError('LocalKernelFinder: Failed to write to cache', ex);
}
Expand Down
43 changes: 32 additions & 11 deletions src/kernels/raw/finder/contributedLocalPythonEnvFinder.node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
'use strict';

import { inject, injectable } from 'inversify';
import { Event, EventEmitter } from 'vscode';
import { EventEmitter } from 'vscode';
import { IKernelFinder, PythonKernelConnectionMetadata } from '../../../kernels/types';
import { LocalPythonAndRelatedNonPythonKernelSpecFinder } from './localPythonAndRelatedNonPythonKernelSpecFinder.node';
import { traceDecoratorError, traceError, traceVerbose } from '../../../platform/logging';
Expand Down Expand Up @@ -46,8 +46,12 @@ export class ContributedLocalPythonEnvFinder
id: string = ContributedKernelFinderKind.LocalPythonEnvironment;
displayName: string = localize.DataScience.localPythonEnvironments();

private _onDidChangeKernels = new EventEmitter<void>();
onDidChangeKernels: Event<void> = this._onDidChangeKernels.event;
private _onDidChangeKernels = new EventEmitter<{
added?: PythonKernelConnectionMetadata[];
updated?: PythonKernelConnectionMetadata[];
removed?: PythonKernelConnectionMetadata[];
}>();
onDidChangeKernels = this._onDidChangeKernels.event;

private wasPythonInstalledWhenFetchingControllers = false;

Expand Down Expand Up @@ -150,27 +154,44 @@ export class ContributedLocalPythonEnvFinder
traceError('Exception Saving loaded kernels', ex);
}
}

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

this._onDidChangeKernels.fire();
const oldValues = this.cache;
const oldKernels = new Map(oldValues.map((item) => [item.id, item]));
const newKernelIds = new Set(values.map((item) => item.id));
const added = values.filter((k) => !oldKernels.has(k.id));
const updated = values.filter(
(k) => oldKernels.has(k.id) && !areObjectsWithUrisTheSame(k, oldKernels.get(k.id))
);
const removed = oldValues.filter((k) => !newKernelIds.has(k.id));

this.cache = values;
if (added.length || updated.length || removed.length) {
this._onDidChangeKernels.fire({ added, updated, removed });
}
traceVerbose(
`Updating cache with Python kernels ${values
.map((k) => `${k.kind}:'${k.id} (interpreter id = ${k.interpreter?.id})'`)
.join(', ')}\n, Added = ${added
.map((k) => `${k.kind}:'${k.id} (interpreter id = ${k.interpreter?.id})'`)
.join(', ')}\n, Updated = ${updated
.map((k) => `${k.kind}:'${k.id} (interpreter id = ${k.interpreter?.id})'`)
.join(', ')}\n, Removed = ${removed
.map((k) => `${k.kind}:'${k.id} (interpreter id = ${k.interpreter?.id})'`)
.join(', ')}`
);
} catch (ex) {
traceError('LocalKernelFinder: Failed to write to cache', ex);
}
Expand Down
40 changes: 26 additions & 14 deletions src/kernels/raw/finder/localKnownPathKernelSpecFinder.node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { LocalKernelSpecFinderBase } from './localKernelSpecFinderBase.node';
import { JupyterPaths } from './jupyterPaths.node';
import { IPythonExtensionChecker } from '../../../platform/api/types';
import { IApplicationEnvironment, IWorkspaceService } from '../../../platform/common/application/types';
import { traceError } from '../../../platform/logging';
import { traceError, traceVerbose } from '../../../platform/logging';
import { IFileSystemNode } from '../../../platform/common/platform/types.node';
import { IMemento, GLOBAL_MEMENTO, IDisposableRegistry } from '../../../platform/common/types';
import { capturePerfTelemetry, Telemetry } from '../../../telemetry';
Expand All @@ -31,7 +31,7 @@ export class LocalKnownPathKernelSpecFinder
extends LocalKernelSpecFinderBase<LocalKernelSpecConnectionMetadata>
implements IExtensionSyncActivationService
{
private _cachedKernels: LocalKernelSpecConnectionMetadata[] = [];
private readonly _cachedKernels = new Map<string, LocalKernelSpecConnectionMetadata>();
private readonly _onDidChangeKernels = new EventEmitter<void>();
/**
* TODO: We can monitor the known kernel spec folders and files for changes and trigger the change event.
Expand All @@ -54,8 +54,8 @@ export class LocalKnownPathKernelSpecFinder
const cancellation = new CancellationTokenSource();
this.listKernelsFirstTimeFromMemento(LocalKernelSpecsCacheKey)
.then((kernels) => {
if (this._cachedKernels.length === 0 && kernels.length) {
this._cachedKernels = kernels;
if (this._cachedKernels.size === 0 && kernels.length) {
kernels.forEach((k) => this._cachedKernels.set(k.id, k));
this._onDidChangeKernels.fire();
}
})
Expand All @@ -65,7 +65,7 @@ export class LocalKnownPathKernelSpecFinder
.finally(() => cancellation.dispose());
}
public get kernels(): LocalKernelSpecConnectionMetadata[] {
return this._cachedKernels;
return Array.from(this._cachedKernels.values());
}
public dispose(): void | undefined {
this._onDidChangeKernels.dispose();
Expand All @@ -91,7 +91,7 @@ export class LocalKnownPathKernelSpecFinder
// First find the on disk kernel specs and interpreters
const kernelSpecs = await this.findKernelSpecs(cancelToken);

const mappedKernelSpecs = kernelSpecs.map((k) =>
const newKernelSpecs = kernelSpecs.map((k) =>
LocalKernelSpecConnectionMetadata.create({
kernelSpec: k,
interpreter: undefined,
Expand All @@ -101,21 +101,33 @@ export class LocalKnownPathKernelSpecFinder
if (cancelToken.isCancellationRequested) {
return [];
}
const oldKernels = this._cachedKernels;
this._cachedKernels = mappedKernelSpecs;
const oldSortedKernels = Array.from(this._cachedKernels.values()).sort((a, b) => a.id.localeCompare(b.id));
const newSortedKernels = newKernelSpecs.sort((a, b) => a.id.localeCompare(b.id));
const newKernelIds = new Set(newKernelSpecs.map((k) => k.id));
const deletedKernels = oldSortedKernels.filter((k) => !newKernelIds.has(k.id));

// Add/update the kernels.
newKernelSpecs.forEach((k) => this._cachedKernels.set(k.id, k));

// Trigger a change event if we have different kernels.
oldKernels.sort();
mappedKernelSpecs.sort();
if (
oldKernels.length !== mappedKernelSpecs.length ||
JSON.stringify(oldKernels) !== JSON.stringify(mappedKernelSpecs)
oldSortedKernels.length !== newSortedKernels.length ||
deletedKernels.length ||
JSON.stringify(oldSortedKernels) !== JSON.stringify(newSortedKernels)
) {
this._onDidChangeKernels.fire();
this.writeToMementoCache(this._cachedKernels, LocalKernelSpecsCacheKey).ignoreErrors();
this.writeToMementoCache(
Array.from(this._cachedKernels.values()),
LocalKernelSpecsCacheKey
).ignoreErrors();
}
this._onDidChangeKernels.fire();
return mappedKernelSpecs;
if (deletedKernels.length) {
traceVerbose(
`Local kernel spec connection deleted ${deletedKernels.map((item) => `${item.kind}:'${item.id}'`)}`
);
}
return newKernelSpecs;
});
this.promiseMonitor.push(promise);
return promise;
Expand Down
Loading