Skip to content

fix: address RC2 vote feedback#745

Merged
elijahbenizzy merged 12 commits intomainfrom
fix/rc2-feedback
Apr 24, 2026
Merged

fix: address RC2 vote feedback#745
elijahbenizzy merged 12 commits intomainfrom
fix/rc2-feedback

Conversation

@elijahbenizzy
Copy link
Copy Markdown
Contributor

Summary

Addresses feedback from Stefan Krawczyk on the 0.42.0-incubating RC2 vote:

  • Add burr/tracking/server/__init__.py — makes it a proper Python package so importlib.resources can find static files when installed from wheel
  • Add tests/__init__.py — allows pytest tests/ to work
  • Add ASF header to examples/deployment/aws/terraform/tutorial.md
  • Update .rat-excludes: fix patterns for burr/examples/ symlink duplicates, add .github/ templates and image file extensions

Test plan

  • Build wheel, install in fresh venv, run burr outside source tree — server should start
  • pytest tests/ should discover and run tests
  • Run Apache RAT — zero unapproved files

@github-actions github-actions Bot added area/tracking Telemetry, tracing, OpenTelemetry area/ci Workflows, build, release scripts area/examples Relates to /examples labels Apr 20, 2026
- Fix patterns for burr/examples/ symlink duplicates (use .* prefix)
- Add .github/ templates (YAML/MD with frontmatter)
- Add image file extensions (.png, .gif, .ico, .jpg)
@elijahbenizzy elijahbenizzy force-pushed the fix/rc2-feedback branch 3 times, most recently from ecdbbe8 to f65be71 Compare April 21, 2026 00:10
elijahbenizzy added a commit that referenced this pull request Apr 21, 2026
Mirror the .rat-excludes updates from #745 so this CI branch passes
license checks against current main. Specifically:

- Use .* prefix for deep-researcher patterns so the symlinked
  burr/examples/deep-researcher/ paths are also matched.
- Exclude .github/ templates (YAML/MD with frontmatter).
- Exclude common binary image extensions (.png, .gif, .ico, .jpg).
- Exclude examples/deployment/aws/terraform/tutorial.md.
@elijahbenizzy elijahbenizzy requested a review from skrawcz April 21, 2026 03:15
The tracking server imports burr.examples.hello-world-counter.server
(added in #675) but the release config still had it in the exclude
list. Move it from exclude to include so the wheel ships with the
module the server needs, preventing ModuleNotFoundError when running
burr from a fresh install.

Also bumps REQUIRED_EXAMPLES in scripts/apache_release.py and updates
tests/test_release_config.py accordingly.
The CLI spawned uvicorn as a bare command, which resolves via PATH.
For users with pyenv installed, ~/.pyenv/shims/ is in PATH ahead of
any venv's bin/, so bare `uvicorn` always hits a pyenv shim that
routes to a pyenv environment — never the venv the user is running
burr from. This means a fresh `pip install` into a non-pyenv venv
fails to start the server.

Using sys.executable -m uvicorn ensures uvicorn runs under the same
Python interpreter that is running burr, regardless of PATH.
Preparation for the release-validation CI workflow.

apache_release.py:
- Add --skip-signing to archive/sdist/wheel/all subcommands
- Split _sign_artifact into checksum (always) and GPG sign (skippable)
- Drop gpg from required tools when --skip-signing is set

ci_smoke_server.py:
- Installs a built wheel into a fresh venv outside the source tree
- Imports burr.tracking.server.run (catches missing-example bugs like #675)
- Starts the burr server on a free port with a dedicated data dir
- Runs a tracked app, verifies the server observes the project
CI runners don't have svn installed. The 'all' subcommand listed svn as
a required tool unconditionally, but svn is only used during the upload
step. Skip the requirement when --no-upload is passed.
Build the full release pipeline on every PR, plus RAT license check
and an end-to-end smoke install of the wheel outside the source tree.
This catches the failure modes that have broken recent RCs:

- build-artifacts: runs apache_release.py all --skip-signing --no-upload,
  verifies the 3 artifacts exist, runs Apache RAT without --report-only
  so license violations fail the job.
- install-and-smoke: on 3.10/3.11/3.12, installs the wheel into a fresh
  venv and runs scripts/ci_smoke_server.py. That helper imports the
  server module, boots the server, runs a tracked app, and asserts the
  server sees the project — so a missing example (like PR #675's
  hello-world-counter) would fail here.

Only "build-artifacts" and "install-and-smoke (3.12)" are required
checks in .asf.yaml; 3.10 and 3.11 are informational.
RAT's -E regex doesn't reliably match through path separators or
symlinked directories. The previous patterns like
'examples/deep-researcher/prompts.py' never excluded anything because
RAT effectively matches against basenames. We never noticed because
all prior RAT runs used --report-only mode.

Switch to basename patterns. The .tsx files are uniquely named, so
basename matching is precise. prompts.py only exists once. utils.py
matches 5 different files (all our own ASF code with headers); the
practical risk of skipping their checks is low.

Update the leading comment in .rat-excludes to document the constraint
so future authors don't repeat the mistake.
- Workflow: drop '**/*.md' from paths-ignore. It would suppress runs
  whenever a bundled .md file (like the deployment tutorial.md) changes
  — exactly the case that triggered RC2 feedback.
- .rat-excludes: document the basename collisions for utils.py and
  button.tsx so the next maintainer doesn't think they're unique.
- test_release_config.py: fix '4 examples' string that should have been
  bumped to 5 when hello-world-counter was added.
…symlinks

On CI runners, burr/examples is a relative symlink ('../examples') that
may not resolve from the process's working directory — os.path.exists
returns False for a broken link while the link itself is still present
on disk. That made _prepare_wheel_contents skip the removal branch and
then fail on os.makedirs with 'File exists: burr/examples'.

Switch to os.path.lexists, which returns True for any directory entry
including broken symlinks, so the cleanup branch always runs when the
link is present.
Copy link
Copy Markdown
Collaborator

@andreahlert andreahlert left a comment

Choose a reason for hiding this comment

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

tks @elijahbenizzy. LGTM

@elijahbenizzy elijahbenizzy merged commit ff76817 into main Apr 24, 2026
25 checks passed
@elijahbenizzy elijahbenizzy deleted the fix/rc2-feedback branch April 24, 2026 23:18
elijahbenizzy added a commit that referenced this pull request Apr 25, 2026
Previously, if RAT crashed partway through scanning (e.g. on the
broken burr/examples symlink that motivated this PR), the verify
script would print a warning, parse the truncated XML report, find
zero unapproved licenses in whatever RAT managed to scan before the
crash, and report success.

That false green is what hid the symlink corruption when PR #745
merged into main, even though our release-validation workflow
technically ran on main. The CI run scanned 14 files (out of ~700),
saw RAT exit code 1, swallowed it, and called the build green.

Treat any nonzero exit from RAT as a hard failure of license
verification. Print the last 25 lines of RAT stderr so the next
debug session has the trace immediately.
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/tracking Telemetry, tracing, OpenTelemetry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants