ci(py-315): prune cp315 wheels from all public publication paths (PROF-15857) - #19880
ci(py-315): prune cp315 wheels from all public publication paths (PROF-15857)#19880vlad-scherbich wants to merge 5 commits into
Conversation
Circular import analysis
|
|
BenchmarksBenchmark execution time: 2026-08-28 18:55:35 Comparing candidate commit 1a43844 in PR branch Found 0 performance improvements and 7 performance regressions! Performance is the same for 556 metrics, 9 unstable metrics, 3 known flaky benchmarks, 13 flaky benchmarks without significant changes.
|
Codeowners resolved asResolved from the full PR diff against |
Dependency direction analysis
|
There was a problem hiding this comment.
Pull request overview
Adjusts the GitLab release pipeline so release_pypi_prod no longer uploads cp315 wheels to PyPI, preventing publication of wheels for an unsupported (and potentially ABI-incompatible) Python 3.15 target while leaving internal distribution paths unchanged.
Changes:
- Builds an explicit upload list from
pywheels/ddtrace-*and filters out any*cp315*artifacts before bothtwine check --strictandtwine upload. - Fails the release job early with a clear error if filtering results in an empty upload set, and logs the exact distributions that will be published.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bea839769e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
brettlangdon
left a comment
There was a problem hiding this comment.
any reason to not update ddtrace package to delete *cp315* wheels before saving the artifacts?
would be much simpler implementation, no?
@brettlangdon Agreed, implemented. Went a bit wider than Upload serverless` produced all 40,814 cp315 serverless wheels on dd-trace-py-builds — which cloud-inventory registers as a public, customer-facing bucket, so dropping cp315 from the default index is a nice improve, imo. So: one prune script called from all three places (twine upload, aws s3 cp, |
PR #19880 filtered cp315 out of the PyPI upload only, on the stated premise that the other destination was "internal S3 and the private prerelease index". That is wrong: s3://dd-trace-py-builds is anonymously readable and listable and is registered in DataDog/cloud-inventory as a "Public bucket to host dev builds of dd-trace-py from GitLab to share with customers". The pipeline republished cp315 wheels to main/ on 2026-08-27, so the leak was live, not historical. Three mechanisms publish a ddtrace wheel outside the pipeline: twine to PyPI, aws s3 cp to that bucket, and adms to pypi-private-prereleases. All three now call .gitlab/scripts/prune-unsupported-wheels.sh first, which holds the withheld ABI tags in one place. Putting it inside upload-wheels-to-s3.sh covers every S3 caller and every index suffix, including "upload serverless", which no filter reached before. The PyPI-side shell in release.yml reverts to the pre-filter form plus one prune call, per review feedback: with cp315 gone from the "ddtrace package" artifact, twine check no longer needs a hand-built file list. tests/internal/test_unsupported_wheel_pruning.py replaces the PyPI-specific test. It pins the prune behaviour and asserts that every file under .gitlab/ running a publish command also calls the prune script, so a new upload path cannot silently reintroduce cp315.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
tests/internal/test_unsupported_wheel_pruning.py:223
- Like the missing-directory test above, this test assumes
bashexists but doesn’t guard againstshutil.which("bash")returning None, which would crash the test suite with aTypeErrorrather than producing a useful skip.
shell = shutil.which("bash")
|
|
||
| def test_a_missing_directory_is_a_hard_error(prune, tmp_path): | ||
| """A typo in a call site must fail the job, not skip the prune.""" | ||
| shell = shutil.which("bash") |
3ca9aef to
f668305
Compare
PR #19880 filtered cp315 out of the PyPI upload only, on the stated premise that the other destination was "internal S3 and the private prerelease index". That is wrong: s3://dd-trace-py-builds is anonymously readable and listable and is registered in DataDog/cloud-inventory as a "Public bucket to host dev builds of dd-trace-py from GitLab to share with customers". The pipeline republished cp315 wheels to main/ on 2026-08-27, so the leak was live, not historical. Three mechanisms publish a ddtrace wheel outside the pipeline: twine to PyPI, aws s3 cp to that bucket, and adms to pypi-private-prereleases. All three now call .gitlab/scripts/prune-unsupported-wheels.sh first, which holds the withheld ABI tags in one place. Putting it inside upload-wheels-to-s3.sh covers every S3 caller and every index suffix, including "upload serverless", which no filter reached before. The PyPI-side shell in release.yml reverts to the pre-filter form plus one prune call, per review feedback: with cp315 gone from the "ddtrace package" artifact, twine check no longer needs a hand-built file list. tests/internal/test_unsupported_wheel_pruning.py replaces the PyPI-specific test. It pins the prune behaviour and asserts that every file under .gitlab/ running a publish command also calls the prune script, so a new upload path cannot silently reintroduce cp315.
f668305 to
def36ad
Compare
The release_pypi job uploaded every artifact matching pywheels/ddtrace-*, which includes the cp315 wheels that "build linux" produces for internal S3 and the private prerelease index. Nothing prevented a Python 3.15 artifact from reaching PyPI other than the Requires-Python bound baked into the wheel metadata, which is a property of the artifact rather than a release guard. Filter cp315 out of the upload list and refuse to upload at all if the resulting list is empty, so a broken filter fails the job instead of silently uploading everything or nothing. twine check --strict still runs over the whole of pywheels/, so cp315 artifacts remain validated. The S3 and adms prerelease paths are unchanged: those are the intended consumers.
…is uploaded The comment used DEV(py-315), a marker form this repo has never used; main has three TODO(py-315) anchors and only a bare "# DEV:" convention. Build the upload list before "twine check" so both steps see the same distributions. --strict promotes warnings to errors, so checking the unfiltered directory let a malformed cp315 wheel from an allow_failure build fail a release whose supported wheels were all fine. release_pypi_prod depends on "ddtrace package", which carries no serverless artifacts, so the narrowed check loses no coverage beyond cp315.
release_pypi_prod is gated on .is_release, so a PR pipeline never runs its shell and the filter added in the previous commit had no coverage. Pull the script block out of .gitlab/release.yml and run it against fixture directories with aws and uvx stubbed on PATH, so the assertions cannot drift from the shipped YAML. Pins three invariants: cp315 wheels are withheld while 3.15 is unsupported and everything else including the sdist still uploads; twine check and twine upload receive the same list, so a malformed wheel from the allow_failure cp315 build cannot fail a release it is not part of; and an empty list is a hard error rather than a silent no-op.
Test docstrings are never Sphinx-rendered, and AGENTS.md (L43-50) treats rST inline markup in them as editor noise. Replace the double-backtick literals in tests/internal/test_release_pypi_filter.py with plain text.
PR #19880 filtered cp315 out of the PyPI upload only, on the stated premise that the other destination was "internal S3 and the private prerelease index". That is wrong: s3://dd-trace-py-builds is anonymously readable and listable and is registered in DataDog/cloud-inventory as a "Public bucket to host dev builds of dd-trace-py from GitLab to share with customers". The pipeline republished cp315 wheels to main/ on 2026-08-27, so the leak was live, not historical. Three mechanisms publish a ddtrace wheel outside the pipeline: twine to PyPI, aws s3 cp to that bucket, and adms to pypi-private-prereleases. All three now call .gitlab/scripts/prune-unsupported-wheels.sh first, which holds the withheld ABI tags in one place. Putting it inside upload-wheels-to-s3.sh covers every S3 caller and every index suffix, including "upload serverless", which no filter reached before. The PyPI-side shell in release.yml reverts to the pre-filter form plus one prune call, per review feedback: with cp315 gone from the "ddtrace package" artifact, twine check no longer needs a hand-built file list. tests/internal/test_unsupported_wheel_pruning.py replaces the PyPI-specific test. It pins the prune behaviour and asserts that every file under .gitlab/ running a publish command also calls the prune script, so a new upload path cannot silently reintroduce cp315.
def36ad to
1a43844
Compare
Description
build linuxandbuild linux serverlessbuildcp315wheels under anallow_failurerule for CI signal only — 3.15 is not a supported target
(
requires-python = ">=3.9,<3.15") — but nothing downstream removed them, so they reachedPyPI,
s3://dd-trace-py-builds, and the private prerelease index.Requires-Pythonis nota sufficient guard: pip enforces the upper bound, uv
"ignores upper bounds entirely".
New
.gitlab/scripts/prune-unsupported-wheels.shdeletes*<tag>*.whlfor each tag inUNSUPPORTED_TAGS(cp315today), and runs before all three publish commands —twine upload,aws s3 cp,adms first-party python upload— and in bothddtrace packagejobs. It exits 1 on a missing directory, so a typo in a call site fails the job.
Testing
tests/internal/test_unsupported_wheel_pruning.pyruns the real script against fixturesand reads the wiring from the shipped YAML, so a new publisher that doesn't prune fails
CI.
Risks
Low. The glob matches no supported wheel filename, and
validate-ddtrace-package.pystillruns after the prune and fails on a missing wheel.
Additional Notes
Known gap: cp315 entries in
debugwheelhouse/*.zipstill reach S3 — not installable,out of scope. Follow-up: make
validate-ddtrace-package.pyfail on unrecognised tags.No release note: CI chore,
changelog/no-changelogapplied.Base:
main. Next: #19911.