You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2021/06/14 07:28:02 UTC
[GitHub] [dubbo-go-pixiu] williamfeng323 commented on a change in pull request #171: make Pixiu Admin config management finer-grained
williamfeng323 commented on a change in pull request #171:
URL: https://github.com/apache/dubbo-go-pixiu/pull/171#discussion_r649641858
##########
File path: pkg/config/api_config.go
##########
@@ -67,64 +80,116 @@ func LoadAPIConfigFromFile(path string) (*fc.APIConfig, error) {
// LoadAPIConfig load the api config from config center
func LoadAPIConfig(metaConfig *model.APIMetaConfig) (*fc.APIConfig, error) {
- client = etcdv3.NewConfigClient(
+ client, _ = etcdv3.NewConfigClientWithErr(
Review comment:
should we handle the error return from the new client function?
##########
File path: pkg/config/api_config.go
##########
@@ -67,64 +80,116 @@ func LoadAPIConfigFromFile(path string) (*fc.APIConfig, error) {
// LoadAPIConfig load the api config from config center
func LoadAPIConfig(metaConfig *model.APIMetaConfig) (*fc.APIConfig, error) {
- client = etcdv3.NewConfigClient(
+ client, _ = etcdv3.NewConfigClientWithErr(
Review comment:
It also limited the config center can only be etcd. Could we get the config center client from something like getConfigCenterClient(some_indicator)? It is not generic enough I think.
##########
File path: pkg/config/api_config.go
##########
@@ -67,64 +80,116 @@ func LoadAPIConfigFromFile(path string) (*fc.APIConfig, error) {
// LoadAPIConfig load the api config from config center
func LoadAPIConfig(metaConfig *model.APIMetaConfig) (*fc.APIConfig, error) {
- client = etcdv3.NewConfigClient(
+ client, _ = etcdv3.NewConfigClientWithErr(
etcdv3.WithName(etcdv3.RegistryETCDV3Client),
etcdv3.WithTimeout(10*time.Second),
etcdv3.WithEndpoints(strings.Split(metaConfig.Address, ",")...),
)
- go listenAPIConfigNodeEvent(metaConfig.APIConfigPath)
-
- content, err := client.Get(metaConfig.APIConfigPath)
+ kList, vList, err := client.GetChildren(metaConfig.APIConfigPath)
if err != nil {
return nil, perrors.Errorf("Get remote config fail error %v", err)
}
-
- if err = initAPIConfigFromString(content); err != nil {
+ if err = initAPIConfigFromKVList(kList, vList); err != nil {
return nil, err
}
+ // TODO: init other setting which need fetch from remote
+ go listenResourceAndMethodEvent(metaConfig.APIConfigPath)
+ // TODO: watch other setting which need fetch from remote
return apiConfig, nil
}
-func initAPIConfigFromString(content string) error {
+func initAPIConfigFromKVList(kList, vList []string) error {
lock.Lock()
defer lock.Unlock()
- apiConf := &fc.APIConfig{}
- if len(content) != 0 {
- err := yaml.UnmarshalYML([]byte(content), apiConf)
- if err != nil {
- return perrors.Errorf("unmarshalYmlConfig error %v", perrors.WithStack(err))
- }
+ tmpApiConf := &fc.APIConfig{}
+
+ for i, k := range kList {
+ v := vList[i]
- valid := validateAPIConfig(apiConf)
- if !valid {
- return perrors.Errorf("api config not valid error %v", perrors.WithStack(err))
+ // handle resource
+ re := getCheckResourceRegexp()
+ if m := re.Match([]byte(k)); m {
Review comment:
maybe we could use below judgment to reduce the complexity.
```
if m := re.Match([]byte(k)); m {
continue
}
```
##########
File path: pkg/config/api_config.go
##########
@@ -67,64 +80,116 @@ func LoadAPIConfigFromFile(path string) (*fc.APIConfig, error) {
// LoadAPIConfig load the api config from config center
func LoadAPIConfig(metaConfig *model.APIMetaConfig) (*fc.APIConfig, error) {
- client = etcdv3.NewConfigClient(
+ client, _ = etcdv3.NewConfigClientWithErr(
etcdv3.WithName(etcdv3.RegistryETCDV3Client),
etcdv3.WithTimeout(10*time.Second),
etcdv3.WithEndpoints(strings.Split(metaConfig.Address, ",")...),
)
- go listenAPIConfigNodeEvent(metaConfig.APIConfigPath)
-
- content, err := client.Get(metaConfig.APIConfigPath)
+ kList, vList, err := client.GetChildren(metaConfig.APIConfigPath)
if err != nil {
return nil, perrors.Errorf("Get remote config fail error %v", err)
}
-
- if err = initAPIConfigFromString(content); err != nil {
+ if err = initAPIConfigFromKVList(kList, vList); err != nil {
return nil, err
}
+ // TODO: 其他需要从远端获取配置的初始化操作
+ go listenResourceAndMethodEvent(metaConfig.APIConfigPath)
+ // TODO: 其他监控配置的操作，比如 PluginGroup 等
return apiConfig, nil
}
-func initAPIConfigFromString(content string) error {
+func initAPIConfigFromKVList(kList, vList []string) error {
Review comment:
I agree with cityiron. the complexity of this function is high. If we introduce sonarqube, it cannot pass the check.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org