Skip to content

Commit 209edb6

Browse files
DonJayamanneDonJayamanne
authored
Refactor how we check whether interpreters are available (#3809)
For #3369
1 parent 4f19099 commit 209edb6

16 files changed

+92
-34
lines changed

‎src/client/application/diagnostics/checks/pythonInterpreter.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,9 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService {
5454
}
5555

5656
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
57-
const interpreters = await interpreterService.getInterpreters();
57+
const hasInterpreters = await interpreterService.hasInterpreters;
5858

59-
if (interpreters.length === 0) {
59+
if (!hasInterpreters) {
6060
return [new InvalidPythonInterpreterDiagnostic(DiagnosticCodes.NoPythonInterpretersDiagnostic)];
6161
}
6262

@@ -77,6 +77,7 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService {
7777
if (!currentInterpreter || currentInterpreter.type !== InterpreterType.Unknown) {
7878
return [];
7979
}
80+
const interpreters = await interpreterService.getInterpreters();
8081
if (interpreters.filter(i => !helper.isMacDefaultPythonPath(i.path)).length === 0) {
8182
return [new InvalidPythonInterpreterDiagnostic(DiagnosticCodes.MacInterpreterSelectedAndNoOtherInterpretersDiagnostic)];
8283
}

‎src/client/interpreter/contracts.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ export const IInterpreterLocatorService = Symbol('IInterpreterLocatorService');
2929

3030
export interface IInterpreterLocatorService extends Disposable {
3131
readonly onLocating: Event<Promise<PythonInterpreter[]>>;
32+
readonly hasInterpreters: Promise<boolean>;
3233
getInterpreters(resource?: Uri, ignoreCache?: boolean): Promise<PythonInterpreter[]>;
3334
}
3435

@@ -81,6 +82,7 @@ export type WorkspacePythonPath = {
8182
export const IInterpreterService = Symbol('IInterpreterService');
8283
export interface IInterpreterService {
8384
onDidChangeInterpreter: Event<void>;
85+
hasInterpreters: Promise<boolean>;
8486
getInterpreters(resource?: Uri): Promise<PythonInterpreter[]>;
8587
autoSetInterpreter(): Promise<void>;
8688
getActiveInterpreter(resource?: Uri): Promise<PythonInterpreter | undefined>;

‎src/client/interpreter/interpreterService.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ export class InterpreterService implements Disposable, IInterpreterService {
3636
this.fs = this.serviceContainer.get<IFileSystem>(IFileSystem);
3737
this.persistentStateFactory = this.serviceContainer.get<IPersistentStateFactory>(IPersistentStateFactory);
3838
}
39+
public get hasInterpreters(): Promise<boolean> {
40+
return this.locator.hasInterpreters;
41+
}
3942

4043
public async refresh(resource?: Uri) {
4144
const interpreterDisplay = this.serviceContainer.get<IInterpreterDisplay>(IInterpreterDisplay);

‎src/client/interpreter/locators/index.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { inject, injectable } from 'inversify';
22
import { Disposable, Event, EventEmitter, Uri } from 'vscode';
33
import { IPlatformService } from '../../common/platform/types';
44
import { IDisposableRegistry } from '../../common/types';
5+
import { createDeferred, Deferred } from '../../common/utils/async';
56
import { OSType } from '../../common/utils/platform';
67
import { IServiceContainer } from '../../ioc/types';
78
import {
@@ -28,9 +29,11 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
2829
private readonly disposables: Disposable[] = [];
2930
private readonly platform: IPlatformService;
3031
private readonly interpreterLocatorHelper: IInterpreterLocatorHelper;
32+
private readonly _hasInterpreters: Deferred<boolean>;
3133
constructor(
3234
@inject(IServiceContainer) private serviceContainer: IServiceContainer
3335
) {
36+
this._hasInterpreters = createDeferred<boolean>();
3437
serviceContainer.get<Disposable[]>(IDisposableRegistry).push(this);
3538
this.platform = serviceContainer.get<IPlatformService>(IPlatformService);
3639
this.interpreterLocatorHelper = serviceContainer.get<IInterpreterLocatorHelper>(IInterpreterLocatorHelper);
@@ -46,6 +49,9 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
4649
public get onLocating(): Event<Promise<PythonInterpreter[]>> {
4750
return new EventEmitter<Promise<PythonInterpreter[]>>().event;
4851
}
52+
public get hasInterpreters(): Promise<boolean> {
53+
return this._hasInterpreters.promise;
54+
}
4955

5056
/**
5157
* Release any held resources.
@@ -65,11 +71,19 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
6571
public async getInterpreters(resource?: Uri): Promise<PythonInterpreter[]> {
6672
const locators = this.getLocators();
6773
const promises = locators.map(async provider => provider.getInterpreters(resource));
74+
locators.forEach(locator => {
75+
locator.hasInterpreters.then(found => {
76+
if (found) {
77+
this._hasInterpreters.resolve(true);
78+
}
79+
}).ignoreErrors();
80+
});
6881
const listOfInterpreters = await Promise.all(promises);
6982

7083
const items = flatten(listOfInterpreters)
7184
.filter(item => !!item)
7285
.map(item => item!);
86+
this._hasInterpreters.resolve(items.length > 0);
7387
return this.interpreterLocatorHelper.mergeInterpreters(items);
7488
}
7589

‎src/client/interpreter/locators/services/KnownPathsService.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ export class KnownPathsService extends CacheableLocatorService {
6161
if (!details) {
6262
return;
6363
}
64+
this._hasInterpreters.resolve(true);
6465
return {
6566
...(details as PythonInterpreter),
6667
path: interpreter,

‎src/client/interpreter/locators/services/baseVirtualEnvService.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ export class BaseVirtualEnvService extends CacheableLocatorService {
7777
if (!details) {
7878
return;
7979
}
80+
this._hasInterpreters.resolve(true);
8081
return {
8182
...(details as PythonInterpreter),
8283
envName: virtualEnvName,

‎src/client/interpreter/locators/services/cacheableLocatorService.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,23 @@ import { IInterpreterLocatorService, IInterpreterWatcher, PythonInterpreter } fr
1616

1717
@injectable()
1818
export abstract class CacheableLocatorService implements IInterpreterLocatorService {
19+
protected readonly _hasInterpreters: Deferred<boolean>;
1920
private readonly promisesPerResource = new Map<string, Deferred<PythonInterpreter[]>>();
2021
private readonly handlersAddedToResource = new Set<string>();
2122
private readonly cacheKeyPrefix: string;
2223
private readonly locating = new EventEmitter<Promise<PythonInterpreter[]>>();
2324
constructor(@unmanaged() name: string,
2425
@unmanaged() protected readonly serviceContainer: IServiceContainer,
2526
@unmanaged() private cachePerWorkspace: boolean = false) {
27+
this._hasInterpreters = createDeferred<boolean>();
2628
this.cacheKeyPrefix = `INTERPRETERS_CACHE_v2_${name}`;
2729
}
2830
public get onLocating(): Event<Promise<PythonInterpreter[]>> {
2931
return this.locating.event;
3032
}
33+
public get hasInterpreters(): Promise<boolean> {
34+
return this._hasInterpreters.promise;
35+
}
3136
public abstract dispose();
3237
public async getInterpreters(resource?: Uri, ignoreCache?: boolean): Promise<PythonInterpreter[]> {
3338
const cacheKey = this.getCacheKey(resource);
@@ -49,6 +54,10 @@ export abstract class CacheableLocatorService implements IInterpreterLocatorServ
4954

5055
this.locating.fire(deferred.promise);
5156
}
57+
deferred.promise
58+
.then(items => this._hasInterpreters.resolve(items.length > 0))
59+
.catch(_ => this._hasInterpreters.resolve(false));
60+
5261
if (deferred.completed) {
5362
return deferred.promise;
5463
}

‎src/client/interpreter/locators/services/condaEnvFileService.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export class CondaEnvFileService extends CacheableLocatorService {
4141
*
4242
* This is used by CacheableLocatorService.getInterpreters().
4343
*/
44-
protected getInterpretersImplementation(resource?: Uri): Promise<PythonInterpreter[]> {
44+
protected getInterpretersImplementation(_resource?: Uri): Promise<PythonInterpreter[]> {
4545
return this.getSuggestionsFromConda();
4646
}
4747

@@ -103,6 +103,7 @@ export class CondaEnvFileService extends CacheableLocatorService {
103103
return;
104104
}
105105
const envName = details.envName ? details.envName : path.basename(environmentPath);
106+
this._hasInterpreters.resolve(true);
106107
return {
107108
...(details as PythonInterpreter),
108109
path: interpreter,

‎src/client/interpreter/locators/services/condaEnvService.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ export class CondaEnvService extends CacheableLocatorService {
5858
this.fileSystem,
5959
this.helper
6060
);
61+
this._hasInterpreters.resolve(interpreters.length > 0);
6162
const environments = await this.condaService.getCondaEnvironments(true);
6263
if (Array.isArray(environments) && environments.length > 0) {
6364
interpreters

‎src/client/interpreter/locators/services/currentPathService.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ export class CurrentPathService extends CacheableLocatorService {
7272
if (!details) {
7373
return;
7474
}
75+
this._hasInterpreters.resolve(true);
7576
return {
7677
...(details as PythonInterpreter),
7778
path: pythonPath,

‎src/client/interpreter/locators/services/pipEnvService.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { inject, injectable } from 'inversify';
55
import * as path from 'path';
66
import { Uri } from 'vscode';
77
import { IApplicationShell, IWorkspaceService } from '../../../common/application/types';
8+
import { traceError } from '../../../common/logger';
89
import { IFileSystem, IPlatformService } from '../../../common/platform/types';
910
import { IProcessServiceFactory } from '../../../common/process/types';
1011
import { ICurrentProcess, ILogger } from '../../../common/types';
@@ -62,6 +63,7 @@ export class PipEnvService extends CacheableLocatorService implements IPipEnvSer
6263
if (!details) {
6364
return;
6465
}
66+
this._hasInterpreters.resolve(true);
6567
return {
6668
...(details as PythonInterpreter),
6769
path: interpreterPath,
@@ -86,11 +88,10 @@ export class PipEnvService extends CacheableLocatorService implements IPipEnvSer
8688
}
8789
try {
8890
const pythonPath = await this.invokePipenv('--py', cwd);
89-
// TODO: Why do we need to do this?
90-
return pythonPath && await this.fs.fileExists(pythonPath) ? pythonPath : undefined;
91+
return (pythonPath && await this.fs.fileExists(pythonPath)) ? pythonPath : undefined;
9192
// tslint:disable-next-line:no-empty
9293
} catch (error) {
93-
console.error(error);
94+
traceError('PipEnv identification failed', error);
9495
if (ignoreErrors) {
9596
return;
9697
}

‎src/client/interpreter/locators/services/windowsRegistryService.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ export class WindowsRegistryService extends CacheableLocatorService {
125125
return;
126126
}
127127
const version = interpreterInfo.version ? this.pathUtils.basename(interpreterInfo.version) : this.pathUtils.basename(tagKey);
128+
this._hasInterpreters.resolve(true);
128129
// tslint:disable-next-line:prefer-type-cast no-object-literal-type-assertion
129130
return {
130131
...(details as PythonInterpreter),

‎src/test/application/diagnostics/checks/pythonInterpreter.unit.test.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ suite('Application Diagnostics - Checks Python Interpreter', () => {
108108
.returns(() => false)
109109
.verifiable(typemoq.Times.once());
110110
interpreterService
111-
.setup(i => i.getInterpreters(typemoq.It.isAny()))
112-
.returns(() => Promise.resolve([]))
111+
.setup(i => i.hasInterpreters)
112+
.returns(() => Promise.resolve(false))
113113
.verifiable(typemoq.Times.once());
114114

115115
const diagnostics = await diagnosticService.diagnose();
@@ -122,10 +122,14 @@ suite('Application Diagnostics - Checks Python Interpreter', () => {
122122
.setup(s => s.disableInstallationChecks)
123123
.returns(() => false)
124124
.verifiable(typemoq.Times.once());
125+
interpreterService
126+
.setup(i => i.hasInterpreters)
127+
.returns(() => Promise.resolve(true))
128+
.verifiable(typemoq.Times.once());
125129
interpreterService
126130
.setup(i => i.getInterpreters(typemoq.It.isAny()))
127131
.returns(() => Promise.resolve([{} as any]))
128-
.verifiable(typemoq.Times.once());
132+
.verifiable(typemoq.Times.never());
129133
interpreterService
130134
.setup(i => i.getActiveInterpreter(typemoq.It.isAny()))
131135
.returns(() => { return Promise.resolve({ type: InterpreterType.Unknown } as any); })
@@ -146,10 +150,14 @@ suite('Application Diagnostics - Checks Python Interpreter', () => {
146150
.setup(s => s.disableInstallationChecks)
147151
.returns(() => false)
148152
.verifiable(typemoq.Times.once());
153+
interpreterService
154+
.setup(i => i.hasInterpreters)
155+
.returns(() => Promise.resolve(true))
156+
.verifiable(typemoq.Times.once());
149157
interpreterService
150158
.setup(i => i.getInterpreters(typemoq.It.isAny()))
151159
.returns(() => Promise.resolve([{} as any]))
152-
.verifiable(typemoq.Times.once());
160+
.verifiable(typemoq.Times.never());
153161
interpreterService
154162
.setup(i => i.getActiveInterpreter(typemoq.It.isAny()))
155163
.returns(() => { return Promise.resolve({ type: InterpreterType.Unknown } as any); })

0 commit comments

Comments
 (0)