Skip to content

Commit

Permalink
Fix for env creation errors (#20075)
Browse files Browse the repository at this point in the history
1. We ensure that non-zero error codes are handled
2. We show error message, with got to logs, and select interpreter.
3. Uses conda base environment to find base python.

Fixes: #20072
Fixes: #20062
  • Loading branch information
karthiknadig authored Oct 27, 2022
1 parent 654fd19 commit 83fff4f
Show file tree
Hide file tree
Showing 9 changed files with 188 additions and 30 deletions.
4 changes: 3 additions & 1 deletion src/client/pythonEnvironments/creation/common/commonUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ import { executeCommand } from '../../../common/vscodeApis/commandApis';
import { showErrorMessage } from '../../../common/vscodeApis/windowApis';

export async function showErrorMessageWithLogs(message: string): Promise<void> {
const result = await showErrorMessage(message, Common.openOutputPanel);
const result = await showErrorMessage(message, Common.openOutputPanel, Common.selectPythonInterpreter);
if (result === Common.openOutputPanel) {
await executeCommand(Commands.ViewOutput);
} else if (result === Common.selectPythonInterpreter) {
await executeCommand(Commands.Set_Interpreter);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { createDeferred } from '../../../common/utils/async';
import { getEnvironmentVariable, getOSType, OSType } from '../../../common/utils/platform';
import { createCondaScript } from '../../../common/process/internal/scripts';
import { Common, CreateEnv } from '../../../common/utils/localize';
import { getConda, pickPythonVersion } from './condaUtils';
import { getCondaBaseEnv, pickPythonVersion } from './condaUtils';
import { showErrorMessageWithLogs } from '../common/commonUtils';
import { withProgress } from '../../../common/vscodeApis/windowApis';
import { EventName } from '../../../telemetry/constants';
Expand Down Expand Up @@ -99,7 +99,7 @@ async function createCondaEnv(
pathEnv = `${libPath}${path.delimiter}${pathEnv}`;
}
traceLog('Running Conda Env creation script: ', [command, ...args]);
const { out, dispose } = execObservable(command, args, {
const { proc, out, dispose } = execObservable(command, args, {
mergeStdOutErr: true,
token,
cwd: workspace.uri.fsPath,
Expand All @@ -125,37 +125,33 @@ async function createCondaEnv(
},
() => {
dispose();
if (!deferred.rejected) {
if (proc?.exitCode !== 0) {
traceError('Error while running venv creation script: ', progressAndTelemetry.getLastError());
deferred.reject(progressAndTelemetry.getLastError());
} else {
deferred.resolve(condaEnvPath);
}
},
);
return deferred.promise;
}

function getExecutableCommand(condaPath: string): string {
function getExecutableCommand(condaBaseEnvPath: string): string {
if (getOSType() === OSType.Windows) {
// Both Miniconda3 and Anaconda3 have the following structure:
// Miniconda3 (or Anaconda3)
// |- condabin
// | |- conda.bat <--- this actually points to python.exe below,
// | after adding few paths to PATH.
// |- Scripts
// | |- conda.exe <--- this is the path we get as condaPath,
// | which is really a stub for `python.exe -m conda`.
// |- python.exe <--- this is the python that we want.
return path.join(path.dirname(path.dirname(condaPath)), 'python.exe');
return path.join(condaBaseEnvPath, 'python.exe');
}
// On non-windows machines:
// miniconda (or miniforge or anaconda3)
// |- bin
// |- conda <--- this is the path we get as condaPath.
// |- python <--- this is the python that we want.
return path.join(path.dirname(condaPath), 'python');
return path.join(condaBaseEnvPath, 'bin', 'python');
}

async function createEnvironment(options?: CreateEnvironmentOptions): Promise<CreateEnvironmentResult | undefined> {
const conda = await getConda();
const conda = await getCondaBaseEnv();
if (!conda) {
return undefined;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ export class CondaProgressAndTelemetry {

private condaInstalledPackagesReported = false;

private lastError: string | undefined = undefined;

constructor(private readonly progress: CreateEnvironmentProgress) {}

public process(output: string): void {
Expand Down Expand Up @@ -51,6 +53,7 @@ export class CondaProgressAndTelemetry {
environmentType: 'conda',
reason: 'other',
});
this.lastError = CREATE_CONDA_FAILED_MARKER;
} else if (!this.condaInstallingPackagesReported && output.includes(CONDA_INSTALLING_YML)) {
this.condaInstallingPackagesReported = true;
this.progress.report({
Expand All @@ -66,6 +69,7 @@ export class CondaProgressAndTelemetry {
environmentType: 'conda',
using: 'environment.yml',
});
this.lastError = CREATE_FAILED_INSTALL_YML;
} else if (!this.condaInstalledPackagesReported && output.includes(CREATE_CONDA_INSTALLED_YML)) {
this.condaInstalledPackagesReported = true;
sendTelemetryEvent(EventName.ENVIRONMENT_INSTALLED_PACKAGES, undefined, {
Expand All @@ -74,4 +78,8 @@ export class CondaProgressAndTelemetry {
});
}
}

public getLastError(): string | undefined {
return this.lastError;
}
}
18 changes: 16 additions & 2 deletions src/client/pythonEnvironments/creation/provider/condaUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import { Common } from '../../../browser/localize';
import { CreateEnv } from '../../../common/utils/localize';
import { executeCommand } from '../../../common/vscodeApis/commandApis';
import { showErrorMessage, showQuickPick } from '../../../common/vscodeApis/windowApis';
import { traceLog } from '../../../logging';
import { Conda } from '../../common/environmentManagers/conda';

export async function getConda(): Promise<string | undefined> {
export async function getCondaBaseEnv(): Promise<string | undefined> {
const conda = await Conda.getConda();

if (!conda) {
Expand All @@ -18,7 +19,20 @@ export async function getConda(): Promise<string | undefined> {
}
return undefined;
}
return conda.command;

const envs = (await conda.getEnvList()).filter((e) => e.name === 'base');
if (envs.length === 1) {
return envs[0].prefix;
}
if (envs.length > 1) {
traceLog(
'Multiple conda base envs detected: ',
envs.map((e) => e.prefix),
);
return undefined;
}

return undefined;
}

export async function pickPythonVersion(token?: CancellationToken): Promise<string | undefined> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ async function createVenv(

const deferred = createDeferred<string | undefined>();
traceLog('Running Env creation script: ', [command, ...args]);
const { out, dispose } = execObservable(command, args, {
const { proc, out, dispose } = execObservable(command, args, {
mergeStdOutErr: true,
token,
cwd: workspace.uri.fsPath,
Expand All @@ -101,7 +101,10 @@ async function createVenv(
},
() => {
dispose();
if (!deferred.rejected) {
if (proc?.exitCode !== 0) {
traceError('Error while running venv creation script: ', progressAndTelemetry.getLastError());
deferred.reject(progressAndTelemetry.getLastError());
} else {
deferred.resolve(venvPath);
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ export class VenvProgressAndTelemetry {

private venvInstalledPackagesReported = false;

private lastError: string | undefined = undefined;

constructor(private readonly progress: CreateEnvironmentProgress) {}

public process(output: string): void {
Expand Down Expand Up @@ -60,18 +62,21 @@ export class VenvProgressAndTelemetry {
environmentType: 'venv',
reason: 'noVenv',
});
this.lastError = VENV_NOT_INSTALLED_MARKER;
} else if (!this.venvOrPipMissingReported && output.includes(PIP_NOT_INSTALLED_MARKER)) {
this.venvOrPipMissingReported = true;
sendTelemetryEvent(EventName.ENVIRONMENT_FAILED, undefined, {
environmentType: 'venv',
reason: 'noPip',
});
this.lastError = PIP_NOT_INSTALLED_MARKER;
} else if (!this.venvFailedReported && output.includes(CREATE_VENV_FAILED_MARKER)) {
this.venvFailedReported = true;
sendTelemetryEvent(EventName.ENVIRONMENT_FAILED, undefined, {
environmentType: 'venv',
reason: 'other',
});
this.lastError = CREATE_VENV_FAILED_MARKER;
} else if (!this.venvInstallingPackagesReported && output.includes(INSTALLING_REQUIREMENTS)) {
this.venvInstallingPackagesReported = true;
this.progress.report({
Expand All @@ -96,18 +101,21 @@ export class VenvProgressAndTelemetry {
environmentType: 'venv',
using: 'pipUpgrade',
});
this.lastError = PIP_UPGRADE_FAILED_MARKER;
} else if (!this.venvInstallingPackagesFailedReported && output.includes(INSTALL_REQUIREMENTS_FAILED_MARKER)) {
this.venvInstallingPackagesFailedReported = true;
sendTelemetryEvent(EventName.ENVIRONMENT_INSTALLING_PACKAGES_FAILED, undefined, {
environmentType: 'venv',
using: 'requirements.txt',
});
this.lastError = INSTALL_REQUIREMENTS_FAILED_MARKER;
} else if (!this.venvInstallingPackagesFailedReported && output.includes(INSTALL_PYPROJECT_FAILED_MARKER)) {
this.venvInstallingPackagesFailedReported = true;
sendTelemetryEvent(EventName.ENVIRONMENT_INSTALLING_PACKAGES_FAILED, undefined, {
environmentType: 'venv',
using: 'pyproject.toml',
});
this.lastError = INSTALL_PYPROJECT_FAILED_MARKER;
} else if (!this.venvInstalledPackagesReported && output.includes(INSTALLED_REQUIREMENTS_MARKER)) {
this.venvInstalledPackagesReported = true;
sendTelemetryEvent(EventName.ENVIRONMENT_INSTALLED_PACKAGES, undefined, {
Expand All @@ -122,4 +130,8 @@ export class VenvProgressAndTelemetry {
});
}
}

public getLastError(): string | undefined {
return this.lastError;
}
}
2 changes: 1 addition & 1 deletion src/client/telemetry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export function sendTelemetryEvent<P extends IEventNamePropertyMapping, E extend
break;
}
} catch (exception) {
console.error(`Failed to serialize ${prop} for ${eventName}`, exception);
console.error(`Failed to serialize ${prop} for ${String(eventName)}`, exception);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ chaiUse(chaiAsPromised);
suite('Conda Creation provider tests', () => {
let condaProvider: CreateEnvironmentProvider;
let progressMock: typemoq.IMock<CreateEnvironmentProgress>;
let getCondaStub: sinon.SinonStub;
let getCondaBaseEnvStub: sinon.SinonStub;
let pickPythonVersionStub: sinon.SinonStub;
let pickWorkspaceFolderStub: sinon.SinonStub;
let execObservableStub: sinon.SinonStub;
Expand All @@ -38,7 +38,7 @@ suite('Conda Creation provider tests', () => {

setup(() => {
pickWorkspaceFolderStub = sinon.stub(wsSelect, 'pickWorkspaceFolder');
getCondaStub = sinon.stub(condaUtils, 'getConda');
getCondaBaseEnvStub = sinon.stub(condaUtils, 'getCondaBaseEnv');
pickPythonVersionStub = sinon.stub(condaUtils, 'pickPythonVersion');
execObservableStub = sinon.stub(rawProcessApis, 'execObservable');
withProgressStub = sinon.stub(windowApis, 'withProgress');
Expand All @@ -55,20 +55,20 @@ suite('Conda Creation provider tests', () => {
});

test('No conda installed', async () => {
getCondaStub.resolves(undefined);
getCondaBaseEnvStub.resolves(undefined);

assert.isUndefined(await condaProvider.createEnvironment());
});

test('No workspace selected', async () => {
getCondaStub.resolves('/usr/bin/conda');
getCondaBaseEnvStub.resolves('/usr/bin/conda');
pickWorkspaceFolderStub.resolves(undefined);

assert.isUndefined(await condaProvider.createEnvironment());
});

test('No python version picked selected', async () => {
getCondaStub.resolves('/usr/bin/conda');
getCondaBaseEnvStub.resolves('/usr/bin/conda');
pickWorkspaceFolderStub.resolves({
uri: Uri.file(path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'testMultiRootWkspc', 'workspace1')),
name: 'workspace1',
Expand All @@ -80,7 +80,7 @@ suite('Conda Creation provider tests', () => {
});

test('Create conda environment', async () => {
getCondaStub.resolves('/usr/bin/conda/conda_bin/conda');
getCondaBaseEnvStub.resolves('/usr/bin/conda');
const workspace1 = {
uri: Uri.file(path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'testMultiRootWkspc', 'workspace1')),
name: 'workspace1',
Expand All @@ -95,7 +95,9 @@ suite('Conda Creation provider tests', () => {
execObservableStub.callsFake(() => {
deferred.resolve();
return {
proc: undefined,
proc: {
exitCode: 0,
},
out: {
subscribe: (
next?: (value: Output<string>) => void,
Expand Down Expand Up @@ -134,7 +136,7 @@ suite('Conda Creation provider tests', () => {
});

test('Create conda environment failed', async () => {
getCondaStub.resolves('/usr/bin/conda/conda_bin/conda');
getCondaBaseEnvStub.resolves('/usr/bin/conda');
pickWorkspaceFolderStub.resolves({
uri: Uri.file(path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'testMultiRootWkspc', 'workspace1')),
name: 'workspace1',
Expand Down Expand Up @@ -183,4 +185,60 @@ suite('Conda Creation provider tests', () => {
await assert.isRejected(promise);
assert.isTrue(showErrorMessageWithLogsStub.calledOnce);
});

test('Create conda environment failed (non-zero exit code)', async () => {
getCondaBaseEnvStub.resolves('/usr/bin/conda');
const workspace1 = {
uri: Uri.file(path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'testMultiRootWkspc', 'workspace1')),
name: 'workspace1',
index: 0,
};
pickWorkspaceFolderStub.resolves(workspace1);
pickPythonVersionStub.resolves('3.10');

const deferred = createDeferred();
let _next: undefined | ((value: Output<string>) => void);
let _complete: undefined | (() => void);
execObservableStub.callsFake(() => {
deferred.resolve();
return {
proc: {
exitCode: 1,
},
out: {
subscribe: (
next?: (value: Output<string>) => void,
_error?: (error: unknown) => void,
complete?: () => void,
) => {
_next = next;
_complete = complete;
},
},
dispose: () => undefined,
};
});

progressMock.setup((p) => p.report({ message: CreateEnv.statusStarting })).verifiable(typemoq.Times.once());

withProgressStub.callsFake(
(
_options: ProgressOptions,
task: (
progress: CreateEnvironmentProgress,
token?: CancellationToken,
) => Thenable<CreateEnvironmentResult>,
) => task(progressMock.object),
);

const promise = condaProvider.createEnvironment();
await deferred.promise;
assert.isDefined(_next);
assert.isDefined(_complete);

_next!({ out: `${CONDA_ENV_CREATED_MARKER}new_environment`, source: 'stdout' });
_complete!();
await assert.isRejected(promise);
assert.isTrue(showErrorMessageWithLogsStub.calledOnce);
});
});
Loading

0 comments on commit 83fff4f

Please sign in to comment.