Skip to content

Commit 1ae0dbd

Browse files
author
Kartik Raj
authored
Improve discovery API design (#17027)
* Improve discovery API design * Improve names * Remove unnecessary code
1 parent db1acaa commit 1ae0dbd

5 files changed

Lines changed: 43 additions & 44 deletions

File tree

‎src/client/pythonEnvironments/api.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ class PythonEnvironments implements IDiscoveryAPI {
2222
this.locator = await this.getLocator();
2323
}
2424

25-
public get onRefreshTrigger() {
26-
return this.locator.onRefreshTrigger;
25+
public get onRefreshStart() {
26+
return this.locator.onRefreshStart;
2727
}
2828

2929
public get refreshPromise() {
@@ -41,6 +41,10 @@ class PythonEnvironments implements IDiscoveryAPI {
4141
public async resolveEnv(env: string) {
4242
return this.locator.resolveEnv(env);
4343
}
44+
45+
public async triggerRefresh(query?: PythonLocatorQuery) {
46+
return this.locator.triggerRefresh(query);
47+
}
4448
}
4549

4650
export async function createPythonEnvironments(getLocator: GetLocatorFunc): Promise<IDiscoveryAPI> {

‎src/client/pythonEnvironments/base/locator.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,6 @@ export const NOOP_ITERATOR: IPythonEnvsIterator = iterEmpty<PythonEnvInfo>();
7979
* emitted by watchers.
8080
*/
8181
type BasicPythonLocatorQuery = {
82-
/**
83-
* If set as true, ignore the cache and wait until the fresh list of environments is retrieved.
84-
*/
85-
ignoreCache?: boolean;
8682
/**
8783
* If provided, results should be limited to these env
8884
* kinds; if not provided, the kind of each environment
@@ -173,7 +169,7 @@ export interface IDiscoveryAPI {
173169
/**
174170
* Fires when the known list of environments starts refreshing, i.e when discovery starts or restarts.
175171
*/
176-
readonly onRefreshTrigger: Event<void>;
172+
readonly onRefreshStart: Event<void>;
177173
/**
178174
* Fires with details if the known list changes.
179175
*/
@@ -183,10 +179,14 @@ export interface IDiscoveryAPI {
183179
* discovered.
184180
*/
185181
readonly refreshPromise: Promise<void>;
182+
/**
183+
* Triggers a new refresh for query if there isn't any already running.
184+
*/
185+
triggerRefresh(query?: PythonLocatorQuery): Promise<void>;
186186
/**
187187
* Get current list of known environments.
188188
*/
189-
getEnvs(query?: PythonLocatorQuery): Promise<PythonEnvInfo[]>;
189+
getEnvs(query?: PythonLocatorQuery): PythonEnvInfo[];
190190
/**
191191
* Find as much info about the given Python environment as possible.
192192
* If executable passed is invalid, then `undefined` is returned.

‎src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts‎

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ export class EnvsCollectionService extends PythonEnvsWatcher<PythonEnvCollection
2020
/** Keeps track of ongoing refreshes for various queries. */
2121
private refreshPromises = new Map<PythonLocatorQuery | undefined, Promise<void>>();
2222

23-
private readonly refreshTriggered = new EventEmitter<void>();
23+
private readonly refreshStarted = new EventEmitter<void>();
2424

25-
public get onRefreshTrigger(): Event<void> {
26-
return this.refreshTriggered.event;
25+
public get onRefreshStart(): Event<void> {
26+
return this.refreshStarted.event;
2727
}
2828

2929
public get refreshPromise(): Promise<void> {
@@ -33,7 +33,7 @@ export class EnvsCollectionService extends PythonEnvsWatcher<PythonEnvCollection
3333
constructor(private readonly cache: IEnvsCollectionCache, private readonly locator: IResolvingLocator) {
3434
super();
3535
this.locator.onChanged((event) =>
36-
this.ensureNewRefresh().then(() => {
36+
this.triggerNewRefresh().then(() => {
3737
// Once refresh of cache is complete, notify changes.
3838
this.fire({ type: event.type, searchLocation: event.searchLocation });
3939
}),
@@ -53,42 +53,33 @@ export class EnvsCollectionService extends PythonEnvsWatcher<PythonEnvCollection
5353
return this.locator.resolveEnv(executablePath);
5454
}
5555

56-
public async getEnvs(query?: PythonLocatorQuery): Promise<PythonEnvInfo[]> {
56+
public getEnvs(query?: PythonLocatorQuery): PythonEnvInfo[] {
5757
const cachedEnvs = this.cache.getAllEnvs();
58-
if (query?.ignoreCache) {
59-
await this.ensureCurrentRefresh(query);
60-
} else if (cachedEnvs.length === 0) {
61-
// Ignore query and trigger a refresh to get all envs as cache is empty.
62-
this.ensureCurrentRefresh(undefined).ignoreErrors();
63-
}
6458
return query ? cachedEnvs.filter(getQueryFilter(query)) : cachedEnvs;
6559
}
6660

67-
/**
68-
* Ensures we have a current alive refresh for the query going on.
69-
*/
70-
private async ensureCurrentRefresh(query?: PythonLocatorQuery): Promise<void> {
61+
public triggerRefresh(query?: PythonLocatorQuery): Promise<void> {
7162
let refreshPromiseForQuery = this.refreshPromises.get(query);
7263
if (!refreshPromiseForQuery) {
73-
refreshPromiseForQuery = this.triggerRefresh(query);
64+
refreshPromiseForQuery = this.startRefresh(query);
7465
}
7566
return refreshPromiseForQuery;
7667
}
7768

7869
/**
79-
* Ensure we initialize a fresh refresh after the current refresh (if any) is done.
70+
* Ensure we trigger a fresh refresh after the current refresh (if any) is done.
8071
*/
81-
private async ensureNewRefresh(query?: PythonLocatorQuery): Promise<void> {
72+
private async triggerNewRefresh(query?: PythonLocatorQuery): Promise<void> {
8273
const refreshPromise = this.refreshPromises.get(query);
8374
const nextRefreshPromise = refreshPromise
84-
? refreshPromise.then(() => this.triggerRefresh(query))
85-
: this.triggerRefresh(query);
75+
? refreshPromise.then(() => this.startRefresh(query))
76+
: this.startRefresh(query);
8677
return nextRefreshPromise;
8778
}
8879

89-
private async triggerRefresh(query: PythonLocatorQuery | undefined): Promise<void> {
80+
private async startRefresh(query: PythonLocatorQuery | undefined): Promise<void> {
9081
const stopWatch = new StopWatch();
91-
this.refreshTriggered.fire();
82+
this.refreshStarted.fire();
9283
const iterator = this.locator.iterEnvs(query);
9384
const refreshPromiseForQuery = this.addEnvsToCacheFromIterator(iterator);
9485
this.refreshPromises.set(query, refreshPromiseForQuery);

‎src/client/pythonEnvironments/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ export async function activate(api: IDiscoveryAPI): Promise<ActivationResult> {
6464

6565
addItemsToRunAfterActivation(() => {
6666
// Force an initial background refresh of the environments.
67-
api.getEnvs().ignoreErrors();
67+
api.triggerRefresh().ignoreErrors();
6868
});
6969

7070
return {

‎src/client/pythonEnvironments/legacyIOC.ts‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -250,15 +250,14 @@ class ComponentAdapter implements IComponentAdapter {
250250

251251
// Implements IInterpreterLocatorService
252252
public get hasInterpreters(): Promise<boolean> {
253-
return this.api.getEnvs().then(async (initialEnvs) => {
254-
// Check if the collection already has envs, return true in that case.
255-
if (initialEnvs.length > 0) {
256-
return true;
257-
}
258-
// We should already have initiated discovery. Wait for an env to be added
259-
// to the collection until the refresh has finished.
260-
await Promise.race([this.onAddedToCollection.promise, this.api.refreshPromise]);
261-
const envs = await this.api.getEnvs();
253+
const initialEnvs = this.api.getEnvs();
254+
if (initialEnvs.length > 0) {
255+
return Promise.resolve(true);
256+
}
257+
// We should already have initiated discovery. Wait for an env to be added
258+
// to the collection until the refresh has finished.
259+
return Promise.race([this.onAddedToCollection.promise, this.api.refreshPromise]).then(() => {
260+
const envs = this.api.getEnvs();
262261
return envs.length > 0;
263262
});
264263
}
@@ -287,7 +286,7 @@ class ComponentAdapter implements IComponentAdapter {
287286
options?: GetInterpreterOptions,
288287
source?: PythonEnvSource[],
289288
): Promise<PythonEnvironment[]> {
290-
const query: PythonLocatorQuery = { ignoreCache: options?.ignoreCache };
289+
const query: PythonLocatorQuery = {};
291290
if (resource !== undefined) {
292291
const wsFolder = vscode.workspace.getWorkspaceFolder(resource);
293292
if (wsFolder !== undefined) {
@@ -298,8 +297,11 @@ class ComponentAdapter implements IComponentAdapter {
298297
}
299298
}
300299

300+
if (options?.ignoreCache) {
301+
await this.api.triggerRefresh(query);
302+
}
301303
await this.api.refreshPromise;
302-
let envs = await this.api.getEnvs(query);
304+
let envs = this.api.getEnvs(query);
303305
if (source) {
304306
envs = envs.filter((env) => intersection(source, env.source).length > 0);
305307
}
@@ -319,10 +321,12 @@ class ComponentAdapter implements IComponentAdapter {
319321
searchLocations: {
320322
roots: [workspaceFolder.uri],
321323
},
322-
ignoreCache: options?.ignoreCache,
323324
};
325+
if (options?.ignoreCache) {
326+
await this.api.triggerRefresh(query);
327+
}
324328
await this.api.refreshPromise;
325-
const envs = await this.api.getEnvs(query);
329+
const envs = this.api.getEnvs(query);
326330
return envs.map(convertEnvInfo);
327331
}
328332
}

0 commit comments

Comments
 (0)