Repository navigation
refactor(demo): extract reusable workload runner - #802
Merged
Merged
Conversation
## Summary ### Why? Bulk demo scheduling and tracking lived inside the OSS command, preventing internal clients from reusing the same workloads with their own providers and transports. ### What? Extract an importable runner under service/submitqueue/demo with injected change sources, submission/history clients and readiness checks. Preserve independent, burst and stacked workloads and the existing OSS CLI/provider behavior. Add transport-independent history polling, cancellation handling and partial-artifact results, with behavioral tests and documentation. ## Test Plan - Passed make fmt, make lint, make check-tidy and make check-gazelle. - Passed the Bazel tests for service/submitqueue/demo, service/submitqueue/demo/requests and submitqueue/client. - Previously passed race tests for the affected Go packages with an explicitly supplied local git executable. - No live GitHub PRs or deployed requests were created during automated validation.
## Summary ### Why? The history poller shared its context with workload creation, so a permanent history read error (for example `PermissionDenied`, or a transient `Internal` from a proxy) cancelled in-flight `Source.Open` and `Land` calls. Cancelling a mutation midway leaves its remote outcome unknown, which the runner otherwise goes out of its way to avoid. The tracker also treated `Aborted` and `Internal` as permanent, while callers' own readiness polling treats them as transient. ### What? - Polling runs on its own derived context; a failed watch is noted and joined into the result, but creation and submission finish. - `Aborted` and `Internal` history read errors are retried like other transient codes. - Caller cancellation is reported once instead of twice. - In stacked mode the shared row's readiness note is set once around all links, so the first ready link no longer clears it while others still wait. - `demo.Gateway` parameters are named. ## Test Plan ✅ `bazel test //service/submitqueue/demo/... //submitqueue/client/...` (new `TestWatchFailureDoesNotCancelCreationOrSubmission`; transient table extended) ✅ `make lint`, `make check-gazelle`
## Summary ### Why? Independent and burst workers ran `Source.Open` and `Gateway.Land` under the errgroup's derived context, so one worker failing (for example a readiness timeout) cancelled every sibling's in-flight mutation. A cancelled `Land` may still have been accepted by the gateway, and its request ID was then lost from `RunResult.Requests`. Separately, a watch that ended early (quit key, permanent poll error, cancellation) restored the terminal without ever drawing the final table, so the table vanished with the alternate screen. ### What? - `forEachChange` hands workers a gate that closes when a sibling fails. `Open` and `Land` check the gate before starting but run on the caller's context; readiness waits still use the gate and stop promptly. - After the interactive view, the final table is always drawn into the scrollback; its verdict is returned only when nothing else failed. - The OSS `requests` command no longer lists pinned changes twice after a partially failed `-land=false` run. ## Test Plan ✅ `bazel test //service/submitqueue/demo/... //submitqueue/client/...`, also with `--@rules_go//go/config:race` (new deterministic `TestSiblingFailureDoesNotCancelInFlightLand`) ✅ `make lint`, `make check-gazelle`
behinddwalls
marked this pull request as ready for review
October 7, 2026 17:28
mnoah1
approved these changes
Oct 7, 2026
behinddwalls
temporarily deployed
to
stack-rebase
October 7, 2026 17:50 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why?
Bulk demo scheduling and tracking lived inside the OSS
requestscommand, so other clients could not reuse the same workloads with their own change sources, transports, and readiness policies. Extracting it also surfaced several safety gaps in how the runner handles failures around mutating calls.What?
service/submitqueue/demowith injectedSource,Gateway, and optionalReadiness.Rungenerates changes;RunExistingsubmits caller-resolved, head-pinned changes without invoking the source. Independent, burst, and stacked workloads are preserved, and the OSSrequestscommand and its flags/Make targets keep their behavior on top of it. Results retain known changes, accepted request IDs, and last observed histories, including on partial failure.client.Tracker.PollHistoryreads through aHistoryReader, retries transient errors (Unavailable,DeadlineExceeded,ResourceExhausted,Aborted,Internal) and not-yet-materialized histories, and returns permanent errors without changing a request's recorded status.Source.OpenorGateway.Landalready in flight, whose remote outcome would otherwise be unknown.Intentional behavior changes in the OSS command: leaving the watch with the quit key now exits non-zero ("watch interrupted; submitted requests continue remotely"), the run tag gains a random suffix so simultaneous runs cannot collide, and the opening line no longer reports the resolved folder count.
Test Plan
✅
bazel test //service/submitqueue/demo/... //submitqueue/client/..., also with--@rules_go//go/config:race✅
make fmt,make lint,make check-tidy,make check-gazelleLandis not cancelled.🤖 Generated with Claude Code