Skip to content

Additional graceful handling for pixi #23942

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Aug 20, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { IDisposable } from '../../../../common/types';
import { createDeferred, Deferred } from '../../../../common/utils/async';
import { Disposables } from '../../../../common/utils/resourceLifecycle';
import { traceError } from '../../../../logging';
import { traceError, traceWarn } from '../../../../logging';
import { arePathsSame, isVirtualWorkspace } from '../../../common/externalDependencies';
import { getEnvPath } from '../../info/env';
import { BasicEnvInfo, IPythonEnvsIterator, Locator, PythonLocatorQuery } from '../../locator';
Expand Down Expand Up @@ -36,7 +36,11 @@ export abstract class LazyResourceBasedLocator extends Locator<BasicEnvInfo> imp
protected async activate(): Promise<void> {
await this.ensureResourcesReady();
// There is not need to wait for the watchers to get started.
this.ensureWatchersReady().ignoreErrors();
try {
this.ensureWatchersReady();
} catch (ex) {
traceWarn(`Failed to ensure watchers are ready for locator ${this.constructor.name}`, ex);
}
}

public async dispose(): Promise<void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as path from 'path';
import { Uri } from 'vscode';
import { FileChangeType, watchLocationForPattern } from '../../../../common/platform/fileSystemWatcher';
import { sleep } from '../../../../common/utils/async';
import { traceError, traceVerbose } from '../../../../logging';
import { traceVerbose, traceWarn } from '../../../../logging';
import { getEnvironmentDirFromPath } from '../../../common/commonUtils';
import {
PythonEnvStructure,
Expand All @@ -32,13 +32,13 @@ function checkDirWatchable(dirname: string): DirUnwatchableReason {
names = fs.readdirSync(dirname);
} catch (err) {
const exception = err as NodeJS.ErrnoException;
traceError('Reading directory to watch failed', exception);
traceVerbose('Reading directory failed', exception);
if (exception.code === 'ENOENT') {
// Treat a missing directory as unwatchable since it can lead to CPU load issues:
// https://github.com/microsoft/vscode-python/issues/18459
return 'directory does not exist';
}
throw err; // re-throw
return undefined;
}
// The limit here is an educated guess.
if (names.length > 200) {
Expand Down Expand Up @@ -117,7 +117,7 @@ export abstract class FSWatchingLocator extends LazyResourceBasedLocator {
// that might be watched due to a glob are not checked.
const unwatchable = await checkDirWatchable(root);
if (unwatchable) {
traceError(`Dir "${root}" is not watchable (${unwatchable})`);
traceWarn(`Dir "${root}" is not watchable (${unwatchable})`);
return undefined;
}
return root;
Expand Down
10 changes: 4 additions & 6 deletions src/client/pythonEnvironments/common/environmentManagers/pixi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { OSType, getOSType, getUserHomeDir } from '../../../common/utils/platfor
import { exec, getPythonSetting, onDidChangePythonSetting, pathExists, pathExistsSync } from '../externalDependencies';
import { cache } from '../../../common/utils/decorators';
import { isTestExecution } from '../../../common/constants';
import { traceError, traceVerbose, traceWarn } from '../../../logging';
import { traceVerbose, traceWarn } from '../../../logging';
import { OUTPUT_MARKER_SCRIPT } from '../../../common/process/internal/scripts';

export const PIXITOOLPATH_SETTING_KEY = 'pixiToolPath';
Expand Down Expand Up @@ -119,7 +119,7 @@ export class Pixi {
yield customPixiToolPath;
}
} catch (ex) {
traceError(`Failed to get pixi setting`, ex);
traceWarn(`Failed to get pixi setting`, ex);
}

// Check unqualified filename, in case it's on PATH.
Expand Down Expand Up @@ -182,7 +182,7 @@ export class Pixi {
const pixiInfo: PixiInfo = JSON.parse(infoOutput.stdout);
return pixiInfo;
} catch (error) {
traceError(`Failed to get pixi info for ${cwd}`, error);
traceWarn(`Failed to get pixi info for ${cwd}`, error);
return undefined;
}
}
Expand All @@ -199,15 +199,13 @@ export class Pixi {
if (!versionOutput || !versionOutput.stdout) {
return undefined;
}

const versionParts = versionOutput.stdout.split(' ');
if (versionParts.length < 2) {
return undefined;
}

return versionParts[1].trim();
} catch (error) {
traceError(`Failed to get pixi version`, error);
traceVerbose(`Failed to get pixi version`, error);
return undefined;
}
}
Expand Down
Loading