Skip to content

Commit 43f3258

Browse files
committed
feat(example): per-queue extension wiring; retire buildrunner/noop
## Summary ### Why? Demonstrate SubmitQueue's core premise — per-queue pluggable extensions — in the example orchestrator, rather than wiring one implementation for every queue, and make the fakes drivable end-to-end. Also document where factory implementations and queue routing belong. Stacked on the fakes PR, which adds the extension/*/fake packages this PR wires in. ### What? - Queue-major registry in example main.go: queueExtensions (one queue's full impl set) + queueRegistry (queue -> profile, with a default). Six thin Factory adapters resolve impls by Config.QueueName — all routing lives here in the wiring layer. - Per-queue profiles: test-queue (heuristic over lines changed), e2e-test-queue (composite scorer, no-conflict), plus a new e2e-conflict-error-queue that always fails conflict analysis. Edge integrations + build runner share a baseline; scorer/analyzer are wrapped by the decorator fakes (score-error marker / failing predicate). - Retire buildrunner/noop (subsumed by buildrunner/fake); switch the build controller unit test to buildfake. - queues.yaml: register e2e-conflict-error-queue. - CLAUDE.md: extensions hold contracts + impls only — factory implementations and routing live in the wiring layer; plus a TODO to evaluate promoting the registry into submitqueue/core later. ## Test Plan - ✅ `make test` — 44/44 unit tests - ✅ `make e2e-test` — 1/1 - ✅ `make integration-test` — 7/7 - ✅ `make check-gazelle` / `make check-tidy` — clean - ✅ Live stack (`make local-submitqueue-start`): Ping OK; gateway rejects unknown queues; validated end-to-end — scorer passthrough (test-queue), scorer error (`?sq-fake=score-error` -> "fake: marked score error"), and conflict error (e2e-conflict-error-queue -> "fake: injected analyze error").
1 parent 3471964 commit 43f3258

9 files changed

Lines changed: 238 additions & 243 deletions

File tree

CLAUDE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,8 @@ Vendor-agnostic, pluggable interfaces with implementations in subdirectories:
116116
2. Implementations at `extension/{ext}/{impl}/`
117117
3. Factory interface for dependency injection and lifecycle management
118118
119+
**Extensions hold contracts and impls only — never factory implementations or routing.** An `extension/{ext}` package owns the behavioral interface, its `Config`, the `Factory` *interface*, and impl constructors `New(...)` that return the interface. It must NOT contain `Factory` *implementations* (no `NewFactory()` constructors or factory structs) and no queue-selection/routing logic. The reason: an impl package (e.g. `scorer/heuristic`) cannot know the queue topology or the other impls, so any "which impl for which queue" decision baked there is wrong by construction. A `Factory` whose `For(cfg)` ignores `cfg.QueueName` is just an impl→Factory adapter — that adapting, and all per-queue routing, belongs in the **wiring/integrator layer** (e.g. `example/{domain}/{service}/server/main.go`), which is the one place that knows the queue set. There, adapt `New(...)` impls to the `Factory` contract and route on `Config.QueueName`. Keep this seam strict: if you're about to add a `NewFactory()` or a `map[queue]impl` under `extension/`, it belongs in the wiring layer instead.
120+
119121
**Design interfaces for the technology *space*, not the implementation in front of you.** The interface is a contract every backend will have to satisfy — SQL, key-value (DynamoDB, Bigtable), document, message queue, search, RPC, in-memory, mocks. If the contract assumes a capability that some plausible backend can't provide cheaply, you've baked the current impl's strengths into the API.
120122
121123
Common over-constraints to avoid:

example/submitqueue/gateway/server/queues.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,7 @@ queues:
77
- name: test-queue
88
- name: e2e-test-queue
99
- name: e2e-cancel-queue
10+
# Routes to an analyzer that always errors (conflictfake.FailAlways) so e2e can
11+
# exercise the conflict-analysis error path. See newQueueRegistry in the
12+
# orchestrator example server.
13+
- name: e2e-conflict-error-queue

