Skip to content

Commit 3545cb5

Browse files
committed
fix: suppress headless pip refresh prompts
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3fd1a810-6840-4ac9-ac33-c8a9fda4bfc4
1 parent d6f0a7a commit 3545cb5

5 files changed

Lines changed: 98 additions & 11 deletions

File tree

‎src/managers/builtin/pipPackageManager.ts‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ export class PipPackageManager implements PackageManager, Disposable {
9090
(changes) => {
9191
this._onDidChangePackages.fire({ environment, manager: this, changes });
9292
},
93+
() => this.fetchPackages(environment, !manageOptions.runHeadless),
9394
);
9495
} catch (e) {
9596
if (e instanceof CancellationError) {
@@ -132,18 +133,22 @@ export class PipPackageManager implements PackageManager, Disposable {
132133

133134
async getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise<Package[] | undefined> {
134135
if (options?.skipCache || !this.packages.has(environment.envId.id)) {
135-
const data = await refreshPipPackages(environment, this.log);
136-
if (data === undefined) {
137-
return this.packages.get(environment.envId.id);
138-
}
139-
140-
const packages = data.map((pkg) => this.api.createPackageItem(pkg, environment, this));
141-
this.packages.set(environment.envId.id, packages);
142-
return packages;
136+
return this.fetchPackages(environment);
143137
}
144138
return this.packages.get(environment.envId.id);
145139
}
146140

141+
private async fetchPackages(environment: PythonEnvironment, showErrors = true): Promise<Package[]> {
142+
const data = await refreshPipPackages(environment, this.log, { showErrors });
143+
if (data === undefined) {
144+
return this.packages.get(environment.envId.id) ?? [];
145+
}
146+
147+
const packages = data.map((pkg) => this.api.createPackageItem(pkg, environment, this));
148+
this.packages.set(environment.envId.id, packages);
149+
return packages;
150+
}
151+
147152
async getVersion(environment: PythonEnvironment): Promise<Pep440Version | undefined> {
148153
try {
149154
const useUv = await shouldUseUv(this.log, environment.environmentPath.fsPath);

‎src/managers/builtin/utils.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ async function execPipList(environment: PythonEnvironment, log?: LogOutputChanne
218218
export async function refreshPipPackages(
219219
environment: PythonEnvironment,
220220
log?: LogOutputChannel,
221-
options?: { showProgress: boolean },
221+
options?: { showProgress?: boolean; showErrors?: boolean },
222222
): Promise<PipPackage[] | undefined> {
223223
let data: string;
224224
try {
@@ -238,7 +238,9 @@ export async function refreshPipPackages(
238238
return parsePipListJson(data, log);
239239
} catch (e) {
240240
log?.error('Error refreshing packages', e);
241-
showErrorMessageWithLogs(SysManagerStrings.packageRefreshError, log);
241+
if (options?.showErrors !== false) {
242+
showErrorMessageWithLogs(SysManagerStrings.packageRefreshError, log);
243+
}
242244
return undefined;
243245
}
244246
}

‎src/managers/common/packageChanges.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import { normalizePackageName } from '../builtin/utils';
99
*/
1010
export type PackageChangesCallback = (changes: { kind: PackageChangeKind; pkg: Package }[]) => void;
1111

12+
type PackageFetcher = () => Promise<Package[] | undefined>;
13+
1214
/**
1315
* Computes the list of package changes between a before and after snapshot.
1416
* @param before - The previous list of packages.
@@ -41,15 +43,22 @@ export function getPackageChanges(before: Package[], after: Package[]): { kind:
4143
* This function calls {@link PackageManager.getPackages} with `skipCache` to fetch
4244
* the latest snapshot. The caller should pass the previously cached packages
4345
* so changes can be computed against the pre-refresh state.
46+
*
47+
* @param packageManager The package manager whose packages changed.
48+
* @param environment The environment whose packages should be refreshed.
49+
* @param before The package snapshot from before the operation.
50+
* @param onChanges Callback invoked when package changes are detected.
51+
* @param fetchPackages Optional internal fetcher for operation-specific refresh behavior.
4452
*/
4553
export async function updatePackagesAndNotify(
4654
packageManager: PackageManager,
4755
environment: PythonEnvironment,
4856
before: Package[] | undefined,
4957
onChanges: PackageChangesCallback,
58+
fetchPackages?: PackageFetcher,
5059
): Promise<Package[] | undefined> {
5160
const [after, afterDirectDependenciesNames] = await Promise.all([
52-
packageManager.getPackages(environment, { skipCache: true }).then((pkgs) => pkgs ?? []),
61+
(fetchPackages?.() ?? packageManager.getPackages(environment, { skipCache: true })).then((pkgs) => pkgs ?? []),
5362
// Handle transitive dependencies (best-effort, don't break package refresh on failure)
5463
packageManager.getDirectPackageNames?.(environment).catch(() => undefined),
5564
]);
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import * as assert from 'assert';
5+
import * as sinon from 'sinon';
6+
import { LogOutputChannel, Uri } from 'vscode';
7+
import { PythonEnvironment } from '../../../api';
8+
import * as errorUtils from '../../../common/errors/utils';
9+
import * as helpers from '../../../managers/builtin/helpers';
10+
import { refreshPipPackages } from '../../../managers/builtin/utils';
11+
12+
suite('Pip package refresh', () => {
13+
let environment: PythonEnvironment;
14+
let log: LogOutputChannel;
15+
let showErrorMessageWithLogsStub: sinon.SinonStub;
16+
17+
setup(() => {
18+
environment = {
19+
environmentPath: Uri.file('.'),
20+
execInfo: {
21+
run: {
22+
executable: 'python',
23+
},
24+
},
25+
} as PythonEnvironment;
26+
log = {
27+
error: sinon.stub(),
28+
info: sinon.stub(),
29+
} as unknown as LogOutputChannel;
30+
31+
sinon.stub(helpers, 'shouldUseUv').resolves(false);
32+
sinon.stub(helpers, 'runPython').rejects(new Error('pip list failed'));
33+
showErrorMessageWithLogsStub = sinon.stub(errorUtils, 'showErrorMessageWithLogs').resolves();
34+
});
35+
36+
teardown(() => {
37+
sinon.restore();
38+
});
39+
40+
test('shows an error when an interactive refresh fails', async () => {
41+
const result = await refreshPipPackages(environment, log);
42+
43+
assert.strictEqual(result, undefined);
44+
assert.ok(showErrorMessageWithLogsStub.calledOnce);
45+
});
46+
47+
test('does not show an error when a headless refresh fails', async () => {
48+
const result = await refreshPipPackages(environment, log, { showErrors: false });
49+
50+
assert.strictEqual(result, undefined);
51+
assert.ok(showErrorMessageWithLogsStub.notCalled);
52+
});
53+
});

‎src/test/managers/common/packageChanges.unit.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,24 @@ suite('packageChanges', () => {
127127
assert.strictEqual(changes[0].kind, PackageChangeKind.add);
128128
});
129129

130+
test('uses an operation-specific package fetcher when provided', async () => {
131+
const fetched = [{ name: 'requests', version: '2.31.0' } as Package];
132+
const fetchPackages = sinon.stub().resolves(fetched);
133+
const onChanges = sinon.stub();
134+
135+
const result = await updatePackagesAndNotify(
136+
packageManager,
137+
environment,
138+
undefined,
139+
onChanges,
140+
fetchPackages,
141+
);
142+
143+
assert.deepStrictEqual(result, fetched);
144+
assert.ok(fetchPackages.calledOnce);
145+
assert.ok(getPackagesStub.notCalled);
146+
});
147+
130148
test('does not fire callback when nothing changed', async () => {
131149
const pkgs = [{ name: 'requests', version: '2.31.0' } as Package];
132150
getPackagesStub.resolves(pkgs);

0 commit comments

Comments
 (0)