AlexStocks commented on code in PR #3232:
URL: https://github.com/apache/dubbo-go/pull/3232#discussion_r2902473715
##########
client/options.go:
##########
@@ -585,35 +547,24 @@ func (cliOpts *ClientOptions) init(opts ...ClientOption)
error {
consumerConf := cliOpts.Consumer
- // init application
- application := cliOpts.Application
- if application != nil {
- cliOpts.applicationCompat = compatApplicationConfig(application)
- if err := cliOpts.applicationCompat.Init(); err != nil {
- return err
- }
- }
-
// init registries
regs := cliOpts.Registries
- if regs != nil {
- cliOpts.registriesCompat =
make(map[string]*config.RegistryConfig)
+ if len(regs) > 0 {
if len(consumerConf.RegistryIDs) <= 0 {
- consumerConf.RegistryIDs = make([]string, len(regs))
+ consumerConf.RegistryIDs = make([]string, 0, len(regs))
for key := range regs {
consumerConf.RegistryIDs =
append(consumerConf.RegistryIDs, key)
}
}
consumerConf.RegistryIDs =
commonCfg.TranslateIds(consumerConf.RegistryIDs)
+ newRegs := make(map[string]*global.RegistryConfig)
for _, id := range consumerConf.RegistryIDs {
if reg, ok := regs[id]; ok {
- cliOpts.registriesCompat[id] =
compatRegistryConfig(reg)
- if err := cliOpts.registriesCompat[id].Init();
err != nil {
- return err
- }
+ newRegs[id] = reg
}
}
+ cliOpts.Registries = newRegs
Review Comment:
`ClientOptions.init()` 直接把 `cliOpts.Registries` 替换为过滤后的新 map，原始 map
被永久丢弃。用户在外部持有同一个 `ClientOptions` 指针时，`init()` 之后看到的 `.Registries`
是过滤后的结果，与传入值不一致，调用方对此无感知。
建议：在文档注释中说明 `init()` 会修改此字段；或改为在 `dial()` 时做临时过滤，不修改 `cliOpts` 的持久状态。
##########
client/options.go:
##########
@@ -115,36 +103,23 @@ func (refOpts *ReferenceOptions) init(opts
...ReferenceOption) error {
}
// init registries
- // convert Registries to registriesCompat
if len(refOpts.Registries) > 0 {
- if refOpts.registriesCompat == nil {
- refOpts.registriesCompat =
make(map[string]*config.RegistryConfig)
- }
- for id, reg := range refOpts.Registries {
- refOpts.registriesCompat[id] = compatRegistryConfig(reg)
- if err := refOpts.registriesCompat[id].Init(); err !=
nil {
- return err
- }
- }
- }
-
- if len(refOpts.registriesCompat) > 0 {
- regs := refOpts.registriesCompat
+ regs := refOpts.Registries
if len(refConf.RegistryIDs) <= 0 {
- refConf.RegistryIDs = make([]string, len(regs))
+ refConf.RegistryIDs = make([]string, 0, len(regs))
for key := range regs {
refConf.RegistryIDs =
append(refConf.RegistryIDs, key)
}
}
refConf.RegistryIDs =
commonCfg.TranslateIds(refConf.RegistryIDs)
- newRegs := make(map[string]*config.RegistryConfig)
+ newRegs := make(map[string]*global.RegistryConfig)
for _, id := range refConf.RegistryIDs {
if reg, ok := regs[id]; ok {
newRegs[id] = reg
}
}
- refOpts.registriesCompat = newRegs
+ refOpts.Registries = newRegs
Review Comment:
`ReferenceOptions.init()` 基于 `refConf.RegistryIDs` 对已被 `ClientOptions`
过滤过一次的 map 再次过滤。若 `WithRegistryIDs` 指定的 ID 在第一次过滤后不存在，`newRegs` 为空，服务会静默走 URL
直连而不是注册中心，没有任何 error 或 warn，极难排查。
建议：`newRegs` 为空且原始 `regs` 非空时，应 return error 或至少打 warn 提示 registry ID 不存在。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
