Skip to content

Commit

Permalink
Fix telemetry when updating pip (#20903)
Browse files Browse the repository at this point in the history
@luabud This PR adds a minor telemetry change to create environment.
There is a new telemetry point indicating pip upgrade.
  • Loading branch information
karthiknadig committed Mar 29, 2023
1 parent b208384 commit ef6511e
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 26 deletions.
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 @@ -439,6 +439,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 selectPythonPlaceHolder = l10n.t('Select a Python installation to create the virtual environment');
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

0 comments on commit ef6511e

Please sign in to comment.