Skip to content

Commit

Permalink
Remove code related to old single kernel picker (#12145)
Browse files Browse the repository at this point in the history 
* Remove code related to old single kernel picker

* Fix compliation errors

* logging and fix tests
  • Loading branch information
@DonJayamanne
DonJayamanne authored Nov 23, 2022
1 parent 149d841 commit a95a92f
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 146 deletions.
2 changes: 1 addition & 1 deletion src/kernels/jupyter/finder/remoteKernelFinder.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ import { takeTopRankKernel } from '../../../notebooks/controllers/kernelRanking/
const extensions = mock<IExtensions>();
const featureManager = mock<IFeaturesManager>();
when(featureManager.features).thenReturn({ kernelPickerType });
kernelFinder = new KernelFinder(disposables, instance(featureManager));
kernelFinder = new KernelFinder(disposables);
kernelsChanged = createEventHandler(kernelFinder, 'onDidChangeKernels');
disposables.push(kernelsChanged);
kernelRankHelper = new KernelRankingHelper(preferredRemoteKernelIdProvider);
Expand Down
86 changes: 39 additions & 47 deletions src/kernels/kernelFinder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

import { inject, injectable } from 'inversify';
import { Event, EventEmitter } from 'vscode';
import { IDisposable, IDisposableRegistry, IFeaturesManager } from '../platform/common/types';
import { IDisposable, IDisposableRegistry } from '../platform/common/types';
import { traceInfoIfCI } from '../platform/logging';
import { ContributedKernelFinderKind, IContributedKernelFinder } from './internalTypes';
import { IKernelFinder, KernelConnectionMetadata } from './types';
Expand Down Expand Up @@ -40,10 +40,7 @@ export class KernelFinder implements IKernelFinder {
public get onDidChangeStatus(): Event<void> {
return this._onDidChangeStatus.event;
}
constructor(
@inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry,
@inject(IFeaturesManager) private readonly featureManager: IFeaturesManager
) {
constructor(@inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry) {
disposables.push(this._onDidChangeStatus);
disposables.push(this._onDidChangeRegistrations);
}
Expand Down Expand Up @@ -79,51 +76,46 @@ export class KernelFinder implements IKernelFinder {

// List kernels might be called after finders or connections are removed so just clear out and regenerate
this.connectionFinderMapping.clear();
let finders: IContributedKernelFinder<KernelConnectionMetadata>[] = [];
if (this.featureManager.features.kernelPickerType === 'Insiders') {
const loadedKernelSpecFiles = new Set<string>();
// If we have a global kernel spec returned by Python kernel finder,
// give that preference over the same kernel found using local kernel spec finder.
// This is because the python kernel finder would have more information about the kernel (such as the matching python env).
this._finders
.filter((finder) => finder.kind === ContributedKernelFinderKind.LocalPythonEnvironment)
.forEach((finder) => {
// Add our connection => finder mapping
finder.kernels.forEach((connection) => {
if (
(connection.kind === 'startUsingLocalKernelSpec' ||
connection.kind === 'startUsingPythonInterpreter') &&
connection.kernelSpec.specFile
) {
loadedKernelSpecFiles.add(connection.kernelSpec.specFile);
}
kernels.push(connection);
this.connectionFinderMapping.set(connection.id, finder);
});
const loadedKernelSpecFiles = new Set<string>();
// If we have a global kernel spec returned by Python kernel finder,
// give that preference over the same kernel found using local kernel spec finder.
// This is because the python kernel finder would have more information about the kernel (such as the matching python env).
this._finders
.filter((finder) => finder.kind === ContributedKernelFinderKind.LocalPythonEnvironment)
.forEach((finder) => {
// Add our connection => finder mapping
finder.kernels.forEach((connection) => {
if (
(connection.kind === 'startUsingLocalKernelSpec' ||
connection.kind === 'startUsingPythonInterpreter') &&
connection.kernelSpec.specFile
) {
loadedKernelSpecFiles.add(connection.kernelSpec.specFile);
}
kernels.push(connection);
this.connectionFinderMapping.set(connection.id, finder);
});
this._finders
.filter((finder) => finder.kind === ContributedKernelFinderKind.LocalKernelSpec)
.forEach((finder) => {
// Add our connection => finder mapping
finder.kernels.forEach((connection) => {
if (
(connection.kind === 'startUsingLocalKernelSpec' ||
connection.kind === 'startUsingPythonInterpreter') &&
connection.kernelSpec.specFile &&
loadedKernelSpecFiles.has(connection.kernelSpec.specFile)
) {
return;
}
kernels.push(connection);
this.connectionFinderMapping.set(connection.id, finder);
});
});
this._finders
.filter((finder) => finder.kind === ContributedKernelFinderKind.LocalKernelSpec)
.forEach((finder) => {
// Add our connection => finder mapping
finder.kernels.forEach((connection) => {
if (
(connection.kind === 'startUsingLocalKernelSpec' ||
connection.kind === 'startUsingPythonInterpreter') &&
connection.kernelSpec.specFile &&
loadedKernelSpecFiles.has(connection.kernelSpec.specFile)
) {
return;
}
kernels.push(connection);
this.connectionFinderMapping.set(connection.id, finder);
});
});

finders = this._finders.filter((finder) => finder.kind === ContributedKernelFinderKind.Remote);
} else {
finders = this._finders;
}
for (const finder of finders) {
const remoteFinders = this._finders.filter((finder) => finder.kind === ContributedKernelFinderKind.Remote);
for (const finder of remoteFinders) {
const contributedKernels = finder.kernels;

// Add our connection => finder mapping
Expand Down
5 changes: 2 additions & 3 deletions src/kernels/raw/finder/contributedKerneFinder.node.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ import { ITrustedKernelPaths } from './types';
when(trustedKernels.isTrusted(anything())).thenReturn(true);
const featuresManager = mock<IFeaturesManager>();
when(featuresManager.features).thenReturn({ kernelPickerType });
kernelFinder = new KernelFinder(disposables, instance(featuresManager));
kernelFinder = new KernelFinder(disposables);
localPythonAndRelatedKernelFinder = new LocalPythonAndRelatedNonPythonKernelSpecFinder(
instance(interpreterService),
instance(fs),
Expand All @@ -295,8 +295,7 @@ import { ITrustedKernelPaths } from './types';
[],
instance(extensionChecker),
instance(interpreterService),
instance(extensions),
instance(featuresManager)
instance(extensions)
);
const pythonEnvKernelFinder = new ContributedLocalPythonEnvFinder(
localPythonAndRelatedKernelFinder,
Expand Down
48 changes: 20 additions & 28 deletions src/kernels/raw/finder/contributedLocalKernelSpecFinder.node.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { IKernelFinder, LocalKernelConnectionMetadata } from '../../types';
import { LocalPythonAndRelatedNonPythonKernelSpecFinder } from './localPythonAndRelatedNonPythonKernelSpecFinder.node';
import { LocalKnownPathKernelSpecFinder } from './localKnownPathKernelSpecFinder.node';
import { traceInfo, traceDecoratorError, traceError } from '../../../platform/logging';
import { IDisposableRegistry, IExtensions, IFeaturesManager } from '../../../platform/common/types';
import { IDisposableRegistry, IExtensions } from '../../../platform/common/types';
import { capturePerfTelemetry, Telemetry } from '../../../telemetry';
import { areObjectsWithUrisTheSame, noop } from '../../../platform/common/utils/misc';
import { KernelFinder } from '../../kernelFinder';
Expand Down Expand Up @@ -64,8 +64,7 @@ export class ContributedLocalKernelSpecFinder
@inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry,
@inject(IPythonExtensionChecker) private readonly extensionChecker: IPythonExtensionChecker,
@inject(IInterpreterService) private readonly interpreters: IInterpreterService,
@inject(IExtensions) private readonly extensions: IExtensions,
@inject(IFeaturesManager) private readonly featureManager: IFeaturesManager
@inject(IExtensions) private readonly extensions: IExtensions
) {
kernelFinder.registerKernelFinder(this);
this.disposables.push(this._onDidChangeStatus);
Expand Down Expand Up @@ -169,32 +168,25 @@ export class ContributedLocalKernelSpecFinder
}

public get kernels(): LocalKernelConnectionMetadata[] {
if (this.featureManager.features.kernelPickerType === 'Insiders') {
const loadedKernelSpecFiles = new Set<string>();
const kernels: LocalKernelConnectionMetadata[] = [];
// If we have a global kernel spec returned by Python kernel finder,
// give that preference over the same kernel found using local kernel spec finder.
// This is because the python kernel finder would have more information about the kernel (such as the matching python env).
this.pythonKernelFinder.kernels.forEach((connection) => {
const kernelSpecKind = getKernelRegistrationInfo(connection.kernelSpec);
if (
connection.kernelSpec.specFile &&
kernelSpecKind === 'registeredByNewVersionOfExtForCustomKernelSpec'
) {
loadedKernelSpecFiles.add(connection.kernelSpec.specFile);
kernels.push(connection);
}
});
this.cache.forEach((connection) => {
if (connection.kernelSpec.specFile && loadedKernelSpecFiles.has(connection.kernelSpec.specFile)) {
return;
}
const loadedKernelSpecFiles = new Set<string>();
const kernels: LocalKernelConnectionMetadata[] = [];
// If we have a global kernel spec returned by Python kernel finder,
// give that preference over the same kernel found using local kernel spec finder.
// This is because the python kernel finder would have more information about the kernel (such as the matching python env).
this.pythonKernelFinder.kernels.forEach((connection) => {
const kernelSpecKind = getKernelRegistrationInfo(connection.kernelSpec);
if (connection.kernelSpec.specFile && kernelSpecKind === 'registeredByNewVersionOfExtForCustomKernelSpec') {
loadedKernelSpecFiles.add(connection.kernelSpec.specFile);
kernels.push(connection);
});
return kernels;
} else {
return this.cache;
}
}
});
this.cache.forEach((connection) => {
if (connection.kernelSpec.specFile && loadedKernelSpecFiles.has(connection.kernelSpec.specFile)) {
return;
}
kernels.push(connection);
});
return kernels;
}
private filterKernels(kernels: LocalKernelConnectionMetadata[]) {
return kernels.filter(({ kernelSpec }) => {
Expand Down
3 changes: 1 addition & 2 deletions src/kernels/raw/finder/contributedLocalKernelSpecFinder.node.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ import { LocalPythonAndRelatedNonPythonKernelSpecFinder } from './localPythonAnd
disposables,
instance(extensionChecker),
instance(interpreterService),
instance(extensions),
instance(featureManager)
instance(extensions)
);

clock = fakeTimers.install();
Expand Down
7 changes: 0 additions & 7 deletions src/kernels/raw/finder/localPythonAndRelatedNonPythonKernelSpecFinder.node.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,13 +376,6 @@ export class LocalPythonAndRelatedNonPythonKernelSpecFinder
this,
this.disposables
);
// if (this.featuresManager.features.kernelPickerType === 'Stable') {
this.interpreterService.onDidChangeInterpreter(
() => this.refreshData().catch(noop),
this,
this.disposables
);
// }
});
}
public get kernels(): LocalKernelConnectionMetadata[] {
Expand Down
16 changes: 1 addition & 15 deletions src/notebooks/controllers/controllerLoader.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { inject, injectable } from 'inversify';
import * as vscode from 'vscode';
import { isPythonNotebook } from '../../kernels/helpers';
import { IJupyterServerUriStorage } from '../../kernels/jupyter/types';
import { IKernelFinder, isLocalConnection, isRemoteConnection, KernelConnectionMetadata } from '../../kernels/types';
import { IKernelFinder, KernelConnectionMetadata } from '../../kernels/types';
import { IExtensionSyncActivationService } from '../../platform/activation/types';
import { IPythonExtensionChecker } from '../../platform/api/types';
import { IVSCodeNotebook } from '../../platform/common/application/types';
Expand Down Expand Up @@ -128,20 +128,6 @@ export class ControllerLoader implements IControllerLoader, IExtensionSyncActiva
return connection.id === controller.connection.id;
});

if (this.featuresManager.features.kernelPickerType !== 'Insiders') {
if (this.serverUriStorage.isLocalLaunch && isRemoteConnection(controller.connection)) {
traceVerbose(
`Remote Controller ${controller.connection.kind}:'${controller.id}' for view = '${controller.viewType}' is no longer a valid as we are not connected to a remote server.`
);
return true;
}
if (!this.serverUriStorage.isLocalLaunch && isLocalConnection(controller.connection)) {
traceVerbose(
`Local Controller ${controller.connection.kind}:'${controller.id}' for view = '${controller.viewType}' is no longer a valid as we are connected to a remote server.`
);
return true;
}
}
// Never remove remote kernels that don't exist.
// Always leave them there for user to select, and if the connection is not available/not valid,
// then notify the user and remove them.
Expand Down
5 changes: 2 additions & 3 deletions src/notebooks/controllers/kernelRanking/kernelRankingHelper.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ import { ITrustedKernelPaths } from '../../../kernels/raw/finder/types';
when(trustedKernels.isTrusted(anything())).thenReturn(true);
const featuresManager = mock<IFeaturesManager>();
when(featuresManager.features).thenReturn({ kernelPickerType });
kernelFinder = new KernelFinder([], instance(featuresManager));
kernelFinder = new KernelFinder([]);

localPythonAndRelatedKernelFinder = new LocalPythonAndRelatedNonPythonKernelSpecFinder(
instance(interpreterService),
Expand All @@ -272,8 +272,7 @@ import { ITrustedKernelPaths } from '../../../kernels/raw/finder/types';
[],
instance(extensionChecker),
instance(interpreterService),
instance(extensions),
instance(featuresManager)
instance(extensions)
);
localKernelFinder.activate();
nonPythonKernelSpecFinder.activate();
Expand Down
17 changes: 11 additions & 6 deletions src/notebooks/controllers/pythonEnvKernelConnectionCreator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { getDisplayPath } from '../../platform/common/platform/fs-paths';
import { IDisposable } from '../../platform/common/types';
import { IInterpreterService } from '../../platform/interpreter/contracts';
import { ServiceContainer } from '../../platform/ioc/container';
import { traceWarning } from '../../platform/logging';
import { traceVerbose, traceWarning } from '../../platform/logging';
import { PythonEnvironment } from '../../platform/pythonEnvironments/info';

type CreateEnvironmentResult = {
Expand All @@ -37,22 +37,27 @@ export class PythonEnvKernelConnectionCreator {
let env: PythonEnvironment | undefined;

env = await this.createPythonEnvironment(cancelToken);
if (!env || cancelToken.isCancellationRequested) {
if (cancelToken.isCancellationRequested || !env) {
return;
}
traceVerbose(`Python Environment created ${env.id}`);

const kernelConnection = await this.waitForPythonKernel(env, cancelToken);
if (!kernelConnection || cancelToken.isCancellationRequested) {
if (cancelToken.isCancellationRequested) {
return;
}

if (!kernelConnection) {
traceVerbose(`Python Environment ${env.id} not found as a kernel`);
return;
}
traceVerbose(`Python Environment ${env.id} found as a kernel ${kernelConnection.kind}:${kernelConnection.id}`);
const dependencyService = ServiceContainer.instance.get<IKernelDependencyService>(IKernelDependencyService);
const result = await dependencyService.installMissingDependencies({
resource: notebook.uri,
kernelConnection,
ui: new DisplayOptions(false),
token: cancelToken,
ignoreCache: false,
ignoreCache: true,
cannotChangeKernels: true,
installWithoutPrompting: true
});
Expand Down Expand Up @@ -133,7 +138,7 @@ export class PythonEnvKernelConnectionCreator {
);
return;
}

traceVerbose(`Python Environment created ${path}`);
const interpreterService = ServiceContainer.instance.get<IInterpreterService>(IInterpreterService);
return interpreterService.getInterpreterDetails({ path }).then((env) => {
if (cancelToken.isCancellationRequested) {
Expand Down
Loading

0 comments on commit a95a92f

Please sign in to comment.