heroku / heroku-buildpack-python

Heroku's buildpack for Python applications.
https://www.heroku.com/python
MIT License
974 stars 1.84k forks source link

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

Open abitrolly opened 4 years ago

abitrolly commented 4 years ago

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 commented 4 years ago

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

abitrolly commented 4 years ago

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

edmorley commented 4 years ago

@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 #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:

abitrolly commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Tracked internally at W-7671453.

ipmb commented 3 years ago

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 https://github.com/heroku/heroku-buildpack-python/issues/932#issuecomment-697216718 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 commented 2 years ago

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

B) Add a new pyproject.toml table/property

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

F) Use the CNB project.toml file

G) Use an environment variable

zyv commented 2 years ago

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

edmorley commented 2 years ago

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

abitrolly commented 2 years ago

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 commented 2 years ago

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

The point (as discussed above) is:

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

ipmb commented 2 years ago

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 commented 2 years ago

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:

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 commented 2 years ago

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 commented 1 year ago

In the time since the analysis in https://github.com/heroku/heroku-buildpack-python/issues/913#issuecomment-696655767 occurred:

As such, this resolves a number of the issues with the .python-version option in https://github.com/heroku/heroku-buildpack-python/issues/913#issuecomment-1031468659, 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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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.