drop Python 3.7 from default bundled lockfiles #21389

cburroughs · 2024-09-10T03:01:27Z

As a consequence, most tests no longer work with 3.7 since -- for example -- the lockfile with pytest does not support 3.7. This downgrades 3.7 to "probably works but not well tested", which is the same state as 3.6. Since 3.7 has been EoL for over a year I think this is reasonable. This opens up the way for Python 3.13 support in lockfiles among other benefits.

As another consequence, lockfiles are regenerated which means we have some new versions coming along for the ride. The pretty lockfile diff does not seem to work with the generation script though --> #21388

Uses of Pants can still use 3.7 by generating their own lockfiles. Deprecation plan annouced at
https://www.pantsbuild.org/blog/2024/08/24/venerable-pythons

Based off work started in #21314

ref #21184, #21103, #20852

As a consequence, most tests no longer work with 3.7 since -- for example -- the lockfile with pytest does not support 3.7. This downgrades 3.7 to "probably works but not well tested", which is the same state as 3.6. Since 3.7 has been EoL for over a year I think this is reasonable. This opens up the way for Python 3.13 support in lockfiles among other benefits. As another consequence, lockfiles are regenerated which means we have some new versions coming along for the ride. The pretty lockfile diff does not seem to work with the generation script though --> pantsbuild#21388 Uses of Pants can still use 3.7 by generating their own lockfiles. Deprecation plan annouced at https://www.pantsbuild.org/blog/2024/08/24/venerable-pythons Based off work started in pantsbuild#21314 ref pantsbuild#21184, pantsbuild#21103, pantsbuild#20852

cburroughs · 2024-09-10T03:05:14Z

src/python/pants/backend/helm/subsystems/post_renderer.py

@@ -48,7 +48,7 @@ class HelmPostRendererSubsystem(PythonToolRequirementsBase):
    ]

    register_interpreter_constraints = True
-    default_interpreter_constraints = ["CPython>=3.7,<3.10"]


The 3.10 here is incongruent but pre-existing, intend to clean it up after this one lands.

cburroughs · 2024-09-10T03:06:56Z

