ci: release CI follow-ups: tighter coverage#832
Conversation
Rename examples/deep-researcher/utils.py to deep_researcher_utils.py to avoid basename collision with four other ASF-owned utils.py files that Apache RAT skips. Update .rat-excludes and the application import accordingly. Thread --skip-signing through cmd_verify in scripts/apache_release.py so CI can run full artifact verification without GPG keys present. Extend Apache RAT scanning to wheel (.whl) artifacts in addition to source and sdist tarballs, so license header regressions in packaged files are caught before the release vote. Closes apache#747 (partial)
Replace fixed time.sleep(2) with a polling loop on /api/v0/projects so the smoke test waits only as long as necessary and fails fast if the server process exits unexpectedly. Launch the server in its own process group (start_new_session=True) and send SIGTERM to the whole group on teardown so uvicorn child processes are not orphaned. Add GET / check to verify the UI is being served by the installed wheel. Add --cleanup / --no-cleanup flag; defaults to cleanup locally but preserves the workspace in GITHUB_ACTIONS so artifacts are available for upload on failure. Add tests/test_ci_smoke_server.py covering all new testable helpers. Closes apache#747 (partial)
Add _wheel_content_hashes and _compare_wheel_contents to verify_apache_artifacts.py to compare wheels by file content hashes rather than binary equality (zip timestamps make byte-for-byte comparison unreliable). Add compare-wheels subcommand exposing this from the CLI. Add bare-install job: installs the wheel without optional extras and imports core symbols to catch accidental leakage of optional dependencies into core code. Add sdist-wheel-equivalence job: extracts the sdist tarball, rebuilds the wheel from it (including the npm frontend build), and compares content hashes against the CI-built wheel to catch files missing from the sdist. Pin the Apache RAT JAR download with a SHA256 checksum to guard against supply-chain tampering. Closes apache#747 (partial)
Add scripts/check_asf_headers.py: checks that Python, YAML, and shell files carry the ASF license header. Reads .rat-excludes at runtime so known third-party files are automatically respected without duplicating the exclusion list. Wire the script into .pre-commit-config.yaml as a local hook so missing headers are caught before a commit lands. Add weekly cron schedule (Monday 09:00 UTC) to release-validation.yml so dependency drift against main is detected between releases. Add tests/test_check_asf_headers.py with 15 tests covering all helper functions and the main entry point. Closes apache#747
skrawcz
left a comment
There was a problem hiding this comment.
Nice, tightly-scoped follow-up to #747 — this materially strengthens the release pipeline. Highlights: the bare-install job that guards against optional-dep leakage into core (directly reinforces the project's vendor-neutrality invariant), the shift from whole-file SHA512 to per-file content hashing for wheel equivalence (correctly ignoring zip timestamps and RECORD), replacing the flaky time.sleep(2) with a real poll that fails fast on server death, and process-group teardown to kill orphaned uvicorn. Test coverage is excellent — 54 focused tests, all passing locally, covering happy paths, timeouts, early-exit, and glob-matching edge cases.
Backwards compatibility is clean: no burr/ library surface, defaults, return types, or serialization change; --skip-signing and compare-wheels are additive with safe defaults, and the example rename updates every import consistently.
Two blocking items before merge, both mechanical: (1) black fails on four files under the repo-pinned black 23.11.0 — the pre-commit hook will block CI; (2) two unused import pytest statements trip flake8 F401. There's also one small doc gap: the deep-researcher README still references the old utils.py filename. Fix those three and this is ready.
Inline notes (line refs approximate — folded into body since this is a body-only draft):
- tests/test_check_asf_headers.py:22
[BLOCKING]—import pytestis unused; flake8 (F401) will fail. Remove it. - tests/test_ci_smoke_server.py:22
[BLOCKING]—import pytestis unused; flake8 (F401) will fail. Remove it. - tests/test_ci_smoke_server.py:110
[BLOCKING]— black wants a blank line after the inlineimport jsoninsideread(), and this file has other black diffs. Runblack --line-length 100on the file. (Confirmed with pinned black 23.11.0.) - tests/test_ci_smoke_server.py:147
[BLOCKING]— themonkeypatch.setattr(..., lambda *a, **kw: (_ for _ in ()).throw(...))line exceeds 100 cols; black will wrap it across multiple lines. - scripts/ci_smoke_server.py:245
[BLOCKING]—_fail(f"GET / returned HTTP {resp.status}, ...")exceeds 100 cols; black will wrap the call. Reformat the file. - scripts/verify_apache_artifacts.py:757
[BLOCKING]— theprint(f"Found {len(rat_artifacts)} artifact(s) ...")line exceeds 100 cols; black will wrap it. Reformat the file. - examples/deep-researcher/README.md:28
[NIT]— still referencesutils.py; update todeep_researcher_utils.pyto match the rename. - scripts/check_asf_headers.py:56
[NIT]— prefer parameterizedlist[str]over barelistfor these annotations to match repo conventions (would requirefrom __future__ import annotationsfor 3.9 runtime safety). Non-blocking. - scripts/verify_apache_artifacts.py:625
[SUGGESTION]— the new.whl→zipfile.ZipFile(...).extractall(extract_dir)branch in_check_licenses_with_ratisn't directly unit-tested (only reached via a mocked_check_licenses_with_ratintest_verify_licenses_runs_rat_on_wheel_...). Consider a small test that actually extracts a fake wheel to lock the branch in. Low risk. - .github/workflows/release-validation.yml:125
[SUGGESTION]— the RAT JAR SHA256 pin is a good hardening step; consider a brief comment noting how to regenerate the checksum when bumping the RAT version (0.18 → future), so the next upgrader knows the provenance.
Summary
Implements all remaining sub-items from #747, covering four areas:
Files & Scripts
examples/deep-researcher/utils.pytodeep_researcher_utils.pyto avoid a RAT basename collision with four other ASF-ownedutils.pyfiles--skip-signingthroughcmd_verifyso CI can verify artifacts without GPG keys.whlartifacts in addition to source and sdist tarballsSmoke Test
time.sleep(2)with a polling loop on/api/v0/projectsthat fails fast if the server exitsGET /check to verify the UI is served--cleanup/--no-cleanupflag (auto-disabled inGITHUB_ACTIONSso workspaces survive for artifact upload)CI Coverage Gaps
bare-installjob that installs the wheel without optional extras and imports core symbols, catching accidental leakage of optional dependenciessdist-wheel-equivalencejob that rebuilds the wheel from the sdist tarball and compares file content hashes against the CI-built wheel, catching missing sdist filesHygiene
scripts/check_asf_headers.pyand wire it as a pre-commit hook that checks Python, YAML, and shell files for the ASF license header, reading.rat-excludesat runtime to stay in sync with RAT automaticallyFiles
.rat-excludes,examples/deep-researcher/— rename and exclusion updatescripts/apache_release.py—--skip-signingincmd_verifyscripts/verify_apache_artifacts.py— RAT on wheels,compare-wheelssubcommandscripts/ci_smoke_server.py— polling, process group, cleanup flag, GET /scripts/check_asf_headers.py— new ASF header checker.pre-commit-config.yaml— newcheck-asf-headershook.github/workflows/release-validation.yml— SHA256 pin,bare-install,sdist-wheel-equivalence, weekly crontests/test_apache_release.py,tests/test_verify_apache_artifacts.py,tests/test_ci_smoke_server.py,tests/test_check_asf_headers.py— test coverage for all of the aboveTest Plan
pytest tests/test_apache_release.py tests/test_verify_apache_artifacts.py tests/test_ci_smoke_server.py tests/test_check_asf_headers.py— all 54 tests passbare-installandsdist-wheel-equivalencejobsCloses #747