Skip to content

Commit

Permalink
Emit metrics for how the Python version was chosen
Browse files Browse the repository at this point in the history
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@.
  • Loading branch information
edmorley committed Sep 18, 2020
1 parent 64abfb2 commit 82285e1
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
13 changes: 9 additions & 4 deletions bin/compile
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
Empty file.
24 changes: 24 additions & 0 deletions test/run-versions
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 82285e1

Please sign in to comment.