example/submitqueue/orchestrator/server/BUILD.bazel

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,23 @@ go_library(
2121
"//submitqueue/core/consumer",
2222
"//submitqueue/entity",
2323
"//submitqueue/extension/buildrunner",
24-
"//submitqueue/extension/buildrunner/noop",
24+
"//submitqueue/extension/buildrunner/fake",
2525
"//submitqueue/extension/changeprovider",
26+
"//submitqueue/extension/changeprovider/fake",
2627
"//submitqueue/extension/changeprovider/github",
2728
"//submitqueue/extension/conflict",
2829
"//submitqueue/extension/conflict/all",
30+
"//submitqueue/extension/conflict/fake",
31+
"//submitqueue/extension/conflict/none",
2932
"//submitqueue/extension/mergechecker",
33+
"//submitqueue/extension/mergechecker/fake",
3034
"//submitqueue/extension/mergechecker/github",
3135
"//submitqueue/extension/pusher",
36+
"//submitqueue/extension/pusher/fake",
3237
"//submitqueue/extension/pusher/git",
3338
"//submitqueue/extension/scorer",
39+
"//submitqueue/extension/scorer/composite",
40+
"//submitqueue/extension/scorer/fake",
3441
"//submitqueue/extension/scorer/heuristic",
3542
"//submitqueue/extension/storage",
3643
"//submitqueue/extension/storage/mysql",

example/submitqueue/orchestrator/server/main.go

Lines changed: 216 additions & 93 deletions
Large diffs are not rendered by default.

submitqueue/extension/buildrunner/noop/BUILD.bazel

Lines changed: 0 additions & 24 deletions
This file was deleted.

submitqueue/extension/buildrunner/noop/noop.go

Lines changed: 0 additions & 56 deletions
This file was deleted.

submitqueue/extension/buildrunner/noop/noop_test.go

Lines changed: 0 additions & 61 deletions
This file was deleted.

submitqueue/orchestrator/controller/build/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ go_test(
2828
"//submitqueue/core/consumer",
2929
"//submitqueue/entity",
3030
"//submitqueue/extension/buildrunner",
31+
"//submitqueue/extension/buildrunner/fake",
3132
"//submitqueue/extension/buildrunner/mock",
32-
"//submitqueue/extension/buildrunner/noop",
3333
"//submitqueue/extension/storage",
3434
"//submitqueue/extension/storage/mock",
3535
"@com_github_stretchr_testify//assert",

submitqueue/orchestrator/controller/build/build_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ import (
2828
"github.com/uber/submitqueue/submitqueue/core/consumer"
2929
"github.com/uber/submitqueue/submitqueue/entity"
3030
"github.com/uber/submitqueue/submitqueue/extension/buildrunner"
31+
buildfake "github.com/uber/submitqueue/submitqueue/extension/buildrunner/fake"
3132
buildrunnermock "github.com/uber/submitqueue/submitqueue/extension/buildrunner/mock"
32-
buildnoop "github.com/uber/submitqueue/submitqueue/extension/buildrunner/noop"
3333
"github.com/uber/submitqueue/submitqueue/extension/storage"
3434
storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock"
3535
"go.uber.org/mock/gomock"
@@ -74,7 +74,7 @@ func newMockStorage(ctrl *gomock.Controller, batch entity.Batch) *storagemock.Mo
7474
}
7575

7676
// newTestController creates a controller with test dependencies. br is the
77-
// build runner to inject; pass buildnoop.New() for the pass-through default.
77+
// build runner to inject; pass buildfake.New() for the pass-through default.
7878
// staticBuildRunnerFactory is a test factory that returns a fixed BuildRunner
7979
// for any entityqueue.
8080
type staticBuildRunnerFactory struct{ r buildrunner.BuildRunner }
@@ -111,7 +111,7 @@ func TestNewController(t *testing.T) {
111111
ctrl := gomock.NewController(t)
112112
batch := testBatch()
113113
store := newMockStorage(ctrl, batch)
114-
controller := newTestController(t, ctrl, store, buildnoop.New(), nil)
114+
controller := newTestController(t, ctrl, store, buildfake.New(), nil)
115115

116116
require.NotNil(t, controller)
117117
assert.Equal(t, consumer.TopicKeyBuild, controller.TopicKey())
@@ -124,7 +124,7 @@ func TestController_Process_Success(t *testing.T) {
124124

125125
batch := testBatch()
126126
store := newMockStorage(ctrl, batch)
127-
controller := newTestController(t, ctrl, store, buildnoop.New(), nil)
127+
controller := newTestController(t, ctrl, store, buildfake.New(), nil)
128128

129129
msg := entityqueue.NewMessage(batch.ID, batchIDPayload(t, batch.ID), batch.Queue, nil)
130130
delivery := queuemock.NewMockDelivery(ctrl)
@@ -315,7 +315,7 @@ func TestController_Process_StorageFailure(t *testing.T) {
315315
store.EXPECT().GetRequestStore().Return(storagemock.NewMockRequestStore(ctrl)).AnyTimes()
316316
store.EXPECT().GetBuildStore().Return(storagemock.NewMockBuildStore(ctrl)).AnyTimes()
317317

318-
controller := newTestController(t, ctrl, store, buildnoop.New(), nil)
318+
controller := newTestController(t, ctrl, store, buildfake.New(), nil)
319319

320320
msg := entityqueue.NewMessage("test-queue/batch/1", batchIDPayload(t, "test-queue/batch/1"), "test-queue", nil)
321321
delivery := queuemock.NewMockDelivery(ctrl)
@@ -332,7 +332,7 @@ func TestController_Process_PublishFailure(t *testing.T) {
332332

333333
batch := testBatch()
334334
store := newMockStorage(ctrl, batch)
335-
controller := newTestController(t, ctrl, store, buildnoop.New(), fmt.Errorf("publish failed"))
335+
controller := newTestController(t, ctrl, store, buildfake.New(), fmt.Errorf("publish failed"))
336336

337337
msg := entityqueue.NewMessage(batch.ID, batchIDPayload(t, batch.ID), batch.Queue, nil)
338338
delivery := queuemock.NewMockDelivery(ctrl)
@@ -347,7 +347,7 @@ func TestController_InterfaceImplementation(t *testing.T) {
347347
ctrl := gomock.NewController(t)
348348
batch := testBatch()
349349
store := newMockStorage(ctrl, batch)
350-
controller := newTestController(t, ctrl, store, buildnoop.New(), nil)
350+
controller := newTestController(t, ctrl, store, buildfake.New(), nil)
351351

352352
var _ consumer.Controller = controller
353353
}

0 commit comments

Comments
 (0)