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

Refactor Python version handling #1658

Merged
merged 1 commit into from
Oct 9, 2024
Merged

Refactor Python version handling #1658

merged 1 commit into from
Oct 9, 2024

Conversation

edmorley
Copy link
Member

@edmorley edmorley commented Oct 8, 2024 •
edited
Loading

The previous implementation of Python version detection/resolution revolved heavily around the runtime.txt file, even though the version could originate from other sources. For example, a fake runtime.txt would be written out into the build directory containing the desired Python version, even if the version originated from Pipfile.lock.

This meant all later Python version handling in the buildpack would see a runtime.txt file, along with versions specified in the syntax of that file (e.g. python-N.N.N strings), even though that wasn't the format in which the user had specified the version.

Now, the buildpack explicitly tracks the requested version and its origin (rather than using the runtime.txt file as an API), along with the resolved Python major and full versions (which makes later Python version conditionals less fragile).

In addition, the Python version specifiers are validated upfront at the point of parsing the relevant data source, so that clearer error messages can be shown.

Lastly, the Python version resolution (the mapping of major Python versions to the latest patch release) has been decoupled from the Pipenv version implementation and made more robust, so it can also be used by the upcoming .python-version file support.

Prep for #932 / #913.

GUS-W-16821309.
GUS-W-7924371.
GUS-W-8104668.

@edmorley edmorley self-assigned this Oct 8, 2024
@edmorley edmorley force-pushed the version-refactor branch 2 times, most recently from 617a048 to a615afd Compare October 8, 2024 16:06
@edmorley edmorley marked this pull request as ready for review October 8, 2024 16:12
@edmorley edmorley requested a review from a team as a code owner October 8, 2024 16:12
schneems
schneems approved these changes Oct 8, 2024
View reviewed changes
Copy link
Contributor

@schneems schneems left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read through the warnings/errors and general logic. Read through the test changes. No changes requested.

@edmorley
Refactor Python version handling
5ca16f1 
The previous implementation of Python version detection/resolution
revolved heavily around the `runtime.txt` file, even though the version
could originate from other sources. For example, a fake `runtime.txt`
would be written out into the build directory containing the desired
Python version, even if the version originated from `Pipfile.lock`.

This meant all later Python version handling in the buildpack would see
a `runtime.txt` file, along with versions specified in the syntax of
that file (e.g. `python-N.N.N` strings), even though that wasn't the
format in which the user had specified the version.

Now, the buildpack explicitly tracks the requested version and its
origin (rather than using the `runtime.txt` file as an API), along
with the resolved Python major and full versions (which makes later
Python version conditionals less fragile).

In addition, the Python version specifiers are validated upfront at
the point of parsing the relevant data source, so that clearer error
messages can be shown.

Lastly, the Python version resolution (the mapping of major Python
versions to the latest patch release) has been decoupled from the
Pipenv version implementation and made more robust, so it can also
be used by the upcoming `.python-version` file support.

GUS-W-16821309.
GUS-W-7924371.
@edmorley edmorley enabled auto-merge (squash) October 9, 2024 08:37
@edmorley edmorley disabled auto-merge October 9, 2024 08:38
@edmorley edmorley enabled auto-merge (squash) October 9, 2024 08:38
@edmorley edmorley merged commit 42499ff into main Oct 9, 2024
7 checks passed
@edmorley edmorley deleted the version-refactor branch October 9, 2024 08:40
@heroku-linguist heroku-linguist bot mentioned this pull request Oct 9, 2024
Merged
@edmorley edmorley mentioned this pull request Oct 14, 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

@schneems schneems schneems approved these changes

Assignees

@edmorley edmorley

Labels
bug enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@edmorley @schneems