From 578c5ef25d43c733722c1dd1c272dc00502ed993 Mon Sep 17 00:00:00 2001 From: Yushan Lin Date: Sun, 12 Jul 2026 17:27:27 -0700 Subject: [PATCH 1/5] config: remove fields not documented in config/README.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Intent: - This branch builds on #190 (config: update fields, defaults, and required checks to match README), which aligned names/defaults/required-ness with config/README.md but left several fields that the README doesn't document at all. This removes them so the Go structs match the documented surface exactly. Changes: - Removed RepositoryConfig.FullHashRepos, ExcludedFiles, ExcludeExternalTargets, and StreamBazelLogs, and ServiceConfig.WorkerRootPath. - These fields drove real behavior, so this is a narrow behavior change, not just a doc cleanup: graphrunner/native.go now always includes external targets in the Bazel query and no longer applies a repo-level full-hash or excluded-file list (per-request excludes via RequestOptions still work), orchestrator/native_orchestrator.go no longer streams Bazel logs, and example/main.go derives the worker checkout path as workspaces_root/.workers instead of reading a separate config field. - Updated config/config_test.go and the YAML fixtures (example/tango-config.yaml, orchestrator/testdata/config.yaml, integration/testdata/tango-config.yaml.tmpl) to drop the removed keys. --- Generated by the 🪄 [pr-create](https://sg.uberinternal.com/code.uber.internal/uber-code/devexp-agent-marketplace/-/blob/claude-code/plugins/dev/uber-dev/skills/pr-create/SKILL.md) skill in devexp-agent-marketplace --- config/config.go | 4 ---- config/config_test.go | 21 +-------------------- config/repository_config.go | 13 ++----------- config/service_config.go | 7 ++----- example/main.go | 2 +- example/tango-config.yaml | 7 ------- graphrunner/native.go | 6 +----- integration/testdata/tango-config.yaml.tmpl | 4 ---- orchestrator/native_orchestrator.go | 1 - orchestrator/testdata/config.yaml | 6 ------ 10 files changed, 7 insertions(+), 64 deletions(-) diff --git a/config/config.go b/config/config.go index 58212bce..73da86f4 100644 --- a/config/config.go +++ b/config/config.go @@ -17,7 +17,6 @@ package config import ( "fmt" "os" - "path/filepath" yaml "github.com/goccy/go-yaml" ) @@ -72,9 +71,6 @@ func Parse(configFilePath string) (*Config, error) { if config.Service.WorkspacesRoot == "" { return nil, fmt.Errorf("service.workspaces_root must be set") } - if config.Service.WorkerRootPath == "" { - config.Service.WorkerRootPath = filepath.Join(config.Service.WorkspacesRoot, ".workers") - } if config.Service.MaxWorkerPoolSize <= 0 { return nil, fmt.Errorf("service.max_worker_pool_size must be > 0, got %d", config.Service.MaxWorkerPoolSize) } diff --git a/config/config_test.go b/config/config_test.go index 2b00edb4..7f7f7914 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -74,33 +74,16 @@ func TestParse_ServiceDefaults(t *testing.T) { tests := []struct { name string give string - wantWorkerRootPath string wantMaxNumTargets int wantMaxNumChangedTargets int wantMaxNumMetadataEntries int }{ { - name: "worker_root_path and streaming default when unset", + name: "streaming defaults when unset", give: _baseServiceYAML + ` repository: - remote: "r1" `, - wantWorkerRootPath: filepath.Join("/tmp/x", ".workers"), - wantMaxNumTargets: _defaultMaxNumTargets, - wantMaxNumChangedTargets: _defaultMaxNumChangedTargets, - wantMaxNumMetadataEntries: _defaultMaxNumMetadataEntries, - }, - { - name: "worker_root_path explicit value preserved", - give: ` -service: - workspaces_root: "/tmp/x" - worker_root_path: "/tmp/custom-workers" - max_worker_pool_size: 1 -repository: - - remote: "r1" -`, - wantWorkerRootPath: "/tmp/custom-workers", wantMaxNumTargets: _defaultMaxNumTargets, wantMaxNumChangedTargets: _defaultMaxNumChangedTargets, wantMaxNumMetadataEntries: _defaultMaxNumMetadataEntries, @@ -115,7 +98,6 @@ repository: repository: - remote: "r1" `, - wantWorkerRootPath: filepath.Join("/tmp/x", ".workers"), wantMaxNumTargets: 10, wantMaxNumChangedTargets: 20, wantMaxNumMetadataEntries: 30, @@ -126,7 +108,6 @@ repository: t.Run(tt.name, func(t *testing.T) { cfg, err := Parse(writeConfig(t, tt.give)) require.NoError(t, err) - assert.Equal(t, tt.wantWorkerRootPath, cfg.Service.WorkerRootPath) assert.Equal(t, tt.wantMaxNumTargets, cfg.Service.Streaming.MaxNumTargets) assert.Equal(t, tt.wantMaxNumChangedTargets, cfg.Service.Streaming.MaxNumChangedTargets) assert.Equal(t, tt.wantMaxNumMetadataEntries, cfg.Service.Streaming.MaxNumMetadataEntries) diff --git a/config/repository_config.go b/config/repository_config.go index 5d1f56ef..1ba6992d 100644 --- a/config/repository_config.go +++ b/config/repository_config.go @@ -21,12 +21,6 @@ type RepositoryConfig struct { // unique across all entries and match exactly what clients send in // BuildDescription.remote. Remote string `yaml:"remote"` - // TODO: FullHashRepos, ExcludedFiles, and ExcludeExternalTargets are not - // documented in config/README.md. Delete them if they turn out to be unneeded, - // otherwise document them there. - FullHashRepos []string `yaml:"full_hash_repos"` - ExcludedFiles []string `yaml:"excluded_files"` - ExcludeExternalTargets bool `yaml:"exclude_external_targets"` // BzlmodEnabled indicates whether this repository uses Bzlmod for external // dependency management. Defaults to true if unset. Set to false only for // repositories still using WORKSPACE. @@ -34,14 +28,11 @@ type RepositoryConfig struct { // BazelCommand overrides the Bazel binary path. When empty, Tango // automatically downloads and caches Bazelisk from GitHub. BazelCommand string `yaml:"bazel_command"` - // QueryTimeoutSeconds is the Bazel query timeout in seconds. Defaults to 600. - QueryTimeoutSeconds int64 `yaml:"query_timeout_seconds"` // BazelExtraArgs are extra arguments passed to `bazel query` invocations, // inserted between the `query` subcommand and the query expression. BazelExtraArgs []string `yaml:"bazel_extra_args"` - // TODO: StreamBazelLogs is not documented in config/README.md. Delete it if - // it turns out to be unneeded, otherwise document it there. - StreamBazelLogs bool `yaml:"stream_bazel_logs"` + // QueryTimeoutSeconds is the Bazel query timeout in seconds. Defaults to 600. + QueryTimeoutSeconds int64 `yaml:"query_timeout_seconds"` } // RepositoryConfigProvider looks up per-repository configuration by remote. diff --git a/config/service_config.go b/config/service_config.go index ef150396..b5cc07d4 100644 --- a/config/service_config.go +++ b/config/service_config.go @@ -24,11 +24,8 @@ type ServiceConfig struct { // and worker checkouts. Required. Layout: // for // origin clones and /.workers//worker-{1..N}/ for // worker checkouts. - WorkspacesRoot string `yaml:"workspaces_root"` - // TODO: WorkerRootPath is not documented in config/README.md. Delete it if - // it turns out to be unneeded, otherwise document it there. - WorkerRootPath string `yaml:"worker_root_path"` // root directory for worker workspace checkouts; defaults to workspaces_root/.workers - Streaming ChunkConfig `yaml:"streaming"` // streaming chunk sizes; zero values fall back to package defaults + WorkspacesRoot string `yaml:"workspaces_root"` + Streaming ChunkConfig `yaml:"streaming"` // streaming chunk sizes; zero values fall back to package defaults } // ChunkConfig controls the number of entries per gRPC stream message. diff --git a/example/main.go b/example/main.go index 906aed60..462a690c 100644 --- a/example/main.go +++ b/example/main.go @@ -72,7 +72,7 @@ func run() error { // Repo manager and orchestrator repoManagerClonePath := cfg.Service.WorkspacesRoot - workerRootPath := cfg.Service.WorkerRootPath + workerRootPath := filepath.Join(repoManagerClonePath, ".workers") if err := os.MkdirAll(repoManagerClonePath, 0o755); err != nil { return fmt.Errorf("failed to create repo manager clone path: %w", err) } diff --git a/example/tango-config.yaml b/example/tango-config.yaml index cd0fd3a1..0941a8af 100644 --- a/example/tango-config.yaml +++ b/example/tango-config.yaml @@ -9,11 +9,6 @@ storage: # Repository configuration repository: - remote: "https://github.com/uber/tango.git" - full_hash_repos: - - "" - excluded_files: - - "^@@?bazel_tools/" - exclude_external_targets: true bzlmod_enabled: true query_timeout_seconds: 300 @@ -22,5 +17,3 @@ service: max_worker_pool_size: 5 # root for origin clones; required workspaces_root: "/tmp/tango-repo-manager" - # root for worker checkouts; defaults to workspaces_root/.workers - worker_root_path: "/tmp/tango/workers" diff --git a/graphrunner/native.go b/graphrunner/native.go index 0eb1c0af..22ba3664 100644 --- a/graphrunner/native.go +++ b/graphrunner/native.go @@ -59,9 +59,6 @@ func NewNativeGraphRunner(p NativeGraphRunnerParams) GraphRunner { func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace) (targethasher.Result, error) { query := "//external:all-targets + deps(//...:all-targets)" - if g.config.ExcludeExternalTargets { - query = "deps(//...:all-targets)" - } additionalArgs := append( []string{"--order_output=no", "--proto:locations", "--noproto:default_values"}, g.config.BazelExtraArgs..., @@ -92,8 +89,7 @@ func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace) hashConfig := targethasher.HashConfig{ KnownSourceHashes: knownSourceHashes, - FullHashRepos: g.config.FullHashRepos, - ExcludedRegex: append(g.config.ExcludedFiles, g.extraExcludedFiles...), + ExcludedRegex: g.extraExcludedFiles, UseBzlmod: g.config.BzlmodEnabled == nil || *g.config.BzlmodEnabled, } diff --git a/integration/testdata/tango-config.yaml.tmpl b/integration/testdata/tango-config.yaml.tmpl index 186ba9df..be8dd20b 100644 --- a/integration/testdata/tango-config.yaml.tmpl +++ b/integration/testdata/tango-config.yaml.tmpl @@ -7,13 +7,9 @@ repository: {{- if .BazelCommand}} bazel_command: {{.BazelCommand}} {{- end}} - full_hash_repos: [""] - excluded_files: ["^@@?bazel_tools/"] - exclude_external_targets: true bzlmod_enabled: true query_timeout_seconds: 600 service: max_worker_pool_size: 2 workspaces_root: {{.ClonePath}} - worker_root_path: {{.WorkerPath}} diff --git a/orchestrator/native_orchestrator.go b/orchestrator/native_orchestrator.go index 47062c59..1c86d9d4 100644 --- a/orchestrator/native_orchestrator.go +++ b/orchestrator/native_orchestrator.go @@ -191,7 +191,6 @@ func (b *nativeOrchestrator) GetTargetGraph(ctx context.Context, req entity.GetT Logger: b.logger, BazelCommand: repoCfg.BazelCommand, QueryTimeout: time.Duration(repoCfg.QueryTimeoutSeconds) * time.Second, - StreamLogs: repoCfg.StreamBazelLogs, }) if err != nil { return nil, fmt.Errorf("create bazel client: %w", err) diff --git a/orchestrator/testdata/config.yaml b/orchestrator/testdata/config.yaml index 5a9483db..e3432a86 100644 --- a/orchestrator/testdata/config.yaml +++ b/orchestrator/testdata/config.yaml @@ -1,11 +1,5 @@ repository: - remote: "git@github:uber/tango" - full_hash_repos: - - "//" - - "//external" - excluded_files: - - "*.gen.go" - exclude_external_targets: true bzlmod_enabled: true query_timeout_seconds: 15 From 36a431f3ce397f0b4942bc70298a328cb1be1f94 Mon Sep 17 00:00:00 2001 From: Yushan Lin Date: Sun, 12 Jul 2026 17:35:20 -0700 Subject: [PATCH 2/5] config: add ServiceConfig.WorkerRootPath() to guard documented layout The /.workers derivation moved from a config.Parse default into example/main.go when ServiceConfig.WorkerRootPath was removed, but it was untested there (package main). Move it into config as a small method with its own test, so the layout documented on WorkspacesRoot can't drift silently again. --- config/config_test.go | 7 +++++++ config/service_config.go | 8 ++++++++ example/main.go | 2 +- 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/config/config_test.go b/config/config_test.go index 7f7f7914..28715b93 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -70,6 +70,13 @@ repository: } } +func TestServiceConfig_WorkerRootPath(t *testing.T) { + // Guards the layout documented on ServiceConfig.WorkspacesRoot: + // /.workers for worker checkouts. + svc := ServiceConfig{WorkspacesRoot: "/tmp/x"} + assert.Equal(t, filepath.Join("/tmp/x", ".workers"), svc.WorkerRootPath()) +} + func TestParse_ServiceDefaults(t *testing.T) { tests := []struct { name string diff --git a/config/service_config.go b/config/service_config.go index b5cc07d4..888522d2 100644 --- a/config/service_config.go +++ b/config/service_config.go @@ -14,6 +14,8 @@ package config +import "path/filepath" + // ServiceConfig holds operational configuration for the Tango service. type ServiceConfig struct { // MaxWorkerPoolSize is the max number of concurrent requests per repository. @@ -28,6 +30,12 @@ type ServiceConfig struct { Streaming ChunkConfig `yaml:"streaming"` // streaming chunk sizes; zero values fall back to package defaults } +// WorkerRootPath returns the root directory for worker workspace checkouts, +// as documented on WorkspacesRoot: /.workers. +func (s ServiceConfig) WorkerRootPath() string { + return filepath.Join(s.WorkspacesRoot, ".workers") +} + // ChunkConfig controls the number of entries per gRPC stream message. // All fields are optional; a zero value means "use the package default". // Tune these when a monorepo's per-target size causes messages to approach diff --git a/example/main.go b/example/main.go index 462a690c..e805a36b 100644 --- a/example/main.go +++ b/example/main.go @@ -72,7 +72,7 @@ func run() error { // Repo manager and orchestrator repoManagerClonePath := cfg.Service.WorkspacesRoot - workerRootPath := filepath.Join(repoManagerClonePath, ".workers") + workerRootPath := cfg.Service.WorkerRootPath() if err := os.MkdirAll(repoManagerClonePath, 0o755); err != nil { return fmt.Errorf("failed to create repo manager clone path: %w", err) } From 9cb590893fc8bc66e3e6ce19d9c2ea9b48a8685f Mon Sep 17 00:00:00 2001 From: Yushan Lin Date: Sun, 12 Jul 2026 18:46:22 -0700 Subject: [PATCH 3/5] config: compute WorkerRootPath in Parse instead of a method ServiceConfig.WorkerRootPath() recomputed the join on every call and exposed a public method just to derive a path. Set it once as a plain field during Parse instead, so every config initialization gets a consistent, precomputed value without callers needing to call a method. --- config/config.go | 2 ++ config/config_test.go | 11 ++++------- config/service_config.go | 10 +++------- example/main.go | 2 +- 4 files changed, 10 insertions(+), 15 deletions(-) diff --git a/config/config.go b/config/config.go index 73da86f4..244d475d 100644 --- a/config/config.go +++ b/config/config.go @@ -17,6 +17,7 @@ package config import ( "fmt" "os" + "path/filepath" yaml "github.com/goccy/go-yaml" ) @@ -74,6 +75,7 @@ func Parse(configFilePath string) (*Config, error) { if config.Service.MaxWorkerPoolSize <= 0 { return nil, fmt.Errorf("service.max_worker_pool_size must be > 0, got %d", config.Service.MaxWorkerPoolSize) } + config.Service.WorkerRootPath = filepath.Join(config.Service.WorkspacesRoot, ".workers") if config.Service.Streaming.MaxNumTargets <= 0 { config.Service.Streaming.MaxNumTargets = _defaultMaxNumTargets } diff --git a/config/config_test.go b/config/config_test.go index 28715b93..ca0e798e 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -70,17 +70,11 @@ repository: } } -func TestServiceConfig_WorkerRootPath(t *testing.T) { - // Guards the layout documented on ServiceConfig.WorkspacesRoot: - // /.workers for worker checkouts. - svc := ServiceConfig{WorkspacesRoot: "/tmp/x"} - assert.Equal(t, filepath.Join("/tmp/x", ".workers"), svc.WorkerRootPath()) -} - func TestParse_ServiceDefaults(t *testing.T) { tests := []struct { name string give string + wantWorkerRootPath string wantMaxNumTargets int wantMaxNumChangedTargets int wantMaxNumMetadataEntries int @@ -91,6 +85,7 @@ func TestParse_ServiceDefaults(t *testing.T) { repository: - remote: "r1" `, + wantWorkerRootPath: filepath.Join("/tmp/x", ".workers"), wantMaxNumTargets: _defaultMaxNumTargets, wantMaxNumChangedTargets: _defaultMaxNumChangedTargets, wantMaxNumMetadataEntries: _defaultMaxNumMetadataEntries, @@ -105,6 +100,7 @@ repository: repository: - remote: "r1" `, + wantWorkerRootPath: filepath.Join("/tmp/x", ".workers"), wantMaxNumTargets: 10, wantMaxNumChangedTargets: 20, wantMaxNumMetadataEntries: 30, @@ -115,6 +111,7 @@ repository: t.Run(tt.name, func(t *testing.T) { cfg, err := Parse(writeConfig(t, tt.give)) require.NoError(t, err) + assert.Equal(t, tt.wantWorkerRootPath, cfg.Service.WorkerRootPath) assert.Equal(t, tt.wantMaxNumTargets, cfg.Service.Streaming.MaxNumTargets) assert.Equal(t, tt.wantMaxNumChangedTargets, cfg.Service.Streaming.MaxNumChangedTargets) assert.Equal(t, tt.wantMaxNumMetadataEntries, cfg.Service.Streaming.MaxNumMetadataEntries) diff --git a/config/service_config.go b/config/service_config.go index 888522d2..5c0491e3 100644 --- a/config/service_config.go +++ b/config/service_config.go @@ -14,8 +14,6 @@ package config -import "path/filepath" - // ServiceConfig holds operational configuration for the Tango service. type ServiceConfig struct { // MaxWorkerPoolSize is the max number of concurrent requests per repository. @@ -28,12 +26,10 @@ type ServiceConfig struct { // worker checkouts. WorkspacesRoot string `yaml:"workspaces_root"` Streaming ChunkConfig `yaml:"streaming"` // streaming chunk sizes; zero values fall back to package defaults -} -// WorkerRootPath returns the root directory for worker workspace checkouts, -// as documented on WorkspacesRoot: /.workers. -func (s ServiceConfig) WorkerRootPath() string { - return filepath.Join(s.WorkspacesRoot, ".workers") + // WorkerRootPath is the root directory for worker workspace checkouts, + // set by Parse to /.workers. Not settable via YAML. + WorkerRootPath string `yaml:"-"` } // ChunkConfig controls the number of entries per gRPC stream message. diff --git a/example/main.go b/example/main.go index e805a36b..906aed60 100644 --- a/example/main.go +++ b/example/main.go @@ -72,7 +72,7 @@ func run() error { // Repo manager and orchestrator repoManagerClonePath := cfg.Service.WorkspacesRoot - workerRootPath := cfg.Service.WorkerRootPath() + workerRootPath := cfg.Service.WorkerRootPath if err := os.MkdirAll(repoManagerClonePath, 0o755); err != nil { return fmt.Errorf("failed to create repo manager clone path: %w", err) } From 27cc6f7251408332bf226bd8982082196e9d7000 Mon Sep 17 00:00:00 2001 From: Yushan Lin Date: Sun, 12 Jul 2026 23:10:01 -0700 Subject: [PATCH 4/5] fix(integration): repair make test-integration after config field removal Removing RepositoryConfig.ExcludeExternalTargets left the Bazel query always including //external:all-targets, which errors under Bzlmod ("no such package 'external'") since this repo's own config sets bzlmod_enabled: true. Gate the //external:all-targets clause on BzlmodEnabled instead, matching the behavior documented in config/README.md. Removing ExcludedFiles also dropped the "^@@?bazel_tools/" exclusion the integration test relied on to skip hashing @bazel_tools' files, some of which (e.g. tools/test/tw.exe, Windows-only) don't exist on every OS. Restore the equivalent exclusion via the per-request RequestOptions.ExtraExcludeFilesRegex, which still exists precisely for this kind of ad hoc exclusion. --- graphrunner/native.go | 12 ++++++++---- integration/integration_test.go | 11 +++++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/graphrunner/native.go b/graphrunner/native.go index 22ba3664..5cb415be 100644 --- a/graphrunner/native.go +++ b/graphrunner/native.go @@ -58,7 +58,13 @@ func NewNativeGraphRunner(p NativeGraphRunnerParams) GraphRunner { } func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace) (targethasher.Result, error) { - query := "//external:all-targets + deps(//...:all-targets)" + bzlmodEnabled := g.config.BzlmodEnabled == nil || *g.config.BzlmodEnabled + query := "deps(//...:all-targets)" + if !bzlmodEnabled { + // //external is only queryable under legacy WORKSPACE resolution; + // Bzlmod repos resolve external deps under @@ instead. + query = "//external:all-targets + " + query + } additionalArgs := append( []string{"--order_output=no", "--proto:locations", "--noproto:default_values"}, g.config.BazelExtraArgs..., @@ -71,8 +77,6 @@ func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace) // --proto:locations: we need to get external file location to make CTC more accurate // --noproto: parameters exclude fields from the output that are not used for hashing anyways, making // proto blob smaller and serialization/deserialization faster - // TODO: pass in --enable_workspace or --enable_bzlmod based on the config - AdditionalArgs: additionalArgs, }) g.scope.Timer("bazel_query_duration").Record(time.Since(bazelStart)) @@ -90,7 +94,7 @@ func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace) hashConfig := targethasher.HashConfig{ KnownSourceHashes: knownSourceHashes, ExcludedRegex: g.extraExcludedFiles, - UseBzlmod: g.config.BzlmodEnabled == nil || *g.config.BzlmodEnabled, + UseBzlmod: bzlmodEnabled, } hashStart := time.Now() diff --git a/integration/integration_test.go b/integration/integration_test.go index c3373596..8d4ca164 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -48,6 +48,11 @@ const ( configTemplateFile = "testdata/tango-config.yaml.tmpl" ) +// bazelToolsExcludeRegex excludes @bazel_tools's source files from disk-based hashing. +// It ships platform-specific files (e.g. tools/test/tw.exe, a Windows-only test wrapper) +// that don't exist on every OS, so stat-ing them to hash fails on machines missing them. +var bazelToolsExcludeRegex = []string{"^@@?bazel_tools/"} + func repoRemote(t *testing.T) string { t.Helper() remote := os.Getenv("TANGO_REPO_REMOTE") @@ -266,6 +271,9 @@ func getChangedTargets(t *testing.T, client pb.TangoYARPCClient, remote, firstSH Remote: remote, BaseSha: secondSHA, }, + RequestOptions: &pb.RequestOptions{ + ExtraExcludeFilesRegex: bazelToolsExcludeRegex, + }, }) require.NoError(t, err, "failed to initiate GetChangedTargets stream") @@ -361,6 +369,9 @@ func TestIntegration_GetTargetGraph(t *testing.T) { Remote: remote, BaseSha: pinnedSHA, }, + RequestOptions: &pb.RequestOptions{ + ExtraExcludeFilesRegex: bazelToolsExcludeRegex, + }, }) require.NoError(t, err, "failed to initiate GetTargetGraph stream") From abbc8c8a6a4d1530d6351f762a9b5401da0e0de8 Mon Sep 17 00:00:00 2001 From: Yushan Lin Date: Tue, 14 Jul 2026 13:39:12 -0700 Subject: [PATCH 5/5] config: add _path suffix to path-like fields (workspaces_root, bazel_command) Matches the existing convention elsewhere in ServiceConfig (repo_manager_clone_path, worker_root_path) and config/README.md's current documented field names: workspaces_root -> workspaces_root_path, bazel_command -> bazel_command_path. Pure rename, no behavior change. --- config/config.go | 6 +++--- config/config_test.go | 6 +++--- config/repository_config.go | 4 ++-- config/service_config.go | 12 ++++++------ example/main.go | 2 +- example/tango-config.yaml | 2 +- integration/testdata/tango-config.yaml.tmpl | 4 ++-- orchestrator/native_orchestrator.go | 2 +- orchestrator/testdata/config.yaml | 2 +- 9 files changed, 20 insertions(+), 20 deletions(-) diff --git a/config/config.go b/config/config.go index 244d475d..8a57ee2e 100644 --- a/config/config.go +++ b/config/config.go @@ -69,13 +69,13 @@ func Parse(configFilePath string) (*Config, error) { if config.Storage.Type == "" { config.Storage.Type = StorageTypeMemory } - if config.Service.WorkspacesRoot == "" { - return nil, fmt.Errorf("service.workspaces_root must be set") + if config.Service.WorkspacesRootPath == "" { + return nil, fmt.Errorf("service.workspaces_root_path must be set") } if config.Service.MaxWorkerPoolSize <= 0 { return nil, fmt.Errorf("service.max_worker_pool_size must be > 0, got %d", config.Service.MaxWorkerPoolSize) } - config.Service.WorkerRootPath = filepath.Join(config.Service.WorkspacesRoot, ".workers") + config.Service.WorkerRootPath = filepath.Join(config.Service.WorkspacesRootPath, ".workers") if config.Service.Streaming.MaxNumTargets <= 0 { config.Service.Streaming.MaxNumTargets = _defaultMaxNumTargets } diff --git a/config/config_test.go b/config/config_test.go index ca0e798e..25a2235c 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -25,7 +25,7 @@ import ( const _baseServiceYAML = ` service: - workspaces_root: "/tmp/x" + workspaces_root_path: "/tmp/x" max_worker_pool_size: 1 ` @@ -42,7 +42,7 @@ func TestParse_ServiceValidation(t *testing.T) { give string }{ { - name: "workspaces_root required", + name: "workspaces_root_path required", give: ` service: max_worker_pool_size: 1 @@ -54,7 +54,7 @@ repository: name: "max_worker_pool_size must be positive", give: ` service: - workspaces_root: "/tmp/x" + workspaces_root_path: "/tmp/x" max_worker_pool_size: 0 repository: - remote: "r1" diff --git a/config/repository_config.go b/config/repository_config.go index 1ba6992d..f7adbb4b 100644 --- a/config/repository_config.go +++ b/config/repository_config.go @@ -25,9 +25,9 @@ type RepositoryConfig struct { // dependency management. Defaults to true if unset. Set to false only for // repositories still using WORKSPACE. BzlmodEnabled *bool `yaml:"bzlmod_enabled"` - // BazelCommand overrides the Bazel binary path. When empty, Tango + // BazelCommandPath overrides the Bazel binary path. When empty, Tango // automatically downloads and caches Bazelisk from GitHub. - BazelCommand string `yaml:"bazel_command"` + BazelCommandPath string `yaml:"bazel_command_path"` // BazelExtraArgs are extra arguments passed to `bazel query` invocations, // inserted between the `query` subcommand and the query expression. BazelExtraArgs []string `yaml:"bazel_extra_args"` diff --git a/config/service_config.go b/config/service_config.go index 5c0491e3..41fbcbde 100644 --- a/config/service_config.go +++ b/config/service_config.go @@ -20,15 +20,15 @@ type ServiceConfig struct { // Each worker is a lightweight local clone (hardlinked to the origin, not a // full copy) that handles one request at a time. Must be greater than 0. MaxWorkerPoolSize int `yaml:"max_worker_pool_size"` - // WorkspacesRoot is the root directory where Tango stores repository clones - // and worker checkouts. Required. Layout: // for - // origin clones and /.workers//worker-{1..N}/ for + // WorkspacesRootPath is the root directory where Tango stores repository clones + // and worker checkouts. Required. Layout: // for + // origin clones and /.workers//worker-{1..N}/ for // worker checkouts. - WorkspacesRoot string `yaml:"workspaces_root"` - Streaming ChunkConfig `yaml:"streaming"` // streaming chunk sizes; zero values fall back to package defaults + WorkspacesRootPath string `yaml:"workspaces_root_path"` + Streaming ChunkConfig `yaml:"streaming"` // streaming chunk sizes; zero values fall back to package defaults // WorkerRootPath is the root directory for worker workspace checkouts, - // set by Parse to /.workers. Not settable via YAML. + // set by Parse to /.workers. Not settable via YAML. WorkerRootPath string `yaml:"-"` } diff --git a/example/main.go b/example/main.go index 906aed60..6aba8d0c 100644 --- a/example/main.go +++ b/example/main.go @@ -71,7 +71,7 @@ func run() error { logger.Infof("Using storage type: %s", cfg.Storage.Type) // Repo manager and orchestrator - repoManagerClonePath := cfg.Service.WorkspacesRoot + repoManagerClonePath := cfg.Service.WorkspacesRootPath workerRootPath := cfg.Service.WorkerRootPath if err := os.MkdirAll(repoManagerClonePath, 0o755); err != nil { return fmt.Errorf("failed to create repo manager clone path: %w", err) diff --git a/example/tango-config.yaml b/example/tango-config.yaml index 0941a8af..b453057b 100644 --- a/example/tango-config.yaml +++ b/example/tango-config.yaml @@ -16,4 +16,4 @@ repository: service: max_worker_pool_size: 5 # root for origin clones; required - workspaces_root: "/tmp/tango-repo-manager" + workspaces_root_path: "/tmp/tango-repo-manager" diff --git a/integration/testdata/tango-config.yaml.tmpl b/integration/testdata/tango-config.yaml.tmpl index be8dd20b..7edf0396 100644 --- a/integration/testdata/tango-config.yaml.tmpl +++ b/integration/testdata/tango-config.yaml.tmpl @@ -5,11 +5,11 @@ repository: - remote: {{.Remote}} default_branch: "main" {{- if .BazelCommand}} - bazel_command: {{.BazelCommand}} + bazel_command_path: {{.BazelCommand}} {{- end}} bzlmod_enabled: true query_timeout_seconds: 600 service: max_worker_pool_size: 2 - workspaces_root: {{.ClonePath}} + workspaces_root_path: {{.ClonePath}} diff --git a/orchestrator/native_orchestrator.go b/orchestrator/native_orchestrator.go index 1c86d9d4..3f9ab40a 100644 --- a/orchestrator/native_orchestrator.go +++ b/orchestrator/native_orchestrator.go @@ -189,7 +189,7 @@ func (b *nativeOrchestrator) GetTargetGraph(ctx context.Context, req entity.GetT client, err := bazel.NewBazelClient(ctx, bazel.Params{ WorkspacePath: ws.Path(), Logger: b.logger, - BazelCommand: repoCfg.BazelCommand, + BazelCommand: repoCfg.BazelCommandPath, QueryTimeout: time.Duration(repoCfg.QueryTimeoutSeconds) * time.Second, }) if err != nil { diff --git a/orchestrator/testdata/config.yaml b/orchestrator/testdata/config.yaml index e3432a86..93f44392 100644 --- a/orchestrator/testdata/config.yaml +++ b/orchestrator/testdata/config.yaml @@ -5,7 +5,7 @@ repository: service: max_worker_pool_size: 3 - workspaces_root: "/tmp/tango-repo-manager" + workspaces_root_path: "/tmp/tango-repo-manager" streaming: max_num_targets: 250 max_num_changed_targets: 125