refactor: lift resolveEffectiveConfig out of root main.go into configresolve/#511
Open
fuziontech wants to merge 2 commits intomainfrom
Open
refactor: lift resolveEffectiveConfig out of root main.go into configresolve/#511fuziontech wants to merge 2 commits intomainfrom
fuziontech wants to merge 2 commits intomainfrom
Conversation
…resolve/ Moves the ~940-line precedence-aware config merger (CLI > env > YAML > defaults) and its CLIInputs / Resolved types out of the all-in-one duckgres binary's package into a new configresolve/ package. This is the last piece needed before cmd/duckgres-controlplane and cmd/duckgres-worker can move past their config-load stub and actually boot — they can now import and call configresolve.ResolveEffective(...) instead of having to duplicate the resolution logic. Why a new package and not configloader/: configloader is intentionally light (YAML schema + LoadFile only, ~30 lines per file, no deps on server/controlplane). configresolve is heavy by design (depends on server, controlplane, server/ducklake) because it produces fully wired runtime configs. Mixing the two would force everything that just wants the YAML schema to pull in the heavy graph too. Naming changes (all renames mechanical, no logic touched): - configCLIInputs → configresolve.CLIInputs - resolvedConfig → configresolve.Resolved - defaultServerConfig → configresolve.DefaultServerConfig - resolveEffectiveConfig → configresolve.ResolveEffective - splitAndTrim, intPtr, boolPtr stay package-private (unexported) main.go's existing FileConfig type alias to configloader.FileConfig is unchanged; ResolveEffective takes *configloader.FileConfig directly so callers don't need the alias. Verified: - go build ./... + go build -tags kubernetes ./... both clean - go test ./configresolve/... passes both with and without -tags kubernetes - go list -deps ./cmd/duckgres-controlplane confirms libduckdb still NOT pulled in (default tags AND -tags kubernetes) The follow-up wiring of cmd/duckgres-controlplane and cmd/duckgres-worker to actually call configresolve.ResolveEffective lands in a separate PR since each binary needs its own flag.* declarations + cliSet capture shape; doing both here would balloon this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
main_test.go is in `package main` so it referenced the moved symbols unqualified (resolveEffectiveConfig, configCLIInputs). go test ./ compiles the binary's own test file, which my local sanity runs missed because I tested ./... which does NOT include the root package's tests when run from a deeper directory in some shells. Lint and unit-tests caught it on PR #511's CI. 49 references updated to configresolve.ResolveEffective / configresolve.CLIInputs; configresolve import added; FileConfig type alias kept (still used for the flag-binding code). golangci-lint clean locally now. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Lifts the ~940-line precedence-aware config merger (CLI > env > YAML > defaults) and its
CLIInputs/Resolvedtypes out of the all-in-one duckgres binary's package into a newconfigresolve/package. This is the last piece needed beforecmd/duckgres-controlplaneandcmd/duckgres-workercan move past their config-load stub and actually boot — they can now import and callconfigresolve.ResolveEffective(...)directly instead of having to duplicate the resolution logic.Why a new package and not
configloader/?configloaderis intentionally light (YAML schema +LoadFileonly, no deps onserver/controlplane).configresolveis heavy by design (pulls inserver,controlplane,server/ducklake) because it produces fully-wired runtime configs. Mixing the two would force everything that just wants the YAML schema to pull in the heavy graph.Changes
Mechanical rename + move; no logic touched:
config_resolution.go→configresolve/resolve.go(95% rename similarity)config_resolution_k8s_test.go→configresolve/resolve_k8s_test.go(56%, package + symbol rename)main.goupdated: importsconfigresolve, callsconfigresolve.ResolveEffective(...)withconfigresolve.CLIInputs{...}Symbol renames:
configCLIInputs→configresolve.CLIInputsresolvedConfig→configresolve.ResolveddefaultServerConfig→configresolve.DefaultServerConfigresolveEffectiveConfig→configresolve.ResolveEffectivesplitAndTrim,intPtr,boolPtrstay package-privateFunction signature switched to take
*configloader.FileConfigdirectly (was*FileConfigrelying on a main-package type alias).Test plan
go build ./...cleango build -tags kubernetes ./...cleango test ./configresolve/...passesgo test -tags kubernetes ./configresolve/...passesgo list -deps ./cmd/duckgres-controlplaneconfirms libduckdb still NOT in the import graph (default and-tags kubernetes)Follow-up
Wiring of
cmd/duckgres-controlplaneandcmd/duckgres-workerto actually callconfigresolve.ResolveEffectiveis a separate PR — each binary needs its ownflag.*declarations +cliSetcapture shape; doing both here would balloon this PR. With this PR landed, that follow-up is mechanical.🤖 Generated with Claude Code