Skip to content
This repository has been archived by the owner on Dec 23, 2021. It is now read-only.

Venv fixes #228

Merged
merged 7 commits into from
Feb 27, 2020
Merged

Venv fixes #228

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1262a9b
progress on venv bug fix
andreamah Feb 27, 2020
c0c6409
fixes to virtual env workflow
andreamah Feb 27, 2020
82d78bc
formatting
andreamah Feb 27, 2020
76057db
removed console logs
andreamah Feb 27, 2020
dbeb7fe
Merge branch 'dev' into users/t-anmah/ext-venv-fix
andreamah Feb 27, 2020
6ea6dec
formatting
andreamah Feb 27, 2020
6ed2556
Merge branch 'dev' into users/t-anmah/ext-venv-fix
andreamah Feb 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
168 changes: 72 additions & 96 deletions src/extension_utils/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,23 +345,11 @@ export const promptInstallVenv = (
if (selection === DialogResponses.YES) {
return installPythonVenv(context, pythonExecutable);
} else {
return vscode.window
.showInformationMessage(
CONSTANTS.INFO.ARE_YOU_SURE,
DialogResponses.INSTALL_NOW,
DialogResponses.DONT_INSTALL
)
.then((installChoice: vscode.MessageItem | undefined) => {
if (installChoice === DialogResponses.INSTALL_NOW) {
return installPythonVenv(context, pythonExecutable);
} else {
// return an empty string, notifying the caller
// that the user was unwilling to create venv
// and by default, this will trigger the extension to
// try using pythonExecutable
return "";
}
});
// return pythonExecutable, notifying the caller
// that the user was unwilling to create venv
// and by default, this will trigger the extension to
// try using pythonExecutable
return pythonExecutable;
}
});
};
Expand Down Expand Up @@ -410,22 +398,7 @@ export const installPythonVenv = async (
return pythonExecutable;
}

if (!(await installDependencies(context, pythonPath))) {
vscode.window
.showErrorMessage(
`${CONSTANTS.ERROR.DEPENDENCY_DOWNLOAD_ERROR} Using original interpreter at: ${pythonExecutable}.`,
DialogResponses.READ_INSTALL_MD
)
.then((selection: vscode.MessageItem | undefined) => {
if (selection === DialogResponses.READ_INSTALL_MD) {
open(CONSTANTS.LINKS.INSTALL);
}
});

return pythonExecutable;
}

return pythonPath;
return installDependenciesWrapper(context, pythonPath, pythonExecutable);
};

export const areDependenciesInstalled = async (
Expand Down Expand Up @@ -481,6 +454,30 @@ export const installDependencies = async (
}
};

