Skip to content

feat(buildrunner): drop per-call queueName, add Factory#184

Closed
behinddwalls wants to merge 1 commit into
mainfrom
preetam/buildrunner-factory
Closed

feat(buildrunner): drop per-call queueName, add Factory#184
behinddwalls wants to merge 1 commit into
mainfrom
preetam/buildrunner-factory

Conversation

@behinddwalls
Copy link
Copy Markdown
Collaborator

@behinddwalls behinddwalls commented Jun 3, 2026

Summary

Why?

BuildRunner.Trigger took a queueName argument that selected the
runner-specific job configuration on every call. That put queue routing
on the hot path and on a single verb, leaving Status and Cancel to
rediscover the queue from the build ID. The queue belongs at construction
time, not per call.

What?

  • Drop queueName from BuildRunner.Trigger; the verbs speak only in
    builds and changes.
  • Add a Factory interface (New(cfg Config) (BuildRunner, error)) and
    a Config struct carrying QueueID. A runner is bound to its Config at
    construction; backends extend Config with their own settings.
  • noop: add NewFactory(); keep New().
  • Wire the factory end to end: the build and buildsignal controllers hold
    a buildrunner.Factory and build a runner per queue via
    New(Config{QueueID: batch.Queue}). buildsignal loads the batch to
    resolve the queue (TODO: denormalize queue onto the Build). The example
    orchestrator wires buildnoop.NewFactory().
  • Docs: BuildRunner is no longer described as a "singleton" but must stay
    safe for concurrent use. Updated the RFC (Construction section) and
    README.

Caching of runners per queue is intentionally omitted for now.

Test Plan

make mocks, make gazelle, make tidy, make fmt
bazel build //...
make test (unit), incl. buildrunner + build + buildsignal

## Summary

### Why?

`BuildRunner.Trigger` took a `queueName` argument that selected the
runner-specific job configuration on every call. That put queue routing
on the hot path and on a single verb, leaving `Status` and `Cancel` to
rediscover the queue from the build ID. The queue belongs at construction
time, not per call.

### What?

- Drop `queueName` from `BuildRunner.Trigger`; the verbs speak only in
  builds and changes.
- Add a `Factory` interface (`New(cfg Config) (BuildRunner, error)`) and
  a `Config` struct carrying `QueueID`. A runner is bound to its Config at
  construction; backends extend `Config` with their own settings.
- noop: add `NewFactory()`; keep `New()`.
- Wire the factory end to end: the build and buildsignal controllers hold
  a `buildrunner.Factory` and build a runner per queue via
  `New(Config{QueueID: batch.Queue})`. buildsignal loads the batch to
  resolve the queue (TODO: denormalize queue onto the Build). The example
  orchestrator wires `buildnoop.NewFactory()`.
- Docs: BuildRunner is no longer described as a "singleton" but must stay
  safe for concurrent use. Updated the RFC (Construction section) and
  README.

Caching of runners per queue is intentionally omitted for now.

## Test Plan

✅ `make mocks`, `make gazelle`, `make tidy`, `make fmt`
✅ `bazel build //...`
✅ `make test` (unit), incl. buildrunner + build + buildsignal
@behinddwalls behinddwalls force-pushed the preetam/buildrunner-factory branch from aa1bd49 to 30458c9 Compare June 3, 2026 05:34
@behinddwalls behinddwalls deleted the preetam/buildrunner-factory branch June 3, 2026 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant