Skip to content

Commit 84f8a72

Browse files
committed
feat(example): per-queue extension wiring; retire buildrunner/noop
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. - 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. - ✅ `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 3e0f012 commit 84f8a72

9 files changed

Lines changed: 244 additions & 243 deletions

File tree

CLAUDE.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,14 @@ 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 implementations only — not factories or routing.**
120+
121+
An `extension/{ext}` package contains the behavioral interface, its `Config`, the `Factory` *interface*, and impl constructors `New(...)` that return the interface. It must **not** contain `Factory` *implementations* (`NewFactory()` constructors or factory structs) or any queue-selection logic.
122+
123+
Why: an impl package (e.g. `scorer/heuristic`) can't know the queue topology or the other impls, so a "which impl for which queue" decision doesn't belong there. Per-queue routing — and the small adapters that wrap a `New(...)` impl in the `Factory` interface — live in the wiring layer (e.g. `example/{domain}/{service}/server/main.go`), the one place that knows the full queue set. That's where you route on `Config.QueueName`.
124+
125+
Rule of thumb: if you're about to add a `NewFactory()` or a `map[queue]impl` under `extension/`, it belongs in the wiring layer instead.
126+
119127
**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.
120128
121129
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
@@ -22,16 +22,23 @@ go_library(
2222
"//submitqueue/core/consumer",
2323
"//submitqueue/entity",
2424
"//submitqueue/extension/buildrunner",
25-
"//submitqueue/extension/buildrunner/noop",
25+
"//submitqueue/extension/buildrunner/fake",
2626
"//submitqueue/extension/changeprovider",
27+
"//submitqueue/extension/changeprovider/fake",
2728
"//submitqueue/extension/changeprovider/github",
2829
"//submitqueue/extension/conflict",
2930
"//submitqueue/extension/conflict/all",
31+
"//submitqueue/extension/conflict/fake",
32+
"//submitqueue/extension/conflict/none",
3033
"//submitqueue/extension/mergechecker",
34+
"//submitqueue/extension/mergechecker/fake",
3135
"//submitqueue/extension/mergechecker/github",
3236
"//submitqueue/extension/pusher",
37+
"//submitqueue/extension/pusher/fake",
3338
"//submitqueue/extension/pusher/git",
3439
"//submitqueue/extension/scorer",
40+
"//submitqueue/extension/scorer/composite",
41+
"//submitqueue/extension/scorer/fake",
3542
"//submitqueue/extension/scorer/heuristic",
3643
"//submitqueue/extension/storage",
3744
"//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)