refactor: remove grpc_server YAML key compatibility#155
Conversation
Gateway config now loads only the gateway_server key and no longer parses the deprecated grpc_server alias. Docs and tests are updated accordingly. Co-authored-by: Cursor <cursoragent@cursor.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the legacy grpc_server configuration key and its associated structures, fully transitioning to gateway_server. It also updates the documentation, configuration files, and tests accordingly. The review feedback highlights a potential compilation issue due to the modified signature of LoadConfig (which no longer accepts a logger), a potential bug regarding how boolean zero-values (like false for GRPCPassthrough) are merged, and a suggestion to rename a test to better reflect the updated logic.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| // LoadConfig reads gateway settings from the active funk config path, if any. | ||
| func LoadConfig(logger log.Logger) *Config { | ||
| func LoadConfig() *Config { |
There was a problem hiding this comment.
LoadConfig 的函数签名已被修改，移除了 logger 参数。请确保代码库中所有调用 LoadConfig 的地方（例如 cmds/grpcservercmd 或其他命令行入口）都已同步更新为新的签名，否则会导致编译失败。
| case loader.GatewayServer != nil: | ||
| func ResolveConfig(loader ConfigLoader) *Config { | ||
| if loader.GatewayServer != nil { | ||
| return merge.Struct(defaultCfg(), loader.GatewayServer).Unwrap() |
There was a problem hiding this comment.
在 ResolveConfig 中，使用 merge.Struct 将默认配置 defaultCfg() 与用户配置 loader.GatewayServer 进行合并。由于 GRPCPassthrough 是一个普通的 bool 类型（默认值为 true），如果合并库（merge.Struct）在合并时忽略了零值（即 false），那么用户在 YAML 中显式设置 grpc_passthrough: false 将无法生效（仍会保持默认的 true）。建议确认合并库的行为，或者将 GRPCPassthrough 改为指针类型 *bool 以支持显式的 false 配置。
| func TestResolveConfigPrefersGatewayServer(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| cfg := ResolveConfig(CombinedConfigLoader{ | ||
| cfg := ResolveConfig(ConfigLoader{ | ||
| GatewayServer: &Config{EnablePrintRouter: true}, | ||
| GrpcServer: &Config{EnablePrintRouter: false}, | ||
| }, nil) | ||
| if !cfg.EnablePrintRouter { | ||
| t.Fatal("expected gateway_server to win") | ||
| } | ||
| } | ||
|
|
||
| func TestResolveConfigLegacyGrpcServer(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| cfg := ResolveConfig(CombinedConfigLoader{ | ||
| GrpcServer: &Config{EnablePrintRouter: true}, | ||
| }, log.GetLogger("test")) | ||
| }) | ||
| if !cfg.EnablePrintRouter { | ||
| t.Fatal("expected legacy grpc_server config to apply") | ||
| t.Fatal("expected gateway_server config to apply") | ||
| } | ||
| } |
There was a problem hiding this comment.
由于旧的 grpc_server 键已被完全移除，ResolveConfig 不再包含多个键之间的优先级或偏好逻辑。测试名称 TestResolveConfigPrefersGatewayServer 建议重命名为更符合当前逻辑的名称，例如 TestResolveConfigAppliesGatewayServer。
| func TestResolveConfigPrefersGatewayServer(t *testing.T) { | |
| t.Parallel() | |
| cfg := ResolveConfig(CombinedConfigLoader{ | |
| cfg := ResolveConfig(ConfigLoader{ | |
| GatewayServer: &Config{EnablePrintRouter: true}, | |
| GrpcServer: &Config{EnablePrintRouter: false}, | |
| }, nil) | |
| if !cfg.EnablePrintRouter { | |
| t.Fatal("expected gateway_server to win") | |
| } | |
| } | |
| func TestResolveConfigLegacyGrpcServer(t *testing.T) { | |
| t.Parallel() | |
| cfg := ResolveConfig(CombinedConfigLoader{ | |
| GrpcServer: &Config{EnablePrintRouter: true}, | |
| }, log.GetLogger("test")) | |
| }) | |
| if !cfg.EnablePrintRouter { | |
| t.Fatal("expected legacy grpc_server config to apply") | |
| t.Fatal("expected gateway_server config to apply") | |
| } | |
| } | |
| func TestResolveConfigAppliesGatewayServer(t *testing.T) { | |
| t.Parallel() | |
| cfg := ResolveConfig(ConfigLoader{ | |
| GatewayServer: &Config{EnablePrintRouter: true}, | |
| }) | |
| if !cfg.EnablePrintRouter { | |
| t.Fatal("expected gateway_server config to apply") | |
| } | |
| } |
|
Superseded by #156 (consolidated pre-v2 legacy cleanup). |
1 participant
Summary
gatewayserver对 legacygrpc_serverYAML 键的解析逻辑LoadConfig仅加载gateway_server（ConfigLoader）Breaking change
grpc_server:的配置文件将不再生效；请改为gateway_server:Test plan
go test -short -race ./...Made with Cursor