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.

Sign up for GitHub

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

Jump to bottom

Improve cache handling #1679

Merged
merged 1 commit into from
Oct 31, 2024
Merged

Improve cache handling #1679

merged 1 commit into from
Oct 31, 2024

Conversation

edmorley
Copy link
Member

@edmorley edmorley commented Oct 30, 2024 •
edited
Loading

Refactors cache handling to dedicated functions under lib/cache.sh (rather than being scattered around the buildpack), and makes the following improvements:

  • Ensures the cache is now also discards the cache when the package manager (or its version) changes.
  • Improves the build log output shown when restoring or discarding the cache. For example, if the cache was invalidated all reasons are now shown.
  • Stops performing unnecessary cache file copies when the cache is due to be invalidated. This required moving the cache restoration step to after the bin/pre_compile hook runs.
  • Fixes cache restoration in the case where an app's requirements.txt was formerly a symlink.
  • Adds buildpack metrics for the status of the cache and duration of cache restoration/saving.

Note: The .heroku/ namespaced cache contents are considered an internal Heroku buildpack implementation detail and should not be directly modified by any bin/{pre,post}_compile hooks or third-party buildpacks.

Fixes #1673.
Fixes #1674.
Fixes #1675.
Fixes #1676.
Fixes #1677.
Fixes #1678.
Prep for #796.
Unblocks upgrading pip (since #1674 prevents pypa/pip#12950).
GUS-W-16811131.

@edmorley edmorley self-assigned this Oct 30, 2024
@edmorley edmorley marked this pull request as ready for review October 30, 2024 21:04
@edmorley edmorley requested a review from a team as a code owner October 30, 2024 21:04
@edmorley edmorley enabled auto-merge (squash) October 30, 2024 21:05
@edmorley edmorley disabled auto-merge October 30, 2024 21:07
@edmorley edmorley enabled auto-merge (squash) October 30, 2024 21:10
@edmorley edmorley disabled auto-merge October 30, 2024 21:41
@edmorley
Improve cache handling
ce4ad9f 
Refactors cache handling to dedicated functions under `lib/cache.sh`
(rather than being scattered around the buildpack), and makes the
following improvements:
- Ensures the cache is now also discards the cache when the package
  manager (or its version) changes.
- Improves the build log output shown when restoring or discarding the
  cache. For example, if the cache was invalidated all reasons are now
  shown.
- Stops performing unnecessary cache file copies when the cache is due
  to be invalidated. This required moving the cache restoration step to
  after the `bin/pre_compile` hook runs.
- Fixes cache restoration in the case where an app's `requirements.txt`
  was formerly a symlink.
- Adds buildpack metrics for the status of the cache and duration of
  cache restoration/saving.

Fixes #1673.
Fixes #1674.
Fixes #1675.
Fixes #1676.
Fixes #1677.
Fixes #1678.
Prep for #796.
Unblocks upgrading pip (since #1674 prevents pypa/pip#12950).
GUS-W-16811131.
@edmorley edmorley merged commit f00f258 into main Oct 31, 2024
7 checks passed
@edmorley edmorley deleted the cache-refactor branch October 31, 2024 14:56
@heroku-linguist heroku-linguist bot mentioned this pull request Oct 31, 2024
Merged
edmorley added a commit that referenced this pull request Dec 6, 2024
@edmorley
Make Python version pinning pin to the major version only
cc277c9 
For apps that do not specify an explicit Python version (eg: via a
`.python-version` or `runtime.txt` file), the buildpack uses a curated
default version for the first build of the app.

Then for subsequent builds of the app, the buildpack selects a Python
version based on the version found in the build cache, so that the
version used for the app doesn't change in a breaking way over time as
the buildpack's own default version changes. This feature is referred to
as "version pinning" and/or "sticky versions".

The existing implementation of this feature pinned the version to the
full Python version (eg `3.13.0`), meaning that the app would always use
that exact Python version, even as newer backwards-compatible patch
releases (such as `3.13.1`) become available over time.

Now that we have Python major version -> latest patch version resolution
support (as of #1658) and improved build output around cache
invalidation reasons (as of #1679), we can switch to instead only
pinning to the major Python version (eg `3.13`). This allows apps that
do not specify a Python version to pick up any bug and security fixes
for their major Python version the next time the app is built, whilst
still keeping the compatibility properties of version pinning.

Longer term, the plan is to deprecate/sunset version pinning entirely
(since it leads to confusing UX / lack of parity between multiple apps
deployed from the same codebase at different times, eg review apps), and
the Python CNB has already dropped support for it. However, that will
be a breaking change for the classic buildpack, so out of scope for now.

GUS-W-17384879.
edmorley added a commit that referenced this pull request Dec 6, 2024
@edmorley
Make Python version pinning pin to the major version only
1a318e0 
For apps that do not specify an explicit Python version (eg: via a
`.python-version` or `runtime.txt` file), the buildpack uses a curated
default version for the first build of the app.

Then for subsequent builds of the app, the buildpack selects a Python
version based on the version found in the build cache, so that the
version used for the app doesn't change in a breaking way over time as
the buildpack's own default version changes. This feature is referred to
as "version pinning" and/or "sticky versions".

The existing implementation of this feature pinned the version to the
full Python version (eg `3.13.0`), meaning that the app would always use
that exact Python version, even as newer backwards-compatible patch
releases (such as `3.13.1`) become available over time.

Now that we have Python major version -> latest patch version resolution
support (as of #1658) and improved build output around cache
invalidation reasons (as of #1679), we can switch to instead only
pinning to the major Python version (eg `3.13`). This allows apps that
do not specify a Python version to pick up any bug and security fixes
for their major Python version the next time the app is built, whilst
still keeping the compatibility properties of version pinning.

Longer term, the plan is to deprecate/sunset version pinning entirely
(since it leads to confusing UX / lack of parity between multiple apps
deployed from the same codebase at different times, eg review apps), and
the Python CNB has already dropped support for it. However, that will
be a breaking change for the classic buildpack, so out of scope for now.

GUS-W-17384879.
@edmorley edmorley mentioned this pull request Dec 6, 2024
Merged
edmorley added a commit that referenced this pull request Dec 6, 2024
@edmorley
Make Python version pinning pin to the major version only (#1714)
b77dd09 
For apps that do not specify an explicit Python version (e.g.: via a
`.python-version` or `runtime.txt` file), the buildpack uses a curated
default version for the first build of the app.

Then for subsequent builds of the app, the buildpack selects a Python
version based on the version found in the build cache, so that the
version used for the app doesn't change in a breaking way over time as
the buildpack's own default version changes. This feature is referred to
as "version pinning" and/or "sticky versions".

The existing implementation of this feature pinned the version to the
full Python version (e.g. `3.13.0`), meaning that the app would always use
that exact Python version, even when newer backwards-compatible patch
releases (such as `3.13.1`) became available over time.

Now that we have Python major version -> latest patch version resolution
support (as of #1658) and improved build output around cache
invalidation reasons (as of #1679), we can switch to instead only
pinning to the major Python version (e.g. `3.13`). This allows apps that
do not specify a Python version to pick up any bug and security fixes
for their major Python version the next time the app is built, whilst
still keeping the compatibility properties of version pinning.

Longer term, the plan is to deprecate/sunset version pinning entirely
(since it leads to confusing UX / lack of parity between multiple apps
deployed from the same codebase at different times, e.g. review apps), and
the Python CNB has already dropped support for it. However, that will
be a breaking change for the classic buildpack, so out of scope for now.

GUS-W-17384879.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Malax Malax Malax approved these changes

Assignees

@edmorley edmorley

Labels
bug enhancement
Projects
None yet
Milestone
No milestone
2 participants
@edmorley @Malax