Skip to content
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

Fix telemetry when updating pip #20903

Merged
merged 1 commit into from
Mar 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 8 additions & 8 deletions pythonFiles/create_venv.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,12 @@ def install_requirements(venv_path: str, requirements: List[str]) -> None:
if not requirements:
return

print(f"VENV_INSTALLING_REQUIREMENTS: {requirements}")
args = []
for requirement in requirements:
args += ["-r", requirement]
run_process(
[venv_path, "-m", "pip", "install"] + args,
"CREATE_VENV.PIP_FAILED_INSTALL_REQUIREMENTS",
)
print(f"VENV_INSTALLING_REQUIREMENTS: {requirement}")
run_process(
[venv_path, "-m", "pip", "install", "-r", requirement],
"CREATE_VENV.PIP_FAILED_INSTALL_REQUIREMENTS",
)
print("CREATE_VENV.PIP_INSTALLED_REQUIREMENTS")


Expand All @@ -111,10 +109,12 @@ def install_toml(venv_path: str, extras: List[str]) -> None:


def upgrade_pip(venv_path: str) -> None:
print("CREATE_VENV.UPGRADING_PIP")
run_process(
[venv_path, "-m", "pip", "install", "--upgrade", "pip"],
"CREATE_VENV.PIP_UPGRADE_FAILED",
"CREATE_VENV.UPGRADE_PIP_FAILED",
)
print("CREATE_VENV.UPGRADED_PIP")


def add_gitignore(name: str) -> None:
Expand Down
20 changes: 7 additions & 13 deletions pythonFiles/tests/test_create_venv.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def run_process(args, error_message):
nonlocal pip_upgraded, installing
if args[1:] == ["-m", "pip", "install", "--upgrade", "pip"]:
pip_upgraded = True
assert error_message == "CREATE_VENV.PIP_UPGRADE_FAILED"
assert error_message == "CREATE_VENV.UPGRADE_PIP_FAILED"
elif args[1:-1] == ["-m", "pip", "install", "-r"]:
installing = "requirements"
assert error_message == "CREATE_VENV.PIP_FAILED_INSTALL_REQUIREMENTS"
Expand Down Expand Up @@ -146,34 +146,28 @@ def run_process(args, error_message):
@pytest.mark.parametrize(
("extras", "expected"),
[
([], None),
([], []),
(
["requirements/test.txt"],
[sys.executable, "-m", "pip", "install", "-r", "requirements/test.txt"],
[[sys.executable, "-m", "pip", "install", "-r", "requirements/test.txt"]],
),
(
["requirements/test.txt", "requirements/doc.txt"],
[
sys.executable,
"-m",
"pip",
"install",
"-r",
"requirements/test.txt",
"-r",
"requirements/doc.txt",
[sys.executable, "-m", "pip", "install", "-r", "requirements/test.txt"],
[sys.executable, "-m", "pip", "install", "-r", "requirements/doc.txt"],
],
),
],
)
def test_requirements_args(extras, expected):
importlib.reload(create_venv)

actual = None
actual = []

def run_process(args, error_message):
nonlocal actual
actual = args
actual.append(args)

create_venv.run_process = run_process

