Skip to content

Commit 484dbcd

Browse files
committed
feat(conflict): Rename and iterate the interface contract
1 parent b06bb06 commit 484dbcd

4 files changed

Lines changed: 40 additions & 23 deletions

File tree

submitqueue/extension/dependency/conflict/tango/BUILD.bazel renamed to submitqueue/extension/dependency/conflict/targetoverlap/BUILD.bazel

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ load("@rules_go//go:def.bzl", "go_library", "go_test")
22

33
go_library(
44
name = "go_default_library",
5-
srcs = ["tango.go"],
6-
importpath = "github.com/uber/submitqueue/submitqueue/extension/dependency/conflict/tango",
5+
srcs = ["targetoverlap.go"],
6+
importpath = "github.com/uber/submitqueue/submitqueue/extension/dependency/conflict/targetoverlap",
77
visibility = ["//visibility:public"],
88
deps = [
99
"//submitqueue/entity:go_default_library",
@@ -14,11 +14,12 @@ go_library(
1414

1515
go_test(
1616
name = "go_default_test",
17-
srcs = ["tango_test.go"],
17+
srcs = ["targetoverlap_test.go"],
1818
embed = [":go_default_library"],
1919
deps = [
2020
"//submitqueue/entity:go_default_library",
2121
"//submitqueue/extension/conflict:go_default_library",
22+
"//submitqueue/extension/dependency/resolver:go_default_library",
2223
"@com_github_stretchr_testify//assert:go_default_library",
2324
"@com_github_stretchr_testify//require:go_default_library",
2425
],

submitqueue/extension/dependency/conflict/tango/tango.go renamed to submitqueue/extension/dependency/conflict/targetoverlap/targetoverlap.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
// Package tango provides a conflict.Analyzer that reports a conflict between
16-
// two batches when their changed build targets overlap. The targets a batch
17-
// affects are resolved through an injected resolver.TargetResolver, whose
18-
// production implementation calls the Tango service.
19-
package tango
15+
// Package targetoverlap provides a conflict.Analyzer that reports a conflict
16+
// between two batches when their changed build targets overlap. The targets a
17+
// batch affects are resolved through an injected resolver.TargetResolver,
18+
// keeping the analyzer independent of any particular target-resolution backend.
19+
package targetoverlap
2020

2121
import (
2222
"context"
@@ -52,7 +52,7 @@ func (a *analyzer) Analyze(ctx context.Context, batch entity.Batch, inFlight []e
5252

5353
// TODO: when TargetResolver fails, fall back to a queue-configured
5454
// analyzer (all or none) instead of propagating the error. The queue config
55-
// decides whether a Tango outage over-serializes (all) or maximizes
55+
// decides whether a resolver outage over-serializes (all) or maximizes
5656
// parallelism (none).
5757
candidate, err := a.resolve(ctx, batch)
5858
if err != nil {
@@ -87,7 +87,7 @@ func (a *analyzer) resolve(ctx context.Context, batch entity.Batch) (map[string]
8787

8888
keys := make(map[string]struct{}, len(targets))
8989
for _, t := range targets {
90-
keys[t] = struct{}{}
90+
keys[t.Name] = struct{}{}
9191
}
9292
return keys, nil
9393
}

submitqueue/extension/dependency/conflict/tango/tango_test.go renamed to submitqueue/extension/dependency/conflict/targetoverlap/targetoverlap_test.go

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
package tango
15+
package targetoverlap
1616

1717
import (
1818
"context"
@@ -24,6 +24,7 @@ import (
2424

2525
"github.com/uber/submitqueue/submitqueue/entity"
2626
"github.com/uber/submitqueue/submitqueue/extension/conflict"
27+
"github.com/uber/submitqueue/submitqueue/extension/dependency/resolver"
2728
)
2829

2930
// fakeResolver is an in-test TargetResolver that returns pre-configured target
@@ -47,11 +48,16 @@ func (f *fakeResolver) failWith(err error) *fakeResolver {
4748
return f
4849
}
4950

50-
func (f *fakeResolver) ChangedTargets(_ context.Context, batch entity.Batch) ([]string, error) {
51+
func (f *fakeResolver) ChangedTargets(_ context.Context, batch entity.Batch) ([]resolver.Target, error) {
5152
if f.err != nil {
5253
return nil, f.err
5354
}
54-
return f.targets[batch.ID], nil
55+
names := f.targets[batch.ID]
56+
targets := make([]resolver.Target, len(names))
57+
for i, n := range names {
58+
targets[i] = resolver.Target{Name: n}
59+
}
60+
return targets, nil
5561
}
5662

5763
func cfg() conflict.Config {
@@ -123,14 +129,14 @@ func TestAnalyze(t *testing.T) {
123129

124130
for _, tt := range tests {
125131
t.Run(tt.name, func(t *testing.T) {
126-
resolver := newFakeResolver().set(tt.candidate, tt.candTargets...)
132+
r := newFakeResolver().set(tt.candidate, tt.candTargets...)
127133
inFlight := make([]entity.Batch, 0, len(tt.inFlight))
128134
for _, f := range tt.inFlight {
129-
resolver.set(f.id, f.targets...)
135+
r.set(f.id, f.targets...)
130136
inFlight = append(inFlight, entity.Batch{ID: f.id})
131137
}
132138

133-
got, err := New(cfg(), resolver).Analyze(context.Background(), entity.Batch{ID: tt.candidate}, inFlight)
139+
got, err := New(cfg(), r).Analyze(context.Background(), entity.Batch{ID: tt.candidate}, inFlight)
134140
require.NoError(t, err)
135141

136142
var ids []string
@@ -150,17 +156,17 @@ func TestAnalyze_EmptyInFlight(t *testing.T) {
150156
}
151157

152158
func TestAnalyze_ResolverError(t *testing.T) {
153-
sentinel := errors.New("tango unavailable")
159+
sentinel := errors.New("resolver unavailable")
154160

155161
t.Run("candidate resolution fails", func(t *testing.T) {
156-
resolver := newFakeResolver().failWith(sentinel)
157-
_, err := New(cfg(), resolver).Analyze(context.Background(), entity.Batch{ID: "cand"}, []entity.Batch{{ID: "x"}})
162+
r := newFakeResolver().failWith(sentinel)
163+
_, err := New(cfg(), r).Analyze(context.Background(), entity.Batch{ID: "cand"}, []entity.Batch{{ID: "x"}})
158164
require.ErrorIs(t, err, sentinel)
159165
})
160166

161167
t.Run("in-flight resolution fails", func(t *testing.T) {
162-
resolver := newFakeResolver().set("cand", "//foo:lib").failWith(sentinel)
163-
_, err := New(cfg(), resolver).Analyze(context.Background(), entity.Batch{ID: "cand"}, []entity.Batch{{ID: "x"}})
168+
r := newFakeResolver().set("cand", "//foo:lib").failWith(sentinel)
169+
_, err := New(cfg(), r).Analyze(context.Background(), entity.Batch{ID: "cand"}, []entity.Batch{{ID: "x"}})
164170
require.ErrorIs(t, err, sentinel)
165171
})
166172
}

submitqueue/extension/dependency/resolver/targetresolver.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
// of build targets a batch affects. The interface is deliberately free of
1717
// Tango wire types so that each deployment can provide its own adapter against
1818
// whatever proto import path its monorepo uses — the analyzer sees only batch
19-
// identity in and target names out.
19+
// identity in and targets out.
2020
package resolver
2121

2222
import (
@@ -25,9 +25,19 @@ import (
2525
"github.com/uber/submitqueue/submitqueue/entity"
2626
)
2727

28+
// Target is a build target a batch affects.
29+
type Target struct {
30+
// Name identifies the target (e.g. "//service/foo:lib").
31+
Name string
32+
// Attributes carries backend-specific metadata the analyzer does not
33+
// interpret today. Future consumers (e.g. conflict relaxation) can read
34+
// keys like "distance" or "rule_type" without an interface change.
35+
Attributes map[string]string
36+
}
37+
2838
// TargetResolver resolves the set of build targets a batch affects. The
2939
// production implementation translates the batch's changes into a Tango
3040
// GetChangedTargets call; tests supply a fake.
3141
type TargetResolver interface {
32-
ChangedTargets(ctx context.Context, batch entity.Batch) ([]string, error)
42+
ChangedTargets(ctx context.Context, batch entity.Batch) ([]Target, error)
3343
}

0 commit comments

Comments
 (0)