-
-
Notifications
You must be signed in to change notification settings - Fork 638
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
update Pants' own Python to 3.11 #21528
Conversation
ca4fae3
to
d4fa8dd
Compare
1b4c9c4
to
64b26f0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should add Thanks to:
- @kaos for doing the hardest part
- @thejcannon for the earlier draft
- @benjyw for getting the needed versions of Python on to the Mac builders
@@ -18,6 +18,8 @@ extend-ignore: | |||
NIC102, | |||
# Unnecessary dict call - rewrite as a literal | |||
C408, | |||
# Temporarily exclude during Python upgrade |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self post merge.
@@ -63,7 +63,7 @@ class Target: | |||
target_type: str = strawberry.field( | |||
description="The target type, such as `python_sources` or `pex_binary` etc." | |||
) | |||
fields: JSONScalar = strawberry.field( | |||
fields: JSONScalar = strawberry.field( # type: ignore[valid-type] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this clearly 'should work' given strawberry-graphql/strawberry#1205
but there are various similar looking errors at strawberry-graphql/strawberry#3579 Given the low current usage of the explorer I went with the ignore.
@@ -8,7 +8,7 @@ | |||
import os | |||
import re | |||
from dataclasses import dataclass, field | |||
from enum import Enum | |||
from enum import Enum, ReprEnum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -22,6 +22,8 @@ | |||
PY_37 = "3.7" | |||
PY_38 = "3.8" | |||
PY_39 = "3.9" | |||
PY_310 = "3.10" | |||
PY_311 = "3.11" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added for completeness. I'm suspicious that some of these existing annotations around Python 3.9 semantically should be "the version of Python used by Pants" but have not pulled harder on that thread.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome, thank you! The code looks good, but want to nail down the deprecation/phasing details.
MINIMUM_SCIE_PANTS_VERSION = Version("0.10.0") | ||
# First version with Python 3.11 support: | ||
# https://github.com/pantsbuild/scie-pants/releases/tag/v0.12.0 | ||
MINIMUM_SCIE_PANTS_VERSION = Version("0.12.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we done any deprecations in advance? I'm thinking it's probably best if someone who's, say, got scie-pants 0.11.0 installed doesn't first learn about this by upgrading to 2.25.x and finding it fails hard/immediately.
Maybe we could sneak a warning about <0.12.0 into 2.24? (Either before branching or cherrypicked soon after)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We internally use an explicit version of scie-pants
in ci/devcontainers, but I suspect that most people just use the bootstrapping script and end up with the latest and only update if forced to by this check or some issue they run into. 0.12.0
has been out since May so I think anyone is likely to have setup 0.11
and Pant 2.2{4,5}
for the first time unless they were following some intentional internal procedure.
My fear we "per-deprectaiton" might be too clever by half and if we end up needing to put out a 0.12.x
for some bug in this thing we are doing for the first time (Python upgrade) are are now forcing multiple upgrades.
@sureshjoshi had a PoC of using the new Pex-makes-the-scies support so that the Pants repo could control the python-build-standalone version and we would have less of this dance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit groggy on this at the moment (and am not home to check), but one thing about deprecations... Which Python interpreter do in-repo plugins use? I'm struggling to recall if Pants runs them using it's own Python, or whether the user needs to have a system Python targeting 3.9.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which Python interpreter do in-repo plugins use?
I believe the answer is... both!
My understanding is that it "runs" them with with the Pants-Python, but if you have them setup as targets with BUILD files the linting/testing/etc happens with a 'regular' Python. (Thus the callout in the release notes plugin section.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My fear we "per-deprectaiton" might be too clever by half and if we end up needing to put out a 0.12.x for some bug in this thing we are doing for the first time (Python upgrade) are are now forcing multiple upgrades.
Yeah, that's a reasonable concern, but it seems unfortunate to give up on providing a user prompt just because we might get it wrong.
Thinking out loud, if we land this and put a pre-deprecation into 2.24 soon, we've got until 2.24.0 stable is released (i.e. at least a few weeks) to nail down any additional scie-pants changes and cherry-pick version bumps back to 2.24.x.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having a heads up for plugin authors in the 2.23 blog post seams a reasonable thing we can do at the least.
I'm not opposed to some sort of warn-but-not-too-often in 2.24, I'm just not sure exactly what that looks like. Best effort to do something by 2.24.0
as we learn more in main
sounds okay to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed #21603
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for doing this!
MINIMUM_SCIE_PANTS_VERSION = Version("0.10.0") | ||
# First version with Python 3.11 support: | ||
# https://github.com/pantsbuild/scie-pants/releases/tag/v0.12.0 | ||
MINIMUM_SCIE_PANTS_VERSION = Version("0.12.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed #21603
dddf00b
to
988ee28
Compare
While trying to land #21528 to was observed that the wheel building jobs were failing during git checkout with wonderful errors like `/__e/node20/bin/node: /lib64/libm.so.6: version `GLIBC_2.27' not found (required by /__e/node20/bin/node)`. This appears to be the same symptoms as actions/checkout#1809 which pointed the the deprecation notice at <https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/>. If this all sounds familiar that is because we *mostly* cleaned this up in #21133, but kept a fewer uses of the older actions for `manylinux2014` compatibility. However, from the notice: "To opt out of this and continue using Node16 while it is still available in the runner, you can choose to set ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION=true as an ‘env’ in their workflow or as an environment variable on your runner machine. This will *only work until we upgrade the runner removing Node16 later in the spring*. (emphasis added)" From the wheel job failures during #21528 and my attempts to fiddle with all of `ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION`, `ACTIONS_RUNNER_FORCED_INTERNAL_NODE_VERSION`, `ACTIONS_RUNNER_FORCE_ACTIONS_NODE_VERSION` I think the removal time promised for the Spring has finally come, but I'm not that familiar with GitHub Actions and could be missing something. As a consequence the version of both `actions/upload-artifacts` and `actions/checkout` is bumped to `v4`. To make this testable now and in the future, wheels are now build on ci config changes. Why is bumping to `manylinux_2_28` (the next oldest manylinux) not a big deal? The "2.28" refers to glibc [2.28](https://sourceware.org/legacy-ml/libc-alpha/2018-08/msg00003.html) in 2018. That is older than Debian stable and if you need to use software that old you are presumably paying someone for an enterprise distribution. NOTE: I think we should bump the manylinux version in `main` regardless, but if my read if the situation is correct we may need to backport this any active release line. Without this or a more complex change we would also be unable to release after December 5th per #21616. ref #21195 #21616
Building on the work in pantsbuild/scie-pants#351 this brings the version of Python used by Pants from 3.9 to 3.11. Why 3.11 and not 3.12 or 3.13? Because that is what we already released on the scie-pants side and two release forward is still a big benefit. NOTE: I'd like to hold off on stomping out all deprecation warnings until one `dev` release since the release process part is what I'm most nervous about.
988ee28
to
3e8fae6
Compare
Building on the work in pantsbuild/scie-pants#351 this brings the version of Python used by Pants from 3.9 to 3.11. Why 3.11 and not 3.12 or 3.13? Because that is what we already released on the scie-pants side and two release forward is still a big benefit.
NOTE: I'd like to hold off on stomping out all deprecation warnings and anything else that "depends" on 3.11until one
dev
release since the release process part is what I'm most nervous about.