iris: include proto files in bundles, remove dashboard from build hook#3654
iris: include proto files in bundles, remove dashboard from build hook#3654
Conversation
The bundle creator uses `git ls-files` which respects .gitignore, excluding generated protobuf files (_pb2.py, _pb2.pyi, _connect.py). When tasks run `uv sync` in containers, the hatch build hook tries to regenerate them via npx, which is slow and unreliable with many concurrent tasks. Include generated build artifacts that exist on disk alongside git-tracked files so the build hook sees up-to-date outputs and skips regeneration.
Dashboard assets are built separately via `iris build dashboard` or `_ensure_dashboard_dist()` in the Docker image pipeline. Having it in the hatch hook meant `uv sync` could trigger a full npm ci + Vue build, surprising developers who only changed Python code. Adds a session-scoped fixture in e2e conftest to build the dashboard once before tests run if dist/ is missing.
|
Claude finished @rjpower's task in 53s —— View job PR Review: iris: include proto files in bundles, remove dashboard from build hookThis PR (1) includes gitignored protobuf generated files in task bundles so containers don't need to regenerate them, and (2) removes the dashboard build from the hatch build hook, deferring it to the CLI or Docker pipeline. Both changes match the stated objectives. No specification check needed — the PR is under 500 lines and the description is clear. Findings:
No bugs or correctness issues found. The PR is clean and well-scoped. |
| """Build dashboard assets once per session so dashboard tests have content to render.""" | ||
| dashboard_dir = IRIS_ROOT / "dashboard" | ||
| dist_dir = dashboard_dir / "dist" | ||
| if dist_dir.exists() and any(dist_dir.iterdir()): |
There was a problem hiding this comment.
fix this, always run the ci to build, we might be out of date
|
@claude fix issues from pr review |
|
Claude finished @rjpower's task in 1m 47s —— View job Fix: always rebuild dashboard in e2e fixture
Change: Removed the Testing: Pre-commit passed ✅ | |
Remove the early return when dist/ already exists so that the dashboard is always rebuilt, ensuring tests never run against stale assets. Co-authored-by: Russell Power <rjpower@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Zip extraction can produce files with slightly different sub-second timestamps. The strict `source > output` mtime check then triggers unnecessary proto regeneration in task containers, which hits BSR rate limits when many tasks run concurrently. A 60-second tolerance prevents this while still catching real edits to .proto files.
| source_newest = _newest_mtime(root, source_globs) | ||
| output_oldest = _oldest_mtime(root, output_globs) | ||
| return source_newest > output_oldest | ||
| return source_newest > output_oldest + 60.0 |
> *Seed scattered wide—* > *bare catches swallow the bugs,* > *narrowed, light breaks through.* - **`file.py`**: Replaced two bare `except Exception:` blocks with `except OSError:` in `FileQueue.pop()` and `FileQueue._recover_expired_leases()`. The broad catches silently swallowed programming errors (TypeError, ValueError, etc.) that should propagate—only filesystem errors (OSError and subclasses like FileNotFoundError) are expected here. - **`http.py`**: Moved `import socket` from inside `get_client_host()` to module level. It's a stdlib import, not an optional dependency guard, so it belongs at the top per coding standards §1.1. Both issues are explicitly flagged as deprecated patterns in `docs/dev-guide/coding-standards.md` §8.2.
The controller restores from checkpoints in remote_state_dir, which can contain stale bundle IDs and job state from a previous run that was killed mid-test. Wipe the remote state dir before starting a fresh controller so tests always start clean.
The test used 900 CPU cores which only works with local mode's virtual 1000-core workers. Cloud TPU VMs have 128 cores, so the job never schedules and the test times out. Use 8 cores in cloud mode.
|
okay enough hacks added, should be fixed now. i now regret trying to be fancy, will never try to be fancy again |
py-spy and memray are installed into .venv/bin/ via uv pip install, but shutil.which() couldn't find them because .venv/bin wasn't on PATH. The CMD already used .venv/bin/python explicitly, but profiling tools need to be discoverable via PATH too.
|
2 participants
_pb2.py,_pb2.pyi,_connect.py) are gitignored but required at runtime. The bundle creator now includes them alongside git-tracked files so task containers skip proto regeneration duringuv sync.uv syncno longer triggersnpm ci+ Vue build. Dashboard is built viairis build dashboardCLI or_ensure_dashboard_dist()in Docker image builds. Added a session-scoped e2e test fixture to build dashboard once ifdist/is missing.