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.

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

Support flexible patch version syntax to allow for automatic Python updates #913

Open
abitrolly opened this issue Dec 26, 2019 · 19 comments

Comments

@abitrolly
Copy link

Pinning Python runtime on Heroku prevents it from getting security updates.

remote: -----> Python app detected
remote:  !     Python has released a security update! Please consider upgrading to python-3.6.10
remote:        Learn More: https://devcenter.heroku.com/articles/python-runtimes

The secure way is to allow any newer patch version to be installed. Proposed syntax.

python-3.6.x
@CaseyFaist
Copy link
Contributor

Heckin yes. Not the next thing on my list, but on the list.

@abitrolly
Copy link
Author

I could probably help you with a patch given the exact place where Python selection happens.

@edmorley
Copy link
Member

@abitrolly Hi! Thank you for filing this. I 100% agree this is something we should support. Whilst some projects will always want to specify an exact Python version to ensure determinism/allow control of when the upgrade occurs, there are likely many more that would like to not have to think about patch updates and have them be performed automatically.

Some thoughts that came to mind:

  1. What syntax(s) should we support? (eg python-3.6 vs python-3.6.x vs python-3.6.*, or even variants without the python- prefix)
  2. Longer term do we see runtime.txt being superceded by something else (eg Python Version via .python-version #932)? If so, does that affect whether we add support for this feature to runtime.txt (vs adding only to the new thing), or should we just go ahead with this anyway?

To try and answer these, I took a look at what other tools/services use currently:

Pyenv

Python Poetry

Pipenv

Travis CI Python

Circle CI Python

Azure Pipelines

Netlify

Docker Hub Official Python images


Given the above, I think it's best if we:

  • support a python-X.Y style syntax (not python-X.Y.* or python-X.Y.x)
  • consider also supporting the version without the python- prefix, given most other platforms don't use it, plus there have been support tickets about builds failing due to use of the non-prefixed version form (we should just support it out of the box)
  • support this in runtime.txt since whilst we will likely want to support .python-version in the future, it doesn't support flexible versions anyway so likely couldn't be used by people wanting that feature

@abitrolly
Copy link
Author

What syntax(s) should we support? (eg python-3.6 vs python-3.6.x vs python-3.6.*, or even variants without the python- prefix)

I see not problem to support them all, but I would leave python- prefix intact, because there are also alternative interpreters like pypy.

  1. python-3.6
  2. python-3.6.*
  3. python-3.6.x

@abitrolly
Copy link
Author

On the second thought a separate component that can parse the requirements from all these platforms and autodetect best Python version given priority list (also explaining its process and selection logic) would be beneficial not only for Heroku.

@edmorley
Copy link
Member

Tracked internally at W-7671453.

@ipmb
Copy link

ipmb commented Mar 11, 2021

Longer term do we see runtime.txt being superceded by something else (eg #932)?

Yes, please! pyproject.toml is the blessed place to do stuff like this. I think #932 (comment) is the right path. Heroku can help drive adoption here. Moving towards it as the singular configuration place a'la package.json for the Node buildpacks would be great.

@edmorley
Copy link
Member

edmorley commented Feb 7, 2022

So I've been revisiting this topic (ways in which the Python version could/should be specified for Heroku apps in the future) as part of the CNB for Python work, and unfortunately all of the options have drawbacks. As such, I'm still mulling over the best way forwards for the moment.

Edit: Since this was posted, pyenv has added support for X.Y style version ranges to .python_version, which means option B below (.python_version) is now looking like a strong contender for a non-package-manager-specific option.

The options that we may want to consider (we'd likely need to support several from this list):

A) Use pyproject.toml's project.requires-python

  • Pros:
    • Is not Heroku-specific.
    • Uses pyproject.toml, which is the ~recommended direction for Python tooling moving forwards.
    • Uses an existing pyproject.toml field rather than adding another that users have to learn.
    • Supports both version ranges (for flexible patch updates) and exact version pinning.
  • Cons:
    • The requires-python field is typically intended to convey the range of Python versions supported by a library. Using it to specify a more exact version/versions of Python to be used for application deployment is a bit of a stretch of the definition of the field.
    • For libraries, it's very much considered bad practice to constrain requires-python too much (some say not to set an upper bound at all, eg https://iscinumpy.dev/post/bound-version-constraints/). Encouraging usage of the requires-python field in this way for applications may lead to users also doing the same for libraries. In addition, what if a repo is both an app and a library? (I guess we could support this by having something else such as .python-version override this field, so they could have both a wide range in python-requires but still deterministic version for their app deploy?)
    • The field supports any valid PEP440 version specifier syntax. We would either have to only support a subset of the syntax (most likely), or else have a full PEP440 version specifiers parser (for which there currently isn't a Rust crate; the crate pep440 only supports the versions themselves, not the full version specifiers including the operator).
    • The PEP440 version specifier syntax is IMO slightly counter-intuitive when it comes to version ranges. eg ~=3.9 and ~=3.9.0 are not equivalent (the former means you'll be given 3.10+, which is not ideal for ~deterministic application deploys), which may confuse users who are more used to semver's ~3.9 and ~3.9.0 being equivalent (as used by Poetry).
  • Reference:

B) Add a new pyproject.toml table/property

  • Pros:
    • It could be made to not be Heroku-specific (depending on naming).
    • Uses pyproject.toml, which is the ~recommended direction for Python tooling moving forwards.
    • Could be made to support both version ranges (for flexible patch updates) and exact version pinning.
    • A much simpler syntax could be used (eg just support X.Y.Z or X.Y) to avoid the pep440/semver parsing hassle and potential for user confusion.
    • This table could hold other Python buildpack related config options too, meaning everything is in one place.
  • Cons:
    • Will require users to learn/use a new concept.
    • Won't be supported by existing tooling (eg pyenv, Dependabot) for some time (if ever; unless we get ecosystem buy-in).
    • Issues with namespaces, since:
      • PEP518 says namespaces inside the [tools] table should only be used if the equivalent package name on PyPI is owned, which makes it awkward for non-vendor specific namespace choices, since people aren't supposed to share namespaces. We own heroku on PyPI already, buildpack seems to be blocked as a package name (so presumably safe to use; though would be nice to not make this deployment option buildpack-specific?), deployment seems to be owned by someone else.
      • PEP518 says any other table names outside of [tools] are reserved and could be defined by future PEPs (so presumably we should avoid using such tables, unless we propose an addition via a PEP ourselves).
  • Reference:

C) Use the existing .python-version

D) For Poetry users, use the existing tool.poetry.dependencies.python in pyproject.toml

E) Use the existing runtime.txt

  • Pros:
    • Existing Heroku users will understand the concept and require no changes to their apps.
    • Could be made to support both version ranges (for flexible patch updates) and exact version pinning.
    • Simpler syntax than full pep440/semver.
  • Cons:
    • Requires users new to Heroku to learn/use a new concept.
    • Whilst the naming of the file isn't Heroku-specific, it's still a concept that Heroku introduced and hasn't really been adopted by much else in the ecosystem (other than Netlify, though they borrowed the filename but changed the syntax to something that's not compatible with the Heroku syntax).
    • Doesn't make use of pyproject.toml.
    • Doesn't work with local tooling, eg pyenv / Poetry.

