fix(deps, pyamber): keep setuptools below 82 for fs's pkg_resources import#6412
fix(deps, pyamber): keep setuptools below 82 for fs's pkg_resources import#6412dependabot[bot] wants to merge 5 commits into
Conversation
Bumps [setuptools](https://github.com/pypa/setuptools) from 80.10.2 to 83.0.0. - [Release notes](https://github.com/pypa/setuptools/releases) - [Changelog](https://github.com/pypa/setuptools/blob/main/NEWS.rst) - [Commits](pypa/setuptools@v80.10.2...v83.0.0) --- updated-dependencies: - dependency-name: setuptools dependency-version: 83.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Automated Reviewer SuggestionsBased on the
|
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 385 | 0.235 | 25,642/32,508/32,508 us | 🔴 +19.6% / 🔴 +126.4% |
| ⚪ | bs=100 sw=10 sl=64 | 800 | 0.488 | 125,818/138,357/138,357 us | ⚪ within ±5% / 🔴 +39.6% |
| ⚪ | bs=1000 sw=10 sl=64 | 917 | 0.559 | 1,096,389/1,116,933/1,116,933 us | ⚪ within ±5% / 🔴 +20.5% |
Baseline details
Latest main 2e5c99c from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 385 tuples/sec | 419 tuples/sec | 831.47 tuples/sec | -8.1% | -53.7% |
| bs=10 sw=10 sl=64 | MB/s | 0.235 MB/s | 0.256 MB/s | 0.507 MB/s | -8.2% | -53.7% |
| bs=10 sw=10 sl=64 | p50 | 25,642 us | 21,445 us | 11,844 us | +19.6% | +116.5% |
| bs=10 sw=10 sl=64 | p95 | 32,508 us | 33,473 us | 14,359 us | -2.9% | +126.4% |
| bs=10 sw=10 sl=64 | p99 | 32,508 us | 33,473 us | 18,888 us | -2.9% | +72.1% |
| bs=100 sw=10 sl=64 | throughput | 800 tuples/sec | 819 tuples/sec | 1,089 tuples/sec | -2.3% | -26.5% |
| bs=100 sw=10 sl=64 | MB/s | 0.488 MB/s | 0.5 MB/s | 0.665 MB/s | -2.4% | -26.6% |
| bs=100 sw=10 sl=64 | p50 | 125,818 us | 121,524 us | 92,503 us | +3.5% | +36.0% |
| bs=100 sw=10 sl=64 | p95 | 138,357 us | 145,254 us | 99,121 us | -4.7% | +39.6% |
| bs=100 sw=10 sl=64 | p99 | 138,357 us | 145,254 us | 112,422 us | -4.7% | +23.1% |
| bs=1000 sw=10 sl=64 | throughput | 917 tuples/sec | 921 tuples/sec | 1,117 tuples/sec | -0.4% | -17.9% |
| bs=1000 sw=10 sl=64 | MB/s | 0.559 MB/s | 0.562 MB/s | 0.682 MB/s | -0.5% | -18.0% |
| bs=1000 sw=10 sl=64 | p50 | 1,096,389 us | 1,095,925 us | 910,046 us | +0.0% | +20.5% |
| bs=1000 sw=10 sl=64 | p95 | 1,116,933 us | 1,130,559 us | 950,716 us | -1.2% | +17.5% |
| bs=1000 sw=10 sl=64 | p99 | 1,116,933 us | 1,130,559 us | 985,832 us | -1.2% | +13.3% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,518.99,200,128000,385,0.235,25641.88,32508.05,32508.05
1,100,10,64,20,2501.10,2000,1280000,800,0.488,125817.77,138356.60,138356.60
2,1000,10,64,20,21821.92,20000,12800000,917,0.559,1096388.82,1116932.52,1116932.52There was a problem hiding this comment.
Pull request overview
This PR updates the Amber (pyamber) Python dependency pin for setuptools to the latest specified version, keeping the Amber Python environment current and consistent with the project’s pinned-dependency approach.
Changes:
- Bump
setuptoolsfrom80.10.2to83.0.0inamber/requirements.txt.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6412 +/- ##
============================================
+ Coverage 69.90% 70.19% +0.29%
- Complexity 3380 3385 +5
============================================
Files 1142 1142
Lines 44844 44844
Branches 4949 4949
============================================
+ Hits 31346 31479 +133
+ Misses 11849 11725 -124
+ Partials 1649 1640 -9
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| # Not imported directly: `fs` needs pkg_resources at import time (#4199). | ||
| setuptools==80.10.2 | ||
| setuptools==83.0.0 |
There was a problem hiding this comment.
why do we need to keep setuptools again?
There was a problem hiding this comment.
We do not import setuptools directly in Texera. It is kept because PyAmber imports PyFilesystem2 (fs) in amber/src/main/python/core/architecture/managers/executor_manager.py, and fs==2.4.16 imports pkg_resources at module import time. pkg_resources is provided by setuptools, so the requirement keeps that indirect import available.
There was a problem hiding this comment.
can we just remove setuptools? if fs depends on pkg_resources, then it should declare pkg_resources as its dependency?
There was a problem hiding this comment.
Unfortunately we can't — pkg_resources isn't a real PyPI package, so nothing can declare it as a dependency. It's a module that ships inside setuptools, and only below 82: setuptools 82.0.0 removed it (changelog). fs already declares setuptools, just with no version bound, and 2.4.16 is the project's final release, so that metadata won't be fixed upstream. The setuptools changelog itself recommends the fix we're using: "Projects and environments relying on pkg_resources … should depend on older versions of setuptools."
Removing the line would still work today, but only by accident: torch==2.12.1+cpu in operator-requirements.txt declares setuptools<82, which is what keeps a pkg_resources-bearing setuptools installed. Once torch relaxes that cap, we'd hit the same ModuleNotFoundError: No module named 'pkg_resources' from #4199 again on some unrelated PR.
That torch cap is also why the original bump to 83.0.0 here looked green: CI installs the two requirement files in separate passes, and the second pass silently downgraded setuptools before pytest ran (run log: Successfully uninstalled setuptools-83.0.0 → Successfully installed … setuptools-81.0.0), so 83.0.0 — which has no pkg_resources — was never actually tested. I verified in a clean venv that setuptools==83.0.0 + fs==2.4.16 fails at import fs.
I've reworked this PR to setuptools<82 instead of the exact pin — it encodes the actual requirement, currently resolves to 81.0.0 (the version CI really ran under), and can't conflict with torch's cap.
This reverts commit 6d768f4.
Originally a Dependabot bump of the
setuptoolspin from80.10.2to83.0.0; reworked to a<82bound after review discussion.What changes were proposed in this PR?
Setuptools 82.0.0 removed
pkg_resources(changelog), whichfs==2.4.16imports at module-import time (fs/__init__.pyline 4) — the exact breakage #4199 pinned against. In a clean venv,setuptools==83.0.0+fs==2.4.16fails:The green CI on the original bump was a false pass:
torch==2.12.1+cpuinamber/operator-requirements.txtdeclaressetuptools<82, and the workflow installs the two requirement files in two separate resolver passes, so the second pass silently downgraded setuptools before pytest ran (run log:Successfully uninstalled setuptools-83.0.0→Successfully installed ... setuptools-81.0.0). A single-pass install of both files (the dev-guide flow) with==83.0.0is unresolvable against torch's<82.Why not remove the line instead:
pkg_resourcesis not an installable PyPI package, sofscannot declare it directly — it declaressetuptools, but with no version bound, and 2.4.16 is the project's final release. Unpinned, the environment would only keep working by riding torch's incidental<82cap. A consumer-side bound is the remedy the setuptools changelog itself recommends ("Projects and environments relying onpkg_resources... should depend on older versions ofsetuptools").amber/requirements.txtsetuptools==80.10.2(Dependabot proposed==83.0.0)setuptools<82<82currently resolves to 81.0.0 — the version CI actually ran under on this PR — and encodes the real requirement instead of an exact version, so future Dependabot bumps past the removal can't land silently.Any related issues, documentation, discussions?
Follow-up to #4199; the #6110 pin audit kept this pin for the same reason.
How was this PR tested?
pip install "setuptools<82" fs==2.4.16resolves setuptools 81.0.0 andimport fssucceeds; the same venv withsetuptools==83.0.0fails withModuleNotFoundError: No module named 'pkg_resources'.test_executor_manager.py→executor_manager.py→import fs).Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (claude-fable-5)