export const installDependenciesWrapper = async (
context: vscode.ExtensionContext,
pythonPath: string,
backupPythonPath: string = ""
) => {
let errMessage = CONSTANTS.ERROR.DEPENDENCY_DOWNLOAD_ERROR;
if (backupPythonPath !== "") {
errMessage = `${errMessage} Using original interpreter at: ${backupPythonPath}.`;
}
if (!(await installDependencies(context, pythonPath))) {
vscode.window
.showErrorMessage(
CONSTANTS.ERROR.DEPENDENCY_DOWNLOAD_ERROR,
DialogResponses.READ_INSTALL_MD
)
.then((selection: vscode.MessageItem | undefined) => {
if (selection === DialogResponses.READ_INSTALL_MD) {
open(CONSTANTS.LINKS.INSTALL);
}
});
return backupPythonPath;
}
return pythonPath;
};
export const getCurrentPythonExecutableName = async () => {
let originalPythonExecutableName = "";

Expand Down Expand Up @@ -549,40 +546,22 @@ export const setupEnv = async (
let pythonExecutableName = originalPythonExecutableName;

if (!(await areDependenciesInstalled(context, pythonExecutableName))) {
const pythonExecutableNameVenv = await getPythonVenv(context);
// environment needs to install dependencies
if (!(await checkIfVenv(context, pythonExecutableName))) {
pythonExecutableName = await getPythonVenv(context);
if (await hasVenv(context)) {
// venv in extention exists with wrong dependencies
if (
!(await areDependenciesInstalled(
context,
pythonExecutableName
pythonExecutableNameVenv
))
) {
if (
!(await installDependencies(
context,
pythonExecutableName
))
) {
vscode.window
.showErrorMessage(
`${CONSTANTS.ERROR.DEPENDENCY_DOWNLOAD_ERROR} Using original interpreter at: ${pythonExecutableName}.`,
DialogResponses.READ_INSTALL_MD
)
.then(
(selection: vscode.MessageItem | undefined) => {
if (
selection ===
DialogResponses.READ_INSTALL_MD
) {
open(CONSTANTS.LINKS.INSTALL);
}
}
);
return pythonExecutableName;
}
pythonExecutableName = await installDependenciesWrapper(
context,
pythonExecutableNameVenv,
pythonExecutableName
);
}
} else {
pythonExecutableName = await promptInstallVenv(
Expand All @@ -591,8 +570,14 @@ export const setupEnv = async (
);
}
}

if (pythonExecutableName === originalPythonExecutableName) {
if (pythonExecutableName === pythonExecutableNameVenv) {
vscode.window.showInformationMessage(
CONSTANTS.INFO.UPDATED_TO_EXTENSION_VENV
);
vscode.workspace
.getConfiguration()
.update(CONFIG.PYTHON_PATH, pythonExecutableName);
} else if (pythonExecutableName === originalPythonExecutableName) {
// going with original interpreter, either because
// already in venv or error in creating custom venv
if (checkConfig(CONFIG.SHOW_DEPENDENCY_INSTALL)) {
Expand All @@ -607,47 +592,38 @@ export const setupEnv = async (
installChoice: vscode.MessageItem | undefined
) => {
if (installChoice === DialogResponses.INSTALL_NOW) {
if (
!(await installDependencies(
context,
pythonExecutableName
))
) {
vscode.window
.showErrorMessage(
CONSTANTS.ERROR
.DEPENDENCY_DOWNLOAD_ERROR,
DialogResponses.READ_INSTALL_MD
)
.then(
(
selection:
| vscode.MessageItem
| undefined
) => {
if (
selection ===
DialogResponses.READ_INSTALL_MD
) {
open(
CONSTANTS.LINKS.INSTALL
);
}
await installDependenciesWrapper(
context,
pythonExecutableName
);
} else {
await vscode.window
.showInformationMessage(
CONSTANTS.INFO.ARE_YOU_SURE,
DialogResponses.INSTALL_NOW,
DialogResponses.DONT_INSTALL
)
Comment on lines +601 to +605
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be ok to remove this prompt, as it is the third prompt in a row asking about installing dependencies. Up to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps we should leave it in for now, since people who just close popups on instinct should be advised that the extension won't work without installing dependencies! Maybe we can incorporate this info into an earlier popup later on :0

.then(
async (
installChoice2:
| vscode.MessageItem
| undefined
) => {
if (
installChoice2 ===
DialogResponses.INSTALL_NOW
) {
await installDependenciesWrapper(
context,
pythonExecutableName
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't install the dependencies (perhaps you miss the prompts) and try running some micro:bit code, VS Code only shows a generic error:

[ERROR] Traceback (most recent call last):
  File "c:\Users\user\.vscode\extensions\vscode-python-devicesimulator\out\process_user_code.py", line 26, in <module>
    from common.telemetry import telemetry_py
  File "c:\Users\user\.vscode\extensions\vscode-python-devicesimulator\out\common\telemetry.py", line 1, in <module>
    from applicationinsights import TelemetryClient
ModuleNotFoundError: No module named 'applicationinsights'

It would be much more helpful if we could show the user a more helpful message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True! Perhaps that'd be good for another release, since we're rushing for the release in an hour :'''(

}
);
return pythonExecutableName;
}
}
);
}
}
);
}
} else {
vscode.window.showInformationMessage(
CONSTANTS.INFO.UPDATED_TO_EXTENSION_VENV
);
vscode.workspace
.getConfiguration()
.update(CONFIG.PYTHON_PATH, pythonExecutableName);
}
} else if (needsResponse) {
vscode.window.showInformationMessage(
Expand Down