config: split ServerOptions into CharonOptions and ProxyOptions#76
Merged
Conversation
…75) Replace the single config.ServerOptions with two focused types: CharonOptions – listen, storage (data dir, TTL, max-*, chain depth), worker interval, telemetry (service name + exporter URL) ProxyOptions – listen, inference backend/apikey/timeout, charon URL, telemetry (service name + exporter URL) Each type has its own New*, AddFlags, Complete, and Validate. The YAML config file structure (proxy.*, charon.*, telemetry.*) is unchanged so existing config files continue to work. Migrate cmd/charon and cmd/proxy to use their respective types. Remove the grab-bag ServerOptions and the TODO comment it left behind. Closes #74 Resolves TODO from #75 (config split task) Signed-off-by: Etai Lev Ran <elevran@gmail.com>
elevran
commented
Jul 8, 2026
elevran
left a comment
elevran
left a comment
Owner
Author
There was a problem hiding this comment.
Clean split of ServerOptions into focused CharonOptions and ProxyOptions. Field renames are consistent across cmd/, the config package, and the tests; the YAML structure is preserved so existing config files keep working. All 6 CI checks green.
A few small things worth addressing before merge — most are simplify/ponytail cleanups, plus one test-coverage gap.
- yagni:
fileProxyConfig.Enabled(line 21 ofinternal/config/options.go) is now dead. The oldServerOptions.Completeconsumedfc.Proxy.Enabledto driveProxyEnabled; that consumer is gone, so nothing reads this anymore. Drop the field (and anyapplyFileDefaults/docs references) to keep the YAML schema honest. - Docs note:
thoughts/shared/tech_debt_and_architecture_review_2026-07.mdstill referencesconfig.NewServerOptions()and theProxyEnabled/ProxyListen/InferenceTimeoutSecondsfields that this PR removes. Not blocking, but the doc will be stale once this lands.
…Validate, test coverage - Extract (*TelemetryOptions).AddFlags(fs) so --telemetry-exporter-url is registered once and called from both CharonOptions and ProxyOptions - Extract visitedFlags(fs) helper to remove duplicated map+Visit pattern from both Complete methods - Give CharonOptions.Validate() a real check: returns error when DataDir is empty - Add TestProxyOptionsBackendCLIOverridesFile and TestProxyOptionsCharonURLCLIOverridesFile to cover the merge semantics for --backend and --charon-url - Drop dead fileProxyConfig.Enabled field (its consumer, ServerOptions, is gone) Signed-off-by: Etai Lev Ran <elevran@gmail.com>
5 tasks
elevran
added a commit
that referenced
this pull request
Jul 8, 2026
* config: split ServerOptions into CharonOptions and ProxyOptions (#74, #75) Replace the single config.ServerOptions with two focused types: CharonOptions – listen, storage (data dir, TTL, max-*, chain depth), worker interval, telemetry (service name + exporter URL) ProxyOptions – listen, inference backend/apikey/timeout, charon URL, telemetry (service name + exporter URL) Each type has its own New*, AddFlags, Complete, and Validate. The YAML config file structure (proxy.*, charon.*, telemetry.*) is unchanged so existing config files continue to work. Migrate cmd/charon and cmd/proxy to use their respective types. Remove the grab-bag ServerOptions and the TODO comment it left behind. Closes #74 Resolves TODO from #75 (config split task) Signed-off-by: Etai Lev Ran <elevran@gmail.com> * config: extract TelemetryOptions.AddFlags, visitedFlags helper, real Validate, test coverage - Extract (*TelemetryOptions).AddFlags(fs) so --telemetry-exporter-url is registered once and called from both CharonOptions and ProxyOptions - Extract visitedFlags(fs) helper to remove duplicated map+Visit pattern from both Complete methods - Give CharonOptions.Validate() a real check: returns error when DataDir is empty - Add TestProxyOptionsBackendCLIOverridesFile and TestProxyOptionsCharonURLCLIOverridesFile to cover the merge semantics for --backend and --charon-url - Drop dead fileProxyConfig.Enabled field (its consumer, ServerOptions, is gone) Signed-off-by: Etai Lev Ran <elevran@gmail.com> --------- Signed-off-by: Etai Lev Ran <elevran@gmail.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
config.ServerOptionsgrab-bag with two focused types:CharonOptions— storage server config (listen, data dir, TTL, chain limits, worker interval, telemetry)ProxyOptions— proxy config (listen, inference backend/apikey/timeout, charon URL, telemetry)New*,AddFlags,Complete, andValidateproxy.*,charon.*,telemetry.*) is unchanged — existing config files keep workingcmd/charonandcmd/proxyto use their respective typesTODOcomment added in PR pkg restructure: pkg/charon, internal/server, cmd/proxy #71 about the config splitCloses
Closes #74
Part of #75 (resolves the config-split TODO item)
Test plan
make presubmitpassesgo test ./internal/config/...covers bothCharonOptionsandProxyOptionsdefaults, file loading, CLI override, and validation