F) Use the CNB project.toml file

  • Pros:
    • It could be made to not be Heroku-specific (depending on naming).
    • Could be made to support both version ranges (for flexible patch updates) and exact version pinning.
    • A much simpler syntax could be used (eg just support X.Y.Z or X.Y) to avoid the pep440/semver parsing hassle and potential for user confusion.
    • If it ends up being common that CNB based apps have a project.toml (to specify other metadata / custom buildpacks / build env vars etc), at least all buildpack related config would be in one place.
  • Cons:
    • Is buildpack-specific.
    • Will require users to learn/use a new concept.
    • Doesn't make use of pyproject.toml.
    • Doesn't work with local tooling, eg pyenv / Poetry.
  • Reference:

G) Use an environment variable

  • Pros:
    • It could be made to not be Heroku-specific (depending on naming).
    • Existing concept/easy to understand.
    • Could be made to support both version ranges (for flexible patch updates) and exact version pinning.
    • A much simpler syntax could be used (eg just support X.Y.Z or X.Y) to avoid the pep440/semver parsing hassle and potential for user confusion.
  • Cons:
    • Goes against infrastructure as code (so I would say is an anti-pattern for this use-case).
    • Will mean drift between environments (eg: Review Apps vs CI vs production).

@zyv
Copy link
Contributor

zyv commented Feb 7, 2022

We are, of course, for D - Poetry :-)

@edmorley
Copy link
Member

edmorley commented Feb 7, 2022

So I think it's inevitable that we support package-manager specific constructs (eg Poetry's), the decision here is more:

  • what else do we support?
  • which should the order of precedence be, if multiple ways of specifying the version are found?

@abitrolly
Copy link
Author

I don't think that one chosen way will work. It should be a priority list. The promise of buildpack is that it can build you project automatically. Otherwise it is not really different from going Docker/Kubernetes/OpenShift way.

The requires-python field is typically intended to convey the range of Python versions supported by a library. Using it to specify a more exact version/versions of Python to be used for application deployment is a bit of a stretch of the definition of the field.

If there is a priority list, then I don't see any problem to use latest available version that fits requires-python. If somebody needs an exact version, it could be done with higher priority override. The version selector tool just needs to be overly transparent which version it chooses and why.

For the spec it would help dividing A-G into priority groups.

@edmorley
Copy link
Member

edmorley commented Feb 7, 2022

@abitrolly Yes, that's exactly the intention - to support several options and to have an ordering of precedence.

The point (as discussed above) is:

  • what options should we support?
  • which options should we recommend over the others? (to at least guide those unsure)
  • what should the order of precedence be? (as you say, which can help with the narrow vs wide version range issue)

The A-G list is a starting point to aid that decision, not mutually exclusive options.

@ipmb
Copy link

ipmb commented Feb 7, 2022

This is a good list. My first choice would be to support B only. Having a cascading list of places to try is clever/convenient, but I think makes things more confusing for users. The way Node/npm/yarn is handled with engines in package.json is really nice. This route feels more "Pythonic" to me. From the Zen of Python:

There should be one-- and preferably only one --obvious way to do it.

I can see runtime.txt continuing to be supported for legacy apps, but with a deprecation warning when found and a link with documentation to move to the new way.

Another benefit of B is that it can be used to not only specify the Python version, but also, pip, setuptools, Poetry, etc. (same as the Node CNB with yarn and npm).


To address the cons of this approach:

Will require users to learn/use a new concept.

pyproject.toml is becoming more and more common. For users that don't use it, providing a code snippet doesn't seem onerous.

Won't be supported by existing tooling (eg pyenv, Dependabot) for some time (if ever; unless we get ecosystem buy-in).

This is a compelling argument for using A, but the cons mentioned there outweigh the benefits. Being able to specify all tooling (Python, pip, etc) versions in one place is worth the trade-off.

Unclear what naming might be best

Naming is always hard. That shouldn't be a deterrent for the best solution 😄

@edmorley
Copy link
Member

edmorley commented Feb 7, 2022

Naming is always hard. That shouldn't be a deterrent for the best solution

I'll update the list above to make this clearer - but my main concern about the naming if adding a [tools] table, was that we would either have to:

  • make it vendor-specific (eg tools.heroku), in which case it's basically moved runtime.txt somewhere else,
  • use something non-vendor-specific (eg buildpack), in which case we're potentially violating PEP518 on ownership (but maybe that's acceptable).

ie: The way PEP518 is written, it doesn't allow tools from different vendors to share the same namespace. And avoiding yet another Heroku-ism was something I was hoping to avoid this time around :-)

However since trying to publish a package called buildpack on PyPI gives the "name not allowed" error (implying it's on the disallowed names list), perhaps it's safe to just use that even though we don't own the package. Though that name is still buildpack-specific, when ideally if we're trying to drive a new standard, it would be generic to any deployment type.

@ipmb
Copy link

ipmb commented Feb 7, 2022

Following PEP518 seems like the right thing to do. I like not having it tied directly to Heroku or Buildpacks so it could possibly become a standard. engines is taken, but how about python-engines? or deployment?

@edmorley
Copy link
Member

edmorley commented Oct 30, 2022

In the time since the analysis in #913 (comment) occurred:

As such, this resolves a number of the issues with the .python-version option in #913 (comment), making it a strong contender for the package-manager-independent replacement for runtime.txt, given that the issues with pyproject.toml and version-pinning are still unresolved.

(There will of course still need to be package manager specific file support, such as parsing poetry.lock).

@abitrolly
Copy link
Author

@edmorley I appreciate the amount of thought you put in this issue, but I still don't see any actionable items since I opened it. It is about time for me to stop licking this cookie and go find a job.

If, however, Heroku has a process to hire me as a contractor (which is doesn't), I could spent a few week to collaborate on a use case test book. Which, QA-style, will allow anybody to quickly go through the cases to validate what we are not breaking anything by switching from runtime.txt to .python-version.

If you've already decided it will be more beneficial to deprecate runtime.txt in favor of .python-version (which can be deprecated too as time passes), let me know the number of the issue so I can close this one.

@edmorley edmorley changed the title Allow flexible patch versions in runtime.txt Support flexible patch version syntax to allow for automatic Python updates Oct 31, 2022
@edmorley
Copy link
Member

@abitrolly Hi! This issue is not blocked on you, and I'm not expecting you to work on it. Whilst this issue was initially filed about runtime.txt, I've been treating it as a general "support flexible versions" issue (that is considering all potential solutions) - I've morphed the title to reflect this.

The reason for my most recent comment was that I received an email notification for the pyenv issue being closed as implemented, and I wanted all context about Python versions to be contained here ready for when I revisit the topic. The discussion/comments here will feed into the design of the Python Cloud Native Buildpack, which I'll (finally) be able to have time for again soon.

@abitrolly
Copy link
Author

There are already a couple of buildpacks supporting Python, some supporting runtime.txt https://docs.cloudfoundry.org/buildpacks/python/index.html and some are not https://paketo.io/docs/howto/python/ for specifying Python version.

Buildpacks based systems can use runtime.txt not only for choosing the Python version, but also for detecting which buildpack to apply. So the topic becomes broader, worthy writing an RFC https://github.com/buildpacks/rfcs/tree/main/text

Maybe it is not the role of the buildpack to choose Python version at all, because package authors know better, and they most likely don't be too pleased to have yet another place to configure Python.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants