Skip to content

Commit 28494bc

Browse files
edvilmeedvilme
authored
Refactor: Standardize PackageManager (#1538)
This pull request introduces a new `GetPackagesOptions` interface to support additional options (such as cache bypassing) when retrieving Python packages from a given environment. It updates the relevant APIs, interfaces, and implementations to accept this new options parameter, and refactors the pip package manager logic to support cache skipping and code simplification. The changes are applied consistently across the main codebase and sample/example files. **API and Interface Enhancements:** * Added a new `GetPackagesOptions` interface, allowing callers to specify options (currently `skipCache`) when retrieving packages. This is now accepted by `getPackages` methods in `PackageManager` and `PythonPackageGetterApi` interfaces. [[1]](diffhunk://#diff-76f83d8de3cb4a7f1a0369f39eee530dc18986c8cabbb2bfb40ccc4bf1baad8eR680-R683) [[2]](diffhunk://#diff-76f83d8de3cb4a7f1a0369f39eee530dc18986c8cabbb2bfb40ccc4bf1baad8eR798-R808) [[3]](diffhunk://#diff-76f83d8de3cb4a7f1a0369f39eee530dc18986c8cabbb2bfb40ccc4bf1baad8eR1040-R1043) [[4]](diffhunk://#diff-d5d7aa293bc97d0d8070d1d44893b3ba0c028df6025dd573772ed7bbf5bdd528R614-R617) [[5]](diffhunk://#diff-d5d7aa293bc97d0d8070d1d44893b3ba0c028df6025dd573772ed7bbf5bdd528R718-R728) [[6]](diffhunk://#diff-769911c416ccf8514d8fd941ae0abe8fb5c606ade0c218e22151a5f5f9f3d700R674-R677) [[7]](diffhunk://#diff-769911c416ccf8514d8fd941ae0abe8fb5c606ade0c218e22151a5f5f9f3d700R792-R802) [[8]](diffhunk://#diff-769911c416ccf8514d8fd941ae0abe8fb5c606ade0c218e22151a5f5f9f3d700R1034-R1037) * Updated all usages and implementations of `getPackages` to accept the new optional `options` parameter, including in `PythonEnvironmentApiImpl`, `InternalPackageManager`, and related example/sample APIs. [[1]](diffhunk://#diff-efc92694367c1c8ab2ba06c997294e0070c38a467ce161aec3deb26ad70e8f0cR13) [[2]](diffhunk://#diff-efc92694367c1c8ab2ba06c997294e0070c38a467ce161aec3deb26ad70e8f0cL260-R267) [[3]](diffhunk://#diff-0b6e4234e1d1be57c31de2504ec4720d77b624f100e70fcc324aff4f815aaab5R12) [[4]](diffhunk://#diff-0b6e4234e1d1be57c31de2504ec4720d77b624f100e70fcc324aff4f815aaab5L370-R372) **Pip Package Manager Refactoring and Improvements:** * Renamed `pipManager.ts` to `pipPackageManager.ts` and updated imports accordingly. * Refactored the pip package manager to support cache bypassing: if `options.skipCache` is set, it fetches a fresh package list instead of using the cache. Also simplified the logic for managing and refreshing packages, delegating change notification to a new shared utility. [[1]](diffhunk://#diff-bd7813d2ba80cb1d4b12c11a3b1e5832dbe0b65a1a68963ae95ca78e0a325291R14-L36) [[2]](diffhunk://#diff-bd7813d2ba80cb1d4b12c11a3b1e5832dbe0b65a1a68963ae95ca78e0a325291L88-R86) [[3]](diffhunk://#diff-bd7813d2ba80cb1d4b12c11a3b1e5832dbe0b65a1a68963ae95ca78e0a325291L117-R123) * Removed redundant code for change detection and replaced with a shared `updatePackagesAndNotify` utility. [[1]](diffhunk://#diff-bd7813d2ba80cb1d4b12c11a3b1e5832dbe0b65a1a68963ae95ca78e0a325291R14-L36) [[2]](diffhunk://#diff-bd7813d2ba80cb1d4b12c11a3b1e5832dbe0b65a1a68963ae95ca78e0a325291L88-R86) [[3]](diffhunk://#diff-bd7813d2ba80cb1d4b12c11a3b1e5832dbe0b65a1a68963ae95ca78e0a325291L117-R123) **Utilities and Supporting Changes:** * Refactored pip package refresh utilities: replaced the raw refresh logic with a more flexible function (`execPipList`) that supports additional arguments, and removed now-unnecessary helper functions. [[1]](diffhunk://#diff-06761af556051e7c3570fc796d9c69f15004c387e5095a909a9a5f9c270511d2L188-R193) [[2]](diffhunk://#diff-06761af556051e7c3570fc796d9c69f15004c387e5095a909a9a5f9c270511d2L231-R235) [[3]](diffhunk://#diff-06761af556051e7c3570fc796d9c69f15004c387e5095a909a9a5f9c270511d2L246-R251) [[4]](diffhunk://#diff-06761af556051e7c3570fc796d9c69f15004c387e5095a909a9a5f9c270511d2L313-L314) **Code Style and Minor Cleanups:** * Minor formatting and code style updates across example/sample API files for consistency. [[1]](diffhunk://#diff-d5d7aa293bc97d0d8070d1d44893b3ba0c028df6025dd573772ed7bbf5bdd528L918-R931) [[2]](diffhunk://#diff-d5d7aa293bc97d0d8070d1d44893b3ba0c028df6025dd573772ed7bbf5bdd528R965-R968) [[3]](diffhunk://#diff-d5d7aa293bc97d0d8070d1d44893b3ba0c028df6025dd573772ed7bbf5bdd528L987-R1002) [[4]](diffhunk://#diff-d5d7aa293bc97d0d8070d1d44893b3ba0c028df6025dd573772ed7bbf5bdd528L1206-R1221) [[5]](diffhunk://#diff-d5d7aa293bc97d0d8070d1d44893b3ba0c028df6025dd573772ed7bbf5bdd528L1258-R1271) [[6]](diffhunk://#diff-efc92694367c1c8ab2ba06c997294e0070c38a467ce161aec3deb26ad70e8f0cL99-R102) [[7]](diffhunk://#diff-450a068335039323c00b342cbfbff615ecf1cda7bd9d73aaed6b8e13d2d9d7f5L66-R66) These changes improve the flexibility and maintainability of the package management APIs and their implementations, preparing the codebase for future enhancements and better cache control.
1 parent bac9000 commit 28494bc

14 files changed

Lines changed: 403 additions & 197 deletions

File tree

‎api/src/main.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -677,9 +677,10 @@ export interface PackageManager {
677677
/**
678678
* Retrieves the list of packages for the specified Python environment.
679679
* @param environment - The Python environment for which to retrieve packages.
680+
* @param options - Optional settings for package retrieval.
680681
* @returns An array of packages, or undefined if the packages could not be retrieved.
681682
*/
682-
getPackages(environment: PythonEnvironment): Promise<Package[] | undefined>;
683+
getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise<Package[] | undefined>;
683684

684685
/**
685686
* Event that is fired when packages change.
@@ -794,6 +795,17 @@ export interface DidChangePythonProjectsEventArgs {
794795
removed: PythonProject[];
795796
}
796797

798+
/**
799+
* Options for retrieving packages from a package manager.
800+
*/
801+
export interface GetPackagesOptions {
802+
/**
803+
* When `true`, bypasses the cache and fetches the latest packages from the underlying tool.
804+
* Defaults to `false`.
805+
*/
806+
skipCache?: boolean;
807+
}
808+
797809
export type PackageManagementOptions =
798810
| {
799811
/**
@@ -1025,9 +1037,10 @@ export interface PythonPackageGetterApi {
10251037
* Get the list of packages in a Python Environment.
10261038
*
10271039
* @param environment The Python Environment for which the list of packages is required.
1040+
* @param options Optional settings for package retrieval.
10281041
* @returns The list of packages in the Python Environment.
10291042
*/
1030-
getPackages(environment: PythonEnvironment): Promise<Package[] | undefined>;
1043+
getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise<Package[] | undefined>;
10311044

10321045
/**
10331046
* Event raised when the list of packages in a Python Environment changes.

‎examples/sample1/src/api.ts‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -611,9 +611,10 @@ export interface PackageManager {
611611
/**
612612
* Retrieves the list of packages for the specified Python environment.
613613
* @param environment - The Python environment for which to retrieve packages.
614+
* @param options - Optional settings for package retrieval.
614615
* @returns An array of packages, or undefined if the packages could not be retrieved.
615616
*/
616-
getPackages(environment: PythonEnvironment): Promise<Package[] | undefined>;
617+
getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise<Package[] | undefined>;
617618

618619
/**
619620
* Event that is fired when packages change.
@@ -714,6 +715,17 @@ export interface DidChangePythonProjectsEventArgs {
714715
removed: PythonProject[];
715716
}
716717

718+
/**
719+
* Options for retrieving packages from a package manager.
720+
*/
721+
export interface GetPackagesOptions {
722+
/**
723+
* When `true`, bypasses the cache and fetches the latest packages from the underlying tool.
724+
* Defaults to `false`.
725+
*/
726+
skipCache?: boolean;
727+
}
728+
717729
/**
718730
* Options for package management.
719731
*/
@@ -915,7 +927,8 @@ export interface PythonProjectEnvironmentApi {
915927
}
916928

917929
export interface PythonEnvironmentManagerApi
918-
extends PythonEnvironmentManagerRegistrationApi,
930+
extends
931+
PythonEnvironmentManagerRegistrationApi,
919932
PythonEnvironmentItemApi,
920933
PythonEnvironmentManagementApi,
921934
PythonEnvironmentsApi,
@@ -949,9 +962,10 @@ export interface PythonPackageGetterApi {
949962
* Get the list of packages in a Python Environment.
950963
*
951964
* @param environment The Python Environment for which the list of packages is required.
965+
* @param options Optional settings for package retrieval.
952966
* @returns The list of packages in the Python Environment.
953967
*/
954-
getPackages(environment: PythonEnvironment): Promise<Package[] | undefined>;
968+
getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise<Package[] | undefined>;
955969

956970
/**
957971
* Event raised when the list of packages in a Python Environment changes.
@@ -984,7 +998,8 @@ export interface PythonPackageManagementApi {
984998
}
985999

9861000
export interface PythonPackageManagerApi
987-
extends PythonPackageManagerRegistrationApi,
1001+
extends
1002+
PythonPackageManagerRegistrationApi,
9881003
PythonPackageGetterApi,
9891004
PythonPackageManagementApi,
9901005
PythonPackageItemApi {}
@@ -1203,10 +1218,7 @@ export interface PythonBackgroundRunApi {
12031218
}
12041219

12051220
export interface PythonExecutionApi
1206-
extends PythonTerminalCreateApi,
1207-
PythonTerminalRunApi,
1208-
PythonTaskRunApi,
1209-
PythonBackgroundRunApi {}
1221+
extends PythonTerminalCreateApi, PythonTerminalRunApi, PythonTaskRunApi, PythonBackgroundRunApi {}
12101222

12111223
/**
12121224
* Event arguments for when the monitored `.env` files or any other sources change.
@@ -1255,7 +1267,8 @@ export interface PythonEnvironmentVariablesApi {
12551267
* The API for interacting with Python environments, package managers, and projects.
12561268
*/
12571269
export interface PythonEnvironmentApi
1258-
extends PythonEnvironmentManagerApi,
1270+
extends
1271+
PythonEnvironmentManagerApi,
12591272
PythonPackageManagerApi,
12601273
PythonProjectApi,
12611274
PythonExecutionApi,

‎src/api.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -671,9 +671,10 @@ export interface PackageManager {
671671
/**
672672
* Retrieves the list of packages for the specified Python environment.
673673
* @param environment - The Python environment for which to retrieve packages.
674+
* @param options - Optional settings for package retrieval.
674675
* @returns An array of packages, or undefined if the packages could not be retrieved.
675676
*/
676-
getPackages(environment: PythonEnvironment): Promise<Package[] | undefined>;
677+
getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise<Package[] | undefined>;
677678

678679
/**
679680
* Event that is fired when packages change.
@@ -788,6 +789,17 @@ export interface DidChangePythonProjectsEventArgs {
788789
removed: PythonProject[];
789790
}
790791

792+
/**
793+
* Options for retrieving packages from a package manager.
794+
*/
795+
export interface GetPackagesOptions {
796+
/**
797+
* When `true`, bypasses the cache and fetches the latest packages from the underlying tool.
798+
* Defaults to `false`.
799+
*/
800+
skipCache?: boolean;
801+
}
802+
791803
export type PackageManagementOptions =
792804
| {
793805
/**
@@ -1019,9 +1031,10 @@ export interface PythonPackageGetterApi {
10191031
* Get the list of packages in a Python Environment.
10201032
*
10211033
* @param environment The Python Environment for which the list of packages is required.
1034+
* @param options Optional settings for package retrieval.
10221035
* @returns The list of packages in the Python Environment.
10231036
*/
1024-
getPackages(environment: PythonEnvironment): Promise<Package[] | undefined>;
1037+
getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise<Package[] | undefined>;
10251038

10261039
/**
10271040
* Event raised when the list of packages in a Python Environment changes.

‎src/features/pythonApi.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
EnvironmentManager,
1111
GetEnvironmentScope,
1212
GetEnvironmentsScope,
13+
GetPackagesOptions,
1314
Package,
1415
PackageId,
1516
PackageInfo,
@@ -96,9 +97,9 @@ class PythonEnvironmentApiImpl implements PythonEnvironmentApi {
9697
// *selected* manager's changes propagate (refreshEnvironment checks
9798
// getEnvironmentManager(scope) internally). It updates the cache and
9899
// fires onDidChangeActiveEnvironment, which the Python API listens to.
99-
this.envManagers.refreshEnvironment(e.uri).catch((err) =>
100-
traceError('Failed to refresh environment on change:', err),
101-
);
100+
this.envManagers
101+
.refreshEnvironment(e.uri)
102+
.catch((err) => traceError('Failed to refresh environment on change:', err));
102103
});
103104
}),
104105
);
@@ -257,13 +258,13 @@ class PythonEnvironmentApiImpl implements PythonEnvironmentApi {
257258
}
258259
return manager.refresh(context);
259260
}
260-
async getPackages(context: PythonEnvironment): Promise<Package[] | undefined> {
261+
async getPackages(context: PythonEnvironment, options?: GetPackagesOptions): Promise<Package[] | undefined> {
261262
await waitForEnvManagerId([context.envId.managerId]);
262263
const manager = this.envManagers.getPackageManager(context);
263264
if (!manager) {
264265
return Promise.resolve(undefined);
265266
}
266-
return manager.getPackages(context);
267+
return manager.getPackages(context, options);
267268
}
268269
onDidChangePackages: Event<DidChangePackagesEventArgs> = this._onDidChangePackages.event;
269270
createPackageItem(info: PackageInfo, environment: PythonEnvironment, manager: PackageManager): Package {

‎src/internal.api.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
EnvironmentManager,
1010
GetEnvironmentScope,
1111
GetEnvironmentsScope,
12+
GetPackagesOptions,
1213
IconPath,
1314
Package,
1415
PackageChangeKind,
@@ -367,8 +368,8 @@ export class InternalPackageManager implements PackageManager {
367368
return this.manager.refresh(environment);
368369
}
369370

370-
getPackages(environment: PythonEnvironment): Promise<Package[] | undefined> {
371-
return this.manager.getPackages(environment);
371+
getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise<Package[] | undefined> {
372+
return this.manager.getPackages(environment, options);
372373
}
373374

374375
onDidChangePackages(handler: (e: DidChangePackagesEventArgs) => void): Disposable {

‎src/managers/builtin/main.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { createSimpleDebounce } from '../../common/utils/debounce';
44
import { createFileSystemWatcher, onDidDeleteFiles } from '../../common/workspace.apis';
55
import { getPythonApi } from '../../features/pythonApi';
66
import { NativePythonFinder } from '../common/nativePythonFinder';
7-
import { PipPackageManager } from './pipManager';
7+
import { PipPackageManager } from './pipPackageManager';
88
import { SysPythonManager } from './sysPythonManager';
99
import { VenvManager } from './venvManager';
1010

@@ -60,10 +60,10 @@ export async function registerSystemPythonFeatures(
6060
);
6161
});
6262
const packageWatcher = createFileSystemWatcher(
63-
'**/site-packages/*.dist-info/METADATA',
63+
'**/site-packages/*.dist-info/METADATA',
6464
false, // don't ignore create events (pip install)
65-
true, // ignore change events (content changes in METADATA don't affect package list)
66-
false // don't ignore delete events (pip uninstall)
65+
true, // ignore change events (content changes in METADATA don't affect package list)
66+
false, // don't ignore delete events (pip uninstall)
6767
);
6868
disposables.push(
6969
packageDebouncedRefresh,

src/managers/builtin/pipManager.ts renamed to src/managers/builtin/pipPackageManager.ts

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -11,29 +11,19 @@ import {
1111
} from 'vscode';
1212
import {
1313
DidChangePackagesEventArgs,
14+
GetPackagesOptions,
1415
IconPath,
1516
Package,
16-
PackageChangeKind,
1717
PackageManagementOptions,
1818
PackageManager,
1919
PythonEnvironment,
2020
PythonEnvironmentApi,
2121
} from '../../api';
22+
import { updatePackagesAndNotify } from '../common/packageChanges';
2223
import { getWorkspacePackagesToInstall } from './pipUtils';
23-
import { managePackages, refreshPackages } from './utils';
24+
import { managePackages, refreshPipPackages } from './utils';
2425
import { VenvManager } from './venvManager';
2526

26-
function getChanges(before: Package[], after: Package[]): { kind: PackageChangeKind; pkg: Package }[] {
27-
const changes: { kind: PackageChangeKind; pkg: Package }[] = [];
28-
before.forEach((pkg) => {
29-
changes.push({ kind: PackageChangeKind.remove, pkg });
30-
});
31-
after.forEach((pkg) => {
32-
changes.push({ kind: PackageChangeKind.add, pkg });
33-
});
34-
return changes;
35-
}
36-
3727
export class PipPackageManager implements PackageManager, Disposable {
3828
private readonly _onDidChangePackages = new EventEmitter<DidChangePackagesEventArgs>();
3929
onDidChangePackages: Event<DidChangePackagesEventArgs> = this._onDidChangePackages.event;
@@ -85,11 +75,15 @@ export class PipPackageManager implements PackageManager, Disposable {
8575
},
8676
async (_progress, token) => {
8777
try {
88-
const before = this.packages.get(environment.envId.id) ?? [];
89-
const after = await managePackages(environment, manageOptions, this.api, this, token);
90-
const changes = getChanges(before, after);
91-
this.packages.set(environment.envId.id, after);
92-
this._onDidChangePackages.fire({ environment, manager: this, changes });
78+
await managePackages(environment, manageOptions, this, token);
79+
await updatePackagesAndNotify(
80+
this,
81+
environment,
82+
this.packages.get(environment.envId.id),
83+
(changes) => {
84+
this._onDidChangePackages.fire({ environment, manager: this, changes });
85+
},
86+
);
9387
} catch (e) {
9488
if (e instanceof CancellationError) {
9589
throw e;
@@ -114,19 +108,19 @@ export class PipPackageManager implements PackageManager, Disposable {
114108
title: 'Refreshing packages',
115109
},
116110
async () => {
117-
const before = this.packages.get(environment.envId.id) ?? [];
118-
const after = await refreshPackages(environment, this.api, this);
119-
const changes = getChanges(before, after);
120-
this.packages.set(environment.envId.id, after);
121-
if (changes.length > 0) {
111+
await updatePackagesAndNotify(this, environment, this.packages.get(environment.envId.id), (changes) => {
122112
this._onDidChangePackages.fire({ environment, manager: this, changes });
123-
}
113+
});
124114
},
125115
);
126116
}
127-
async getPackages(environment: PythonEnvironment): Promise<Package[] | undefined> {
128-
if (!this.packages.has(environment.envId.id)) {
129-
await this.refresh(environment);
117+
118+
async getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise<Package[] | undefined> {
119+
if (options?.skipCache || !this.packages.has(environment.envId.id)) {
120+
const data = await refreshPipPackages(environment, this.log);
121+
const packages = (data ?? []).map((pkg) => this.api.createPackageItem(pkg, environment, this));
122+
this.packages.set(environment.envId.id, packages);
123+
return packages;
130124
}
131125
return this.packages.get(environment.envId.id);
132126
}

‎src/managers/builtin/utils.ts‎

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -185,12 +185,12 @@ export async function refreshPythons(
185185

186186
const PIP_LIST_TIMEOUT_MS = 30_000;
187187

188-
async function refreshPipPackagesRaw(environment: PythonEnvironment, log?: LogOutputChannel): Promise<string> {
188+
async function execPipList(environment: PythonEnvironment, log?: LogOutputChannel, args?: string[]): Promise<string> {
189189
// Use environmentPath directly for consistency with UV environment tracking
190190
const useUv = await shouldUseUv(log, environment.environmentPath.fsPath);
191191
if (useUv) {
192192
return await runUV(
193-
['pip', 'list', '--python', environment.execInfo.run.executable, '--format=json'],
193+
['pip', 'list', '--python', environment.execInfo.run.executable, '--format=json', ...(args ?? [])],
194194
undefined,
195195
log,
196196
undefined,
@@ -228,11 +228,11 @@ export async function refreshPipPackages(
228228
location: ProgressLocation.Notification,
229229
},
230230
async () => {
231-
return await refreshPipPackagesRaw(environment, log);
231+
return await execPipList(environment, log);
232232
},
233233
);
234234
} else {
235-
data = await refreshPipPackagesRaw(environment, log);
235+
data = await execPipList(environment, log);
236236
}
237237

238238
return parsePipListJson(data);
@@ -243,22 +243,12 @@ export async function refreshPipPackages(
243243
}
244244
}
245245

246-
export async function refreshPackages(
247-
environment: PythonEnvironment,
248-
api: PythonEnvironmentApi,
249-
manager: PackageManager,
250-
): Promise<Package[]> {
251-
const data = await refreshPipPackages(environment, manager.log);
252-
return (data ?? []).map((pkg) => api.createPackageItem(pkg, environment, manager));
253-
}
254-
255246
export async function managePackages(
256247
environment: PythonEnvironment,
257248
options: PackageManagementOptions,
258-
api: PythonEnvironmentApi,
259249
manager: PackageManager,
260250
token?: CancellationToken,
261-
): Promise<Package[]> {
251+
): Promise<void> {
262252
if (environment.version.startsWith('2.')) {
263253
throw new Error('Python 2.* is not supported (deprecated)');
264254
}
@@ -310,8 +300,6 @@ export async function managePackages(
310300
);
311301
}
312302
}
313-
314-
return await refreshPackages(environment, api, manager);
315303
}
316304

317305
/**

0 commit comments

Comments
 (0)