src/python/pants/backend/python/goals/pytest_runner_integration_test.py

 def test_uses_correct_python_version(rule_runner: PythonRuleRunner) -> None:
    rule_runner.write_files(
        {
            f"{PACKAGE}/tests.py": dedent(
                """\
                def test() -> None:
-                  y = (x := 5)
+                  y = {} | {}


We use this pattern in a few places where the tests depended the walrus operator change. Where possible I tried to find an similar 3.8-->3.9 syntax change, but I don't think relying on syntax changes in every CPython release is a scalable pattern.

cburroughs · 2024-09-10T03:10:16Z

src/python/pants/backend/python/lint/black/rules_integration_test.py

@@ -90,7 +90,6 @@ def run_black(
    )
    assert len(partitions) == 1
    partition = partitions[0]
-    assert partition.metadata == InterpreterConstraints([expected_ics])


Okay so partition.metadata is going to have the value from PythonRuleRunner. That is "['>=3.8,<3.10',]" in this change and "['>=3.7,<3.10',]" before. In neither case does that match the value in Black.default_interpreter_constraints[0] (which uses CPython<4) and I am utterly baffled how this ever passed.

This doesn't seem to be a common pattern in other tests.

cburroughs · 2024-09-10T03:14:16Z

src/python/pants/backend/python/lint/flake8/rules_integration_test.py

@@ -120,48 +116,6 @@ def test_multiple_targets(rule_runner: PythonRuleRunner) -> None:
    assert "bad.py:1:1: F401" in result[0].stdout


-@skip_unless_python37_and_python39_present


I was unable to figure out a way to get flake8 to spit out E999 for Python3.8 code that would be valid in 3.9. I remain skeptical of the sustainability of this pattern.

$ python --version Python 3.8.19 $ cat f.py y = {} | {} $ flake8 f.py $ echo $? 0

cburroughs · 2024-09-10T03:15:32Z

src/python/pants/backend/python/subsystems/setup.py

@@ -89,7 +89,7 @@ def interpreter_constraints(self) -> tuple[str, ...]:
            #  We'll probably want to find and modify all those tests to set an explicit IC, but
            #  that will take time.
            if "PYTEST_CURRENT_TEST" in os.environ:
-                return (">=3.7,<4",)
+                return (">=3.8,<4",)


Tangent: There are so many places where we set the interpreter constraints. This one took me a while to find.

Maybe we should have a constant MINIMUM_SUPPORTED_PYTHON_VERSION instead of hard coding?

Yeah reasonable followup. (Or rather we should have fewer such constants.)

src/python/pants/backend/python/typecheck/mypy/subsystem.py

huonw

Thanks for this, I think this is really important to get done and let us move forward.

The mechanics look reasonable, but the scale of the number of tools changed and matching risk makes me a bit concerned, along two dimensions:

being a breaking change for anyone who's actually running these tools using a Python 3.7 interpreter (as we're discussing on slack)
this is coming into the already-huge 2.23 release really late

Given it's so late, it doesn't seem like we're going see a massive benefit of the change in 2.23, and instead just risk forcing the release to slip later... so WDYT about waiting for 2.23 to branch and landing as the first thing for 2.24 instead?

We can revise the blog post to match the new plan; given this is becoming more permissive, that seem fine to me.

(This'd give us the potential chance to land some sort of active soft deprecation in 2.23 too, but I'm happy to let that go.)

krishnan-chandra · 2024-09-10T11:01:43Z

This looks good, but I largely agree with @huonw that this might be a bit too large to land in 2.23. I think if we add it to 2.24 that'll be a clean break and would also let us bundle together the deprecations of Python 2.7 and 3.7 into one release.

cburroughs · 2024-09-10T11:28:06Z

Given the setuptools CVE -- minor as it is -- I'd still lean towards doing this for 2.23 in principle. But in practice this was way more involved than I anticipated and still has a lingering test failure as of this writing. (I'm not going to get to that until much later 'today'.) I'm now most itchy for this to land in main regardless of target release before git conflicts pop up on something so large.

krishnan-chandra · 2024-09-10T13:07:46Z

But in practice this was way more involved than I anticipated

And, indeed, this was the exact same lesson I learned when trying to fix the setuptools CVE 😅

I'm now most itchy for this to land in main regardless of target release before git conflicts pop up on something so large.

Agreed on this point - if we cut the 2.23 release branch soon then this can hopefully get merged ASAP

This reverts commit 11286a1.

This reverts commit d03dbce.

cburroughs · 2024-09-10T20:34:48Z

src/python/pants/backend/python/typecheck/mypy/older_mypy_for_testing.lock

@@ -9,7 +9,7 @@
 //     "CPython<4,>=3.7"
 //   ],
 //   "generated_with_requirements": [
-//     "mypy==0.961"
+//     "mypy[python2]==0.961"


The mypy tests that used this lock only accidentally worked before. They were relying on python3.7 bringing along the same deps as this extra:

"typed-ast<2,>=1.4.0; extra == \"python2\"", "typed-ast<2,>=1.4.0; python_version < \"3.8\"",

cburroughs · 2024-09-10T20:37:08Z

(All tests pass now.)

tdyas · 2024-09-11T00:35:22Z

This looks good, but I largely agree with @huonw that this might be a bit too large to land in 2.23. I think if we add it to 2.24 that'll be a clean break and would also let us bundle together the deprecations of Python 2.7 and 3.7 into one release.

FYI I already landed the Python 2.7 soft deprecation in #21387. I prefer we do not bundle both deprecations together. The Hyper 1.0 upgrade in #21331 is blocked on a newer Ubuntu CI image (which does not have Python 2.7) and so I would like to just drop Python 2.7 from CI once we branch 2.23.x off of main.

huonw

I've merged #21383, so this looks good to merge... but we should move the notes to 2.24.x.md (#21395).

cburroughs added category:user api change backend: Python Python backend-related issues labels Sep 10, 2024

cburroughs self-assigned this Sep 10, 2024

docs: note upgade

8d40e11

cburroughs commented Sep 10, 2024

View reviewed changes

cburroughs added 3 commits September 9, 2024 23:31

test update fix

91a5ec3

mypy workaround

d03dbce

mypy fix redux

11286a1

cburroughs commented Sep 10, 2024

View reviewed changes

src/python/pants/backend/python/typecheck/mypy/subsystem.py Outdated Show resolved Hide resolved

cburroughs marked this pull request as ready for review September 10, 2024 04:01

cburroughs requested review from benjyw, huonw and krishnan-chandra September 10, 2024 04:01

huonw reviewed Sep 10, 2024

View reviewed changes

cburroughs added 3 commits September 10, 2024 15:33

actually correct ninja fix

b8e3f46

Revert "mypy fix redux"

be16de0

This reverts commit 11286a1.

Revert "mypy workaround"

4dbbc81

This reverts commit d03dbce.

cburroughs commented Sep 10, 2024

View reviewed changes

krishnan-chandra approved these changes Sep 10, 2024

View reviewed changes

huonw approved these changes Sep 11, 2024

View reviewed changes

cburroughs added 2 commits September 11, 2024 20:23

Merge remote-tracking branch 'upstream/main' into csb/py38-default-diff

836a38d

move changelog to 2.24

57f689d

cburroughs enabled auto-merge (squash) September 12, 2024 00:29

trigger GitHub actions

a09aff2

cburroughs merged commit a7e6d4f into pantsbuild:main Sep 12, 2024
25 checks passed

This was referenced Sep 12, 2024

Upgrade setuptools to fix CVE #21314

Merged

Upgrade IPython 7.34.0 -> 8.12.3, dropping py3.7 support #21059

Closed

huonw mentioned this pull request Sep 13, 2024

bump default version of pip to support Python 3.13 #21400

Merged

krishnan-chandra mentioned this pull request Nov 17, 2024

Add Python 3.13 complete-platforms for AWS Lambda #21656

Merged

This was referenced Nov 17, 2024

Built-in lockfiles won't work with Python 3.13, due to pip version #20852

Closed

Deprecate using Python 3.7 with default tool lockfiles, preparing for pip 24.1 / Python 3.13 support #21103

Closed

jsirois mentioned this pull request Nov 25, 2024

No supported version of Pip is compatible with the given targets #21690

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drop Python 3.7 from default bundled lockfiles #21389

drop Python 3.7 from default bundled lockfiles #21389

cburroughs commented Sep 10, 2024

cburroughs Sep 10, 2024

cburroughs Sep 10, 2024

cburroughs Sep 10, 2024

cburroughs Sep 10, 2024

cburroughs Sep 10, 2024

grihabor Sep 10, 2024

cburroughs Sep 10, 2024

huonw left a comment

krishnan-chandra commented Sep 10, 2024

cburroughs commented Sep 10, 2024

krishnan-chandra commented Sep 10, 2024

cburroughs Sep 10, 2024

cburroughs commented Sep 10, 2024

tdyas commented Sep 11, 2024

huonw left a comment

		@@ -120,48 +116,6 @@ def test_multiple_targets(rule_runner: PythonRuleRunner) -> None:
		assert "bad.py:1:1: F401" in result[0].stdout


		@skip_unless_python37_and_python39_present

drop Python 3.7 from default bundled lockfiles #21389

drop Python 3.7 from default bundled lockfiles #21389

Conversation

cburroughs commented Sep 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huonw left a comment

Choose a reason for hiding this comment

krishnan-chandra commented Sep 10, 2024

cburroughs commented Sep 10, 2024

krishnan-chandra commented Sep 10, 2024

Choose a reason for hiding this comment

cburroughs commented Sep 10, 2024

tdyas commented Sep 11, 2024

huonw left a comment

Choose a reason for hiding this comment