Expand Down
1 change: 1 addition & 0 deletions src/client/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,7 @@ export namespace CreateEnv {
export namespace Venv {
export const creating = l10n.t('Creating venv...');
export const created = l10n.t('Environment created...');
export const upgradingPip = l10n.t('Upgrading pip...');
export const installingPackages = l10n.t('Installing packages...');
export const errorCreatingEnvironment = l10n.t('Error while creating virtual environment.');
export const selectPythonQuickPickTitle = l10n.t('Select a python to use for environment creation');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,19 @@ export const CREATE_VENV_FAILED_MARKER = 'CREATE_VENV.VENV_FAILED_CREATION';
export const VENV_ALREADY_EXISTS_MARKER = 'CREATE_VENV.VENV_ALREADY_EXISTS';
export const INSTALLED_REQUIREMENTS_MARKER = 'CREATE_VENV.PIP_INSTALLED_REQUIREMENTS';
export const INSTALLED_PYPROJECT_MARKER = 'CREATE_VENV.PIP_INSTALLED_PYPROJECT';
export const PIP_UPGRADE_FAILED_MARKER = 'CREATE_VENV.PIP_UPGRADE_FAILED';
export const UPGRADE_PIP_FAILED_MARKER = 'CREATE_VENV.UPGRADE_PIP_FAILED';
export const UPGRADING_PIP_MARKER = 'CREATE_VENV.UPGRADING_PIP';
export const UPGRADED_PIP_MARKER = 'CREATE_VENV.UPGRADED_PIP';

export class VenvProgressAndTelemetry {
private venvCreatedReported = false;

private venvOrPipMissingReported = false;

private venvUpgradingPipReported = false;

private venvUpgradedPipReported = false;

private venvFailedReported = false;

private venvInstallingPackagesReported = false;
Expand Down Expand Up @@ -70,6 +76,15 @@ export class VenvProgressAndTelemetry {
reason: 'noPip',
});
this.lastError = PIP_NOT_INSTALLED_MARKER;
} else if (!this.venvUpgradingPipReported && output.includes(UPGRADING_PIP_MARKER)) {
this.venvUpgradingPipReported = true;
this.progress.report({
message: CreateEnv.Venv.upgradingPip,
});
sendTelemetryEvent(EventName.ENVIRONMENT_INSTALLING_PACKAGES, undefined, {
environmentType: 'venv',
using: 'pipUpgrade',
});
} else if (!this.venvFailedReported && output.includes(CREATE_VENV_FAILED_MARKER)) {
this.venvFailedReported = true;
sendTelemetryEvent(EventName.ENVIRONMENT_FAILED, undefined, {
Expand All @@ -95,13 +110,13 @@ export class VenvProgressAndTelemetry {
environmentType: 'venv',
using: 'pyproject.toml',
});
} else if (!this.venvInstallingPackagesFailedReported && output.includes(PIP_UPGRADE_FAILED_MARKER)) {
} else if (!this.venvInstallingPackagesFailedReported && output.includes(UPGRADE_PIP_FAILED_MARKER)) {
this.venvInstallingPackagesFailedReported = true;
sendTelemetryEvent(EventName.ENVIRONMENT_INSTALLING_PACKAGES_FAILED, undefined, {
environmentType: 'venv',
using: 'pipUpgrade',
});
this.lastError = PIP_UPGRADE_FAILED_MARKER;
this.lastError = UPGRADE_PIP_FAILED_MARKER;
} else if (!this.venvInstallingPackagesFailedReported && output.includes(INSTALL_REQUIREMENTS_FAILED_MARKER)) {
this.venvInstallingPackagesFailedReported = true;
sendTelemetryEvent(EventName.ENVIRONMENT_INSTALLING_PACKAGES_FAILED, undefined, {
Expand All @@ -116,6 +131,12 @@ export class VenvProgressAndTelemetry {
using: 'pyproject.toml',
});
this.lastError = INSTALL_PYPROJECT_FAILED_MARKER;
} else if (!this.venvUpgradedPipReported && output.includes(UPGRADED_PIP_MARKER)) {
this.venvUpgradedPipReported = true;
sendTelemetryEvent(EventName.ENVIRONMENT_INSTALLED_PACKAGES, undefined, {
environmentType: 'venv',
using: 'pipUpgrade',
});
} else if (!this.venvInstalledPackagesReported && output.includes(INSTALLED_REQUIREMENTS_MARKER)) {
this.venvInstalledPackagesReported = true;
sendTelemetryEvent(EventName.ENVIRONMENT_INSTALLED_PACKAGES, undefined, {
Expand Down
4 changes: 2 additions & 2 deletions src/client/telemetry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2054,7 +2054,7 @@ export interface IEventNamePropertyMapping {
*/
[EventName.ENVIRONMENT_INSTALLING_PACKAGES]: {
environmentType: 'venv' | 'conda';
using: 'requirements.txt' | 'pyproject.toml' | 'environment.yml';
using: 'requirements.txt' | 'pyproject.toml' | 'environment.yml' | 'pipUpgrade';
};
/**
* Telemetry event sent after installing packages.
Expand All @@ -2067,7 +2067,7 @@ export interface IEventNamePropertyMapping {
*/
[EventName.ENVIRONMENT_INSTALLED_PACKAGES]: {
environmentType: 'venv' | 'conda';
using: 'requirements.txt' | 'pyproject.toml' | 'environment.yml';
using: 'requirements.txt' | 'pyproject.toml' | 'environment.yml' | 'pipUpgrade';
};
/**
* Telemetry event sent if installing packages failed.
Expand Down