Skip to content

ci: release CI follow-ups: tighter coverage#832

Merged
elijahbenizzy merged 8 commits into
apache:mainfrom
t1mato:ci/release-ci-followups
Jul 12, 2026
Merged

ci: release CI follow-ups: tighter coverage#832
elijahbenizzy merged 8 commits into
apache:mainfrom
t1mato:ci/release-ci-followups

Conversation

@t1mato

@t1mato t1mato commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements all remaining sub-items from #747, covering four areas:

Files & Scripts

  • Rename examples/deep-researcher/utils.py to deep_researcher_utils.py to avoid a RAT basename collision with four other ASF-owned utils.py files
  • Thread --skip-signing through cmd_verify so CI can verify artifacts without GPG keys
  • Extend Apache RAT scanning to .whl artifacts in addition to source and sdist tarballs

Smoke Test

  • Replace the hardcoded time.sleep(2) with a polling loop on /api/v0/projects that fails fast if the server exits
  • Launch the server in its own process group and send SIGTERM to the whole group on teardown to prevent orphaned uvicorn processes
  • Add a GET / check to verify the UI is served
  • Add --cleanup/--no-cleanup flag (auto-disabled in GITHUB_ACTIONS so workspaces survive for artifact upload)

CI Coverage Gaps

  • Add a bare-install job that installs the wheel without optional extras and imports core symbols, catching accidental leakage of optional dependencies
  • Add an sdist-wheel-equivalence job that rebuilds the wheel from the sdist tarball and compares file content hashes against the CI-built wheel, catching missing sdist files
  • Pin the Apache RAT JAR download with a SHA256 checksum

Hygiene

  • Add scripts/check_asf_headers.py and wire it as a pre-commit hook that checks Python, YAML, and shell files for the ASF license header, reading .rat-excludes at runtime to stay in sync with RAT automatically
  • Add a weekly cron (Mondays 09:00 UTC) to the release validation workflow to catch dependency drift between releases

Files

  • .rat-excludes, examples/deep-researcher/ — rename and exclusion update
  • scripts/apache_release.py--skip-signing in cmd_verify
  • scripts/verify_apache_artifacts.py — RAT on wheels, compare-wheels subcommand
  • scripts/ci_smoke_server.py — polling, process group, cleanup flag, GET /
  • scripts/check_asf_headers.py — new ASF header checker
  • .pre-commit-config.yaml — new check-asf-headers hook
  • .github/workflows/release-validation.yml — SHA256 pin, bare-install, sdist-wheel-equivalence, weekly cron
  • tests/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 above

Test Plan

  • Run 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 pass
  • Release Validation workflow runs clean on this PR, including the new bare-install and sdist-wheel-equivalence jobs
  • Pre-commit hook fires on a staged Python file missing the ASF header

Closes #747

@github-actions github-actions Bot added area/ci Workflows, build, release scripts area/examples Relates to /examples labels Jul 1, 2026

@skrawcz skrawcz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 pytest is unused; flake8 (F401) will fail. Remove it.
  • tests/test_ci_smoke_server.py:22 [BLOCKING]import pytest is unused; flake8 (F401) will fail. Remove it.
  • tests/test_ci_smoke_server.py:110 [BLOCKING] — black wants a blank line after the inline import json inside read(), and this file has other black diffs. Run black --line-length 100 on the file. (Confirmed with pinned black 23.11.0.)
  • tests/test_ci_smoke_server.py:147 [BLOCKING] — the monkeypatch.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] — the print(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 references utils.py; update to deep_researcher_utils.py to match the rename.
  • scripts/check_asf_headers.py:56 [NIT] — prefer parameterized list[str] over bare list for these annotations to match repo conventions (would require from __future__ import annotations for 3.9 runtime safety). Non-blocking.
  • scripts/verify_apache_artifacts.py:625 [SUGGESTION] — the new .whlzipfile.ZipFile(...).extractall(extract_dir) branch in _check_licenses_with_rat isn't directly unit-tested (only reached via a mocked _check_licenses_with_rat in test_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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change?

@elijahbenizzy

Copy link
Copy Markdown
Contributor

Thanks! @t1mato can we fix it up so the tests pass? Some simple formatting + the release validation (i.e. the stuff we're testing here.)

@github-actions github-actions Bot added the pr/needs-rebase Conflicts with main label Jul 11, 2026
@elijahbenizzy

Copy link
Copy Markdown
Contributor

Since `maintainerCanModify` is on, I pushed a small fix commit on top of your branch to unblock CI. Feel free to pull it back locally.

  • black on the four new/modified files (`scripts/{ci_smoke_server,verify_apache_artifacts}.py`, `tests/test_{ci_smoke_server,verify_apache_artifacts}.py`). This unblocks the `pre-commit` check.
  • `.rat-excludes` — added `/METADATA`, `/WHEEL`, `**/RECORD` for the wheel dist-info metadata. Sdist and src RAT scans were already passing; only the newly-added wheel scan was failing, and it was failing on those three auto-generated files that have no ASF header. This unblocks `Release Validation / build-artifacts`.

If CI still fails after this, my next guesses are (in order): the built wheel bundles additional non-source content from `telemetry/ui/build/` that also needs excluding, or the licenses-inside-dist-info path structure confuses RAT. Happy to dig deeper if it recurs.

Thanks for the substantive PR!

t1mato and others added 5 commits July 11, 2026 20:08
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
Two small fixes on top of t1mato/burr@ci/release-ci-followups so CI goes
green:

* Black formatting on the four new/modified files (auto-fix; contributor
  can pull this back locally, or maintainers can just squash-in on
  merge).

* Wheel RAT scan: sdist and src tarball RAT scans pass, but the
  newly-added wheel scan fails because auto-generated dist-info metadata
  files (METADATA, WHEEL, RECORD) have no source and no ASF header.
  Added those three basename globs to .rat-excludes.

Verified locally: black + flake8 + isort clean on the touched files.
RAT reproduction not possible without the jar, but the failure signature
in CI matches this class of file exactly and the excludes are the
minimum-necessary additions.
@elijahbenizzy elijahbenizzy force-pushed the ci/release-ci-followups branch from fa0e2ac to c83ef9f Compare July 12, 2026 03:08
@github-actions github-actions Bot removed the pr/needs-rebase Conflicts with main label Jul 12, 2026
Follow-up fixes on top of the black + METADATA/WHEEL/RECORD change:

* RAT was still finding 6 unapproved files in the wheel: the bundled
  tracking-UI static assets (index.html, minified CSS/JS bundles and
  their source maps). These are generated by the telemetry/ui build
  and shipped inside the wheel so the tracking server can serve them
  at runtime; they have no ASF headers, and their sources
  (telemetry/ui/src/**/*.tsx) carry their own headers checked against
  the source release. Added burr/tracking/server/build/** to
  .rat-excludes.

* isort was failing on two files not part of this PR
  (burr/integrations/persisters/b_mongodb.py and
  tests/integrations/persisters/test_b_pymongo_driver_info.py) --
  the current pre-commit config wants the imports collapsed to a
  single line. Applied.

Local RAT run against the extracted wheel now shows 0 unapproved
files (the CI wheel will not contain the stale
examples/deep-researcher/utils.py that my local one had -- that
file was renamed to deep_researcher_utils.py earlier in the PR).
@github-actions github-actions Bot added area/storage Persisters, state storage area/integrations External integrations (LLMs, frameworks) labels Jul 12, 2026
_build_wheel_from_current_dir was hardcoded to leave the wheel at
dist/apache_burr-*.whl regardless of the --output-dir argument
(flit build always writes to dist/). The Release Validation
sdist-wheel-equivalence job (added in this PR) discovered this
by extracting the sdist, cd-ing in, and running

    python scripts/apache_release.py wheel VERSION 0 \
      --skip-signing --output-dir /tmp/sdist-wheel

then comparing that wheel against the release wheel. The wheel was
created but sat in <extracted-sdist>/dist/ instead of /tmp/sdist-wheel,
so the compare step failed with 'Wheel not found: /tmp/sdist-wheel/...'.

After flit build writes to dist/, move the wheel to output_dir if
the caller specified a different location. No-op for the standard
in-place build (output_dir defaults to 'dist').

@elijahbenizzy elijahbenizzy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, CI green after the fixes for pre-commit / RAT / sdist-wheel-equivalence. Nice PR — the sdist↔wheel equivalence check flushed out a real latent bug in `apache_release.py wheel`'s `--output-dir` handling, which is exactly the kind of thing that job is meant to catch. Thanks for the work.

@elijahbenizzy elijahbenizzy merged commit 19d3fcf into apache:main Jul 12, 2026
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ci Workflows, build, release scripts area/examples Relates to /examples area/integrations External integrations (LLMs, frameworks) area/storage Persisters, state storage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Release CI follow-ups: tighter coverage

3 participants