Skip to content

Commit a329c5c

Browse files
rchiodoCopilot
andauthored
Return last-known environment when getEnvironment times out (microsoft#1607)
## Summary Avoid blocking callers of the legacy Python API when an environment can't be resolved quickly during startup/refresh. When `getEnvironment()` would otherwise wait on an in-progress refresh, return the last-known resolved environment immediately and let the fresh value update afterward. This is what lets the Python extension's `getActiveInterpreter()` return within ~100ms during a full environment refresh, so Pylance can start without waiting for enumeration to finish. ## Changes - **`features/pythonApi.ts`** – Track and return the last-known environment when `getEnvironment()` resolution is slow / times out, rather than blocking on the active refresh. - **`features/envManagers.ts`** / **`internal.api.ts`** – Plumb the last-known environment through the manager/internal API. ## Tests - `src/test/features/envManagers.lastKnown.unit.test.ts` covering the last-known fallback behavior. ## Notes Companion to the fast-Pylance-startup work in `microsoft/vscode-python` and the Pylance server. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d9fd3ec commit a329c5c

4 files changed

Lines changed: 165 additions & 2 deletions

File tree

src/features/envManagers.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,12 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
605605
}
606606
}
607607

608+
getLastKnownEnvironment(scope: GetEnvironmentScope): PythonEnvironment | undefined {
609+
const project = scope ? this.pm.get(scope) : undefined;
610+
const key = project ? project.uri.toString() : 'global';
611+
return this._activeSelection.get(key);
612+
}
613+
608614
getProjectEnvManagers(uris: Uri[]): InternalEnvironmentManager[] {
609615
const projectEnvManagers: InternalEnvironmentManager[] = [];
610616
uris.forEach((uri) => {

src/features/pythonApi.ts

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,20 @@ import {
4444
PythonPackageImpl,
4545
PythonProjectManager,
4646
} from '../internal.api';
47+
import { timeout } from '../common/utils/asyncUtils';
4748
import { waitForAllEnvManagers, waitForEnvManager, waitForEnvManagerId } from './common/managerReady';
4849
import { EnvVarManager } from './execution/envVariableManager';
4950
import { runAsTask } from './execution/runAsTask';
5051
import { runInBackground } from './execution/runInBackground';
5152
import { runInTerminal } from './terminal/runInTerminal';
5253
import { TerminalManager } from './terminal/terminalManager';
5354

55+
// Maximum time getEnvironment will block before serving the last-known environment while a
56+
// slow initial resolution/refresh continues in the background. Keeps consumers (e.g. Pylance's
57+
// workspace/configuration handler) from hanging on the full environment enumeration at startup.
58+
const GET_ENVIRONMENT_TIMEOUT_MS = 1000;
59+
const GET_ENVIRONMENT_TIMED_OUT = Symbol('getEnvironmentTimedOut');
60+
5461
class PythonEnvironmentApiImpl implements PythonEnvironmentApi {
5562
private readonly _onDidChangeEnvironments = new EventEmitter<DidChangeEnvironmentsEventArgs>();
5663
private readonly _onDidChangeEnvironment = new EventEmitter<DidChangeEnvironmentEventArgs>();
@@ -216,8 +223,29 @@ class PythonEnvironmentApiImpl implements PythonEnvironmentApi {
216223
}
217224
async getEnvironment(scope: GetEnvironmentScope): Promise<PythonEnvironment | undefined> {
218225
const currentScope = checkUri(scope) as GetEnvironmentScope;
219-
await waitForEnvManager(currentScope ? [currentScope] : undefined);
220-
return this.envManagers.getEnvironment(currentScope);
226+
227+
// Don't block callers (notably Pylance's workspace/configuration handler) on a potentially
228+
// slow initial environment resolution/refresh. Race the real resolution against a short
229+
// timeout; if it doesn't complete promptly, return the last-known environment for the scope.
230+
// The resolution continues in the background and consumers are notified of the real value
231+
// via onDidChangeEnvironment once it settles.
232+
const resolution = (async () => {
233+
await waitForEnvManager(currentScope ? [currentScope] : undefined);
234+
return this.envManagers.getEnvironment(currentScope);
235+
})();
236+
237+
const result = await Promise.race([
238+
resolution,
239+
timeout(GET_ENVIRONMENT_TIMEOUT_MS).then(() => GET_ENVIRONMENT_TIMED_OUT),
240+
]);
241+
if (result !== GET_ENVIRONMENT_TIMED_OUT) {
242+
return result as PythonEnvironment | undefined;
243+
}
244+
245+
// Keep the background resolution alive so the cache/last-known value gets populated and the
246+
// change event fires once it finishes.
247+
resolution.catch((ex) => traceError('Failed to resolve environment in background', ex));
248+
return this.envManagers.getLastKnownEnvironment(currentScope);
221249
}
222250
onDidChangeEnvironment: Event<DidChangeEnvironmentEventArgs> = this._onDidChangeEnvironment.event;
223251
async resolveEnvironment(context: ResolveEnvironmentContext): Promise<PythonEnvironment | undefined> {

src/internal.api.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,13 @@ export interface EnvironmentManagers extends Disposable {
140140
getEnvironment(scope: GetEnvironmentScope): Promise<PythonEnvironment | undefined>;
141141
refreshEnvironment(scope: GetEnvironmentScope): Promise<void>;
142142

143+
/**
144+
* Synchronously returns the last-known environment for a scope without triggering a refresh.
145+
* Used to serve a value promptly while a slow initial environment resolution runs in the
146+
* background. Returns undefined if no environment has been resolved for the scope yet.
147+
*/
148+
getLastKnownEnvironment(scope: GetEnvironmentScope): PythonEnvironment | undefined;
149+
143150
getProjectEnvManagers(uris: Uri[]): InternalEnvironmentManager[];
144151
}
145152

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
/**
5+
* Tests for PythonEnvironmentManagers.getLastKnownEnvironment, the synchronous accessor that
6+
* lets the public getEnvironment API serve a value promptly while a slow initial resolution
7+
* runs in the background (avoids blocking consumers such as Pylance's configuration handler).
8+
*/
9+
10+
import { Extension } from 'vscode';
11+
12+
import * as assert from 'assert';
13+
import * as sinon from 'sinon';
14+
import * as typeMoq from 'typemoq';
15+
import { EventEmitter, Uri } from 'vscode';
16+
import {
17+
DidChangeEnvironmentEventArgs,
18+
DidChangeEnvironmentsEventArgs,
19+
EnvironmentManager,
20+
GetEnvironmentScope,
21+
PythonEnvironment,
22+
PythonEnvironmentId,
23+
} from '../../api';
24+
import * as extensionApis from '../../common/extension.apis';
25+
import { PythonEnvironmentManagers } from '../../features/envManagers';
26+
import * as settingHelpers from '../../features/settings/settingHelpers';
27+
import { PythonProjectManager } from '../../internal.api';
28+
import { setupNonThenable } from '../mocks/helper';
29+
30+
suite('PythonEnvironmentManagers getLastKnownEnvironment', () => {
31+
let envManagers: PythonEnvironmentManagers;
32+
let projectManager: typeMoq.IMock<PythonProjectManager>;
33+
34+
function makeEnv(id: string): PythonEnvironment {
35+
const envId: PythonEnvironmentId = { id, managerId: 'test-manager' };
36+
return {
37+
envId,
38+
name: id,
39+
displayName: id,
40+
displayPath: `/path/${id}`,
41+
version: '3.11.0',
42+
environmentPath: Uri.file(`/path/${id}`),
43+
execInfo: { run: { executable: `/path/${id}/python`, args: [] } },
44+
sysPrefix: `/path/${id}`,
45+
} as PythonEnvironment;
46+
}
47+
48+
setup(() => {
49+
const mockPythonExtension = { id: 'ms-python.python', extensionPath: '/mock/python/extension' };
50+
const mockEnvsExtension = { id: 'ms-python.vscode-python-envs', extensionPath: '/mock/envs/extension' };
51+
52+
const getExtensionStub = sinon.stub(extensionApis, 'getExtension');
53+
getExtensionStub.withArgs('ms-python.python').returns(mockPythonExtension as Extension<unknown>);
54+
getExtensionStub.withArgs('ms-python.vscode-python-envs').returns(mockEnvsExtension as Extension<unknown>);
55+
sinon
56+
.stub(extensionApis, 'allExtensions')
57+
.returns([mockPythonExtension, mockEnvsExtension] as Extension<unknown>[]);
58+
59+
projectManager = typeMoq.Mock.ofType<PythonProjectManager>();
60+
setupNonThenable(projectManager);
61+
// No project for a scope -> refreshEnvironment/getLastKnownEnvironment use the 'global' key.
62+
projectManager.setup((pm) => pm.get(typeMoq.It.isAny())).returns(() => undefined);
63+
64+
envManagers = new PythonEnvironmentManagers(projectManager.object);
65+
});
66+
67+
teardown(() => {
68+
sinon.restore();
69+
envManagers.dispose();
70+
});
71+
72+
function registerManager(getImpl: (scope: GetEnvironmentScope) => Promise<PythonEnvironment | undefined>): string {
73+
const onDidChangeEnvironment = new EventEmitter<DidChangeEnvironmentEventArgs>();
74+
const onDidChangeEnvironments = new EventEmitter<DidChangeEnvironmentsEventArgs>();
75+
const manager = {
76+
name: 'test-env-mgr',
77+
displayName: 'Test Env Manager',
78+
preferredPackageManagerId: 'ms-python.python:pip',
79+
onDidChangeEnvironment: onDidChangeEnvironment.event,
80+
onDidChangeEnvironments: onDidChangeEnvironments.event,
81+
get: getImpl,
82+
getEnvironments: async () => [],
83+
set: async () => undefined,
84+
resolve: async () => undefined,
85+
refresh: async () => undefined,
86+
} as unknown as EnvironmentManager;
87+
88+
envManagers.registerEnvironmentManager(manager);
89+
const id = envManagers.managers[0].id;
90+
// Force the default environment manager (used for undefined/global scope) to resolve to ours.
91+
sinon.stub(settingHelpers, 'getDefaultEnvManagerSetting').returns(id);
92+
return id;
93+
}
94+
95+
test('returns undefined before any environment has been resolved', () => {
96+
registerManager(async () => makeEnv('env1'));
97+
assert.strictEqual(envManagers.getLastKnownEnvironment(undefined), undefined);
98+
});
99+
100+
test('returns the active environment after it has been resolved', async () => {
101+
const env = makeEnv('env1');
102+
registerManager(async () => env);
103+
104+
// Refreshing the active selection populates the last-known cache.
105+
await envManagers.refreshEnvironment(undefined);
106+
107+
// Now available synchronously without any await or refresh.
108+
assert.strictEqual(envManagers.getLastKnownEnvironment(undefined), env);
109+
});
110+
111+
test('reflects the most recent environment after it changes', async () => {
112+
let current = makeEnv('env1');
113+
registerManager(async () => current);
114+
115+
await envManagers.refreshEnvironment(undefined);
116+
assert.strictEqual(envManagers.getLastKnownEnvironment(undefined)?.envId.id, 'env1');
117+
118+
current = makeEnv('env2');
119+
await envManagers.refreshEnvironment(undefined);
120+
assert.strictEqual(envManagers.getLastKnownEnvironment(undefined)?.envId.id, 'env2');
121+
});
122+
});

0 commit comments

Comments
 (0)