Update pants.yaml - #113
Conversation
benjyw
left a comment
benjyw
left a comment
There was a problem hiding this comment.
Aaargh, this is subtle, but:
pants-python-versionensures that a python interpreter suitable for running Pants on exists on the system. I guess the default was 3.9, so we got lucky, but your fix is absolutely correct.- Generally, a CI job might want to use
setup-pythonseparately to ensure that a python interpreter it needs for testing its own code is available. And since this repo is intended to be an example, it's probably a good idea for it to demonstrate that.
However, what we should add is a lot of comments explaining this...
|
Actually now that I think of it, it's not that 3.9 was the default and we got lucky, it's that we installed 3.9 anyway, because of the explicit use of setup-python... |
|
PS thanks for these fixes! One suggestion I have is, in the future, to give more descriptive PR titles. E.g., " Set the pants python version correctly in CI" rather than "Update pants.yaml", since the latter doesn't tell me anything about the reason for the update... |
Yeah... I'd like a demonstration about what you mean here.
I don't get what you mean here. Isn't exactly that? The default Python version the action uses is 3.9. The See this pipeline: https://github.com/Kludex/uvicorn-extensions/actions/runs/3456291869/jobs/5768916908 |
Ah, that might also be the case. But my point was that we run setup-python twice, and at least one of them with the right python version, just not the semantically correct one... Really there could be two matrixes here, one for the pants python version and the other for the code python version. It's only this repo that has this complexity, because this yaml file has two roles: 1) the actual CI for this repo and 2) a demonstration of how to set up GHA in a pants repo... |
[Rolls dice on scie-jump] |
3 participants
This is not obvious... I just noticed because I checked the version on the pipeline. Why do we even run "actions/setup-python" on this pipeline?