From eb6ee49dfe81dc26d8194555be52b451c3d33672 Mon Sep 17 00:00:00 2001 From: Ed Morley <501702+edmorley@users.noreply.github.com> Date: Fri, 18 Sep 2020 18:48:57 +0100 Subject: [PATCH] Emit metrics for how the Python version was chosen (#1069) Currently an app's Python version can be set via a few different means: - explicitly by the user (via `runtime.txt` or `Pipfile.lock`) - implicitly via the sticky versions feature (for existing apps) - implicitly via default version for new apps / those with empty cache In order to determine the priority of features like automatic Python patch version upgrades for sticky-versioned apps, it's useful to have metrics for these. There were previously no tests for either the sticky versions feature, or changing the Python version by updating the `runtime.txt` file, so I've added some now (given that I updated the conditional to add the metrics, so useful to have coverage). I've also removed the confusing overwrite of `DEFAULT_PYTHON_VERSION` with the cached version, and kept them as two separate variables. Closes @W-8099632@. Closes @W-8099645@. --- CHANGELOG.md | 1 + bin/compile | 13 ++++++---- test/fixtures/no-runtime-txt/requirements.txt | 0 test/run-versions | 24 +++++++++++++++++++ 4 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 test/fixtures/no-runtime-txt/requirements.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bdbb09..f0421aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Emit metrics for how the Python version was chosen for an app (#1069). - Emit Python version metric events for all builds, not just clean installs (#1066). ## v178 (2020-09-07) diff --git a/bin/compile b/bin/compile index 40369cc..f7b429f 100755 --- a/bin/compile +++ b/bin/compile @@ -189,7 +189,7 @@ source "$BIN_DIR/steps/hooks/pre_compile" # continue to use that version of Python in perpituity (warnings will be raised if # they are out–of–date). if [ -f "$CACHE_DIR/.heroku/python-version" ]; then - DEFAULT_PYTHON_VERSION=$(cat "$CACHE_DIR/.heroku/python-version") + CACHED_PYTHON_VERSION=$(cat "$CACHE_DIR/.heroku/python-version") fi # We didn't always record the stack version. This code is in place because of that. @@ -206,9 +206,14 @@ fi # shellcheck source=bin/steps/pipenv-python-version source "$BIN_DIR/steps/pipenv-python-version" -# If no runtime was provided by the user, assume the default Python runtime version. -if [ ! -f runtime.txt ]; then - echo "$DEFAULT_PYTHON_VERSION" > runtime.txt +if [[ -f runtime.txt ]]; then + mcount "version.reason.python.specified" +elif [[ -n "${CACHED_PYTHON_VERSION:-}" ]]; then + mcount "version.reason.python.cached" + echo "${CACHED_PYTHON_VERSION}" > runtime.txt +else + mcount "version.reason.python.default" + echo "${DEFAULT_PYTHON_VERSION}" > runtime.txt fi # Create the directory for .profile.d, if it doesn't exist. diff --git a/test/fixtures/no-runtime-txt/requirements.txt b/test/fixtures/no-runtime-txt/requirements.txt new file mode 100644 index 0000000..e69de29 diff --git a/test/run-versions b/test/run-versions index 8413e27..1e1ca30 100755 --- a/test/run-versions +++ b/test/run-versions @@ -214,6 +214,30 @@ testPypy2_7_warn() { fi } +testStickyPythonVersion() { + local cache_dir="$(mktmpdir)" + compile "python3_6_warn" "$cache_dir" + assertCaptured "Installing python-3.6.7" + assertCapturedSuccess + compile "no-runtime-txt" "$cache_dir" + assertCaptured "Installing python-3.6.7" + assertCapturedSuccess + # Whilst this file seems like an implementation detail (so something that should + # not be tested), we must guarantee the filename remains consistent for backwards + # compatibility across buildpack versions for already-built apps. + assertFile "python-3.6.7" ".heroku/python-version" +} + +testPythonVersionChange() { + local cache_dir="$(mktmpdir)" + compile "python3_6_warn" "$cache_dir" + assertCaptured "Installing python-3.6.7" + assertCapturedSuccess + compile "python3_6" "$cache_dir" + assertCaptured "Found python-3.6.7, removing" + assertCapturedSuccess +} + pushd $(dirname 0) >/dev/null popd >/dev/null