Skip to content
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
1 change: 1 addition & 0 deletions news/1 Enhancements/17030.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Do not query to get all interpreters where it's not needed in the extension code.
25 changes: 14 additions & 11 deletions src/client/application/diagnostics/checks/macPythonInterpreter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export class InvalidMacPythonInterpreterService extends BaseDiagnosticsService {
return [];
}

const hasInterpreters = await this.interpreterService.hasInterpreters;
const hasInterpreters = await this.interpreterService.hasInterpreters();
if (!hasInterpreters) {
return [];
}
Expand All @@ -104,17 +104,20 @@ export class InvalidMacPythonInterpreterService extends BaseDiagnosticsService {
return [];
}

const interpreters = await this.interpreterService.getInterpreters(resource);
for (const info of interpreters) {
if (!(await this.helper.isMacDefaultPythonPath(info.path))) {
return [
new InvalidMacPythonInterpreterDiagnostic(
DiagnosticCodes.MacInterpreterSelectedAndHaveOtherInterpretersDiagnostic,
resource,
),
];
}
if (
await this.interpreterService.hasInterpreters((e) =>
this.helper.isMacDefaultPythonPath(e.path).then((x) => !x),
)
) {
// If non-mac default interpreters exist.
return [
new InvalidMacPythonInterpreterDiagnostic(
DiagnosticCodes.MacInterpreterSelectedAndHaveOtherInterpretersDiagnostic,
resource,
),
];
}

return [
new InvalidMacPythonInterpreterDiagnostic(
DiagnosticCodes.MacInterpreterSelectedAndNoOtherInterpretersDiagnostic,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,7 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService {
}

const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
// hasInterpreters being false can mean one of 2 things:
// 1. getInterpreters hasn't returned any interpreters;
// 2. getInterpreters hasn't run yet.
// We want to make sure that false comes from 1, so we're adding this fix until we refactor interpreter discovery.
// Also see https://github.com/microsoft/vscode-python/issues/3023.
const hasInterpreters =
(await interpreterService.hasInterpreters) ||
(await interpreterService.getInterpreters(resource)).length > 0;
const hasInterpreters = await interpreterService.hasInterpreters();

if (!hasInterpreters) {
return [new InvalidPythonInterpreterDiagnostic(DiagnosticCodes.NoPythonInterpretersDiagnostic, resource)];
Expand Down
1 change: 1 addition & 0 deletions src/client/debugger/extension/adapter/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
return interpreter.path;
}

await this.interpreterService.hasInterpreters(); // Wait until we know whether we have an interpreter
const interpreters = await this.interpreterService.getInterpreters(resourceUri);
if (interpreters.length === 0) {
this.notifySelectInterpreter().ignoreErrors();
Expand Down
2 changes: 1 addition & 1 deletion src/client/interpreter/autoSelection/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio
private async autoselectInterpreterWithLocators(resource: Resource): Promise<void> {
// Do not perform a full interpreter search if we already have cached interpreters for this workspace.
const queriedState = this.getAutoSelectionInterpretersQueryState(resource);
const interpreters = await this.interpreterService.getInterpreters(resource, {
const interpreters = await this.interpreterService.getAllInterpreters(resource, {
ignoreCache: queriedState.value !== true,
});
const workspaceUri = this.interpreterHelper.getActiveWorkspaceUri(resource);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,10 @@ export class InterpreterSelector implements IInterpreterSelector {
}

public async getSuggestions(resource: Resource, ignoreCache?: boolean): Promise<IInterpreterQuickPickItem[]> {
const interpreters = await this.interpreterManager.getInterpreters(resource, {
const interpreters = await this.interpreterManager.getAllInterpreters(resource, {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be fixed to use getInterpreters() correctly in #17043.

onSuggestion: true,
ignoreCache,
});

interpreters.sort(this.envTypeComparer.compare.bind(this.envTypeComparer));

return Promise.all(interpreters.map((item) => this.suggestionToQuickPickItem(item, resource)));
Expand Down
26 changes: 19 additions & 7 deletions src/client/interpreter/contracts.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { SemVer } from 'semver';
import { CodeLensProvider, ConfigurationTarget, Disposable, Event, TextDocument, Uri } from 'vscode';
import { IExtensionSingleActivationService } from '../activation/types';
import { FileChangeType } from '../common/platform/fileSystemWatcher';
import { Resource } from '../common/types';
import { PythonEnvSource } from '../pythonEnvironments/base/info';
import { PythonLocatorQuery } from '../pythonEnvironments/base/locator';
import { CondaEnvironmentInfo, CondaInfo } from '../pythonEnvironments/common/environmentManagers/conda';
import { EnvironmentType, PythonEnvironment } from '../pythonEnvironments/info';

Expand Down Expand Up @@ -30,20 +32,26 @@ export interface IVirtualEnvironmentsSearchPathProvider {
getSearchPaths(resource?: Uri): Promise<string[]>;
}

export type PythonEnvironmentsChangedEvent = {
type?: FileChangeType;
resource?: Uri;
old?: PythonEnvironment;
update?: PythonEnvironment | undefined;
};

export const IComponentAdapter = Symbol('IComponentAdapter');
export interface IComponentAdapter {
triggerRefresh(query?: PythonLocatorQuery): Promise<void>;
readonly refreshPromise: Promise<void>;
readonly onChanged: Event<PythonEnvironmentsChangedEvent>;
// InterpreterLocatorProgressStatubarHandler
readonly onRefreshing: Event<void>;
readonly onRefreshed: Event<void>;
// VirtualEnvPrompt
onDidCreate(resource: Resource, callback: () => void): Disposable;
// IInterpreterLocatorService
hasInterpreters: Promise<boolean>;
getInterpreters(
resource?: Uri,
options?: GetInterpreterOptions,
source?: PythonEnvSource[],
): Promise<PythonEnvironment[]>;
hasInterpreters(filter?: (e: PythonEnvironment) => Promise<boolean>): Promise<boolean>;
getInterpreters(resource?: Uri, source?: PythonEnvSource[]): PythonEnvironment[];

// WorkspaceVirtualEnvInterpretersAutoSelectionRule
getWorkspaceVirtualEnvInterpreters(
Expand Down Expand Up @@ -104,11 +112,15 @@ export interface ICondaLocatorService {

export const IInterpreterService = Symbol('IInterpreterService');
export interface IInterpreterService {
triggerRefresh(query?: PythonLocatorQuery): Promise<void>;
readonly refreshPromise: Promise<void>;
readonly onDidChangeInterpreters: Event<PythonEnvironmentsChangedEvent>;
onDidChangeInterpreterConfiguration: Event<Uri | undefined>;
onDidChangeInterpreter: Event<void>;
onDidChangeInterpreterInformation: Event<PythonEnvironment>;
hasInterpreters: Promise<boolean>;
hasInterpreters(filter?: (e: PythonEnvironment) => Promise<boolean>): Promise<boolean>;
getInterpreters(resource?: Uri, options?: GetInterpreterOptions): Promise<PythonEnvironment[]>;
getAllInterpreters(resource?: Uri, options?: GetInterpreterOptions): Promise<PythonEnvironment[]>;
getActiveInterpreter(resource?: Uri): Promise<PythonEnvironment | undefined>;
getInterpreterDetails(pythonPath: string, resoure?: Uri): Promise<undefined | PythonEnvironment>;
refresh(resource: Resource): Promise<void>;
Expand Down
40 changes: 29 additions & 11 deletions src/client/interpreter/interpreterService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ import {
import { sleep } from '../common/utils/async';
import { IServiceContainer } from '../ioc/types';
import { EnvironmentType, PythonEnvironment } from '../pythonEnvironments/info';
import { sendTelemetryEvent } from '../telemetry';
import { EventName } from '../telemetry/constants';
import {
GetInterpreterOptions,
IComponentAdapter,
Expand All @@ -30,23 +28,26 @@ import {
IInterpreterLocatorService,
IInterpreterService,
INTERPRETER_LOCATOR_SERVICE,
PythonEnvironmentsChangedEvent,
} from './contracts';
import { IVirtualEnvironmentManager } from './virtualEnvs/types';
import { getInterpreterHash } from '../pythonEnvironments/discovery/locators/services/hashProvider';
import { inDiscoveryExperiment, inDiscoveryExperimentSync } from '../common/experiments/helpers';
import { StopWatch } from '../common/utils/stopWatch';
import { PythonVersion } from '../pythonEnvironments/info/pythonVersion';
import { PythonLocatorQuery } from '../pythonEnvironments/base/locator';

const EXPIRY_DURATION = 24 * 60 * 60 * 1000;

type StoredPythonEnvironment = PythonEnvironment & { store?: boolean };

@injectable()
export class InterpreterService implements Disposable, IInterpreterService {
public get hasInterpreters(): Promise<boolean> {
public async hasInterpreters(
filter: (e: PythonEnvironment) => Promise<boolean> = async () => true,
): Promise<boolean> {
return inDiscoveryExperiment(this.experimentService).then((inExp) => {
if (inExp) {
return this.pyenvs.hasInterpreters;
return this.pyenvs.hasInterpreters(filter);
}
const locator = this.serviceContainer.get<IInterpreterLocatorService>(
IInterpreterLocatorService,
Expand All @@ -56,10 +57,22 @@ export class InterpreterService implements Disposable, IInterpreterService {
});
}

public triggerRefresh(query?: PythonLocatorQuery): Promise<void> {
return inDiscoveryExperimentSync(this.experimentService)
? this.pyenvs.triggerRefresh(query)
: Promise.resolve();
}

public get refreshPromise(): Promise<void> {
return inDiscoveryExperimentSync(this.experimentService) ? this.pyenvs.refreshPromise : Promise.resolve();
}

public get onDidChangeInterpreter(): Event<void> {
return this.didChangeInterpreterEmitter.event;
}

public onDidChangeInterpreters: Event<PythonEnvironmentsChangedEvent>;

public get onDidChangeInterpreterInformation(): Event<PythonEnvironment> {
return this.didChangeInterpreterInformation.event;
}
Expand Down Expand Up @@ -97,6 +110,7 @@ export class InterpreterService implements Disposable, IInterpreterService {
this.configService = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
this.interpreterPathService = this.serviceContainer.get<IInterpreterPathService>(IInterpreterPathService);
this.experimentsManager = this.serviceContainer.get<IExperimentService>(IExperimentService);
this.onDidChangeInterpreters = pyenvs.onChanged;
}

public async refresh(resource?: Uri): Promise<void> {
Expand Down Expand Up @@ -138,9 +152,8 @@ export class InterpreterService implements Disposable, IInterpreterService {

public async getInterpreters(resource?: Uri, options?: GetInterpreterOptions): Promise<PythonEnvironment[]> {
let environments: PythonEnvironment[] = [];
const stopWatch = new StopWatch();
if (inDiscoveryExperimentSync(this.experimentService)) {
environments = await this.pyenvs.getInterpreters(resource, options);
environments = this.pyenvs.getInterpreters(resource);
} else {
const locator = this.serviceContainer.get<IInterpreterLocatorService>(
IInterpreterLocatorService,
Expand All @@ -149,10 +162,6 @@ export class InterpreterService implements Disposable, IInterpreterService {
environments = await locator.getInterpreters(resource, options);
}

sendTelemetryEvent(EventName.PYTHON_INTERPRETER_DISCOVERY, stopWatch.elapsedTime, {
interpreters: environments?.length ?? 0,
});

await Promise.all(
environments
.filter((item) => !item.displayName)
Expand All @@ -167,6 +176,14 @@ export class InterpreterService implements Disposable, IInterpreterService {
return environments;
}

public async getAllInterpreters(resource?: Uri, options?: GetInterpreterOptions): Promise<PythonEnvironment[]> {
if (options?.ignoreCache) {
this.triggerRefresh().ignoreErrors();
}
await this.refreshPromise;
return this.getInterpreters(resource, options);
}

public dispose(): void {
inDiscoveryExperiment(this.experimentService).then((inExp) => {
if (!inExp) {
Expand Down Expand Up @@ -243,6 +260,7 @@ export class InterpreterService implements Disposable, IInterpreterService {

// This is the preferred approach, hence the delay in option 1.
const option2 = (async () => {
await this.refreshPromise;
const interpreters = await this.getInterpreters(resource);
const found = interpreters.find((i) => fs.arePathsSame(i.path, pythonPath));
if (found) {
Expand Down
2 changes: 1 addition & 1 deletion src/client/jupyter/jupyterIntegration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export class JupyterExtensionIntegration {
getActiveInterpreter: async (resource?: Uri) => this.interpreterService.getActiveInterpreter(resource),
getInterpreterDetails: async (pythonPath: string) =>
this.interpreterService.getInterpreterDetails(pythonPath),
getInterpreters: async (resource: Uri | undefined) => this.interpreterService.getInterpreters(resource),
getInterpreters: async (resource: Uri | undefined) => this.interpreterService.getAllInterpreters(resource),
getActivatedEnvironmentVariables: async (
resource: Resource,
interpreter?: PythonEnvironment,
Expand Down
Loading