flycash commented on a change in pull request #1030:
URL: https://github.com/apache/dubbo-go/pull/1030#discussion_r567779148
##########
File path: common/config/environment.go
##########
@@ -72,22 +72,34 @@ func (env *Environment) UpdateExternalConfigMap(externalMap
map[string]string) {
for k, v := range externalMap {
env.externalConfigMap.Store(k, v)
}
+ env.externalConfigMap.Range(func(key, value interface{}) bool {
+ if _, ok := externalMap[key.(string)]; !ok {
+ env.externalConfigMap.Delete(key)
Review comment:
In which case this code will be excuted?
It looks like that the unit tests do not cover this special case.
##########
File path: common/config/environment.go
##########
@@ -72,22 +72,34 @@ func (env *Environment) UpdateExternalConfigMap(externalMap
map[string]string) {
for k, v := range externalMap {
env.externalConfigMap.Store(k, v)
}
+ env.externalConfigMap.Range(func(key, value interface{}) bool {
+ if _, ok := externalMap[key.(string)]; !ok {
+ env.externalConfigMap.Delete(key)
+ }
+ return true
+ })
}
// UpdateAppExternalConfigMap updates env appExternalConfigMap field
func (env *Environment) UpdateAppExternalConfigMap(externalMap
map[string]string) {
for k, v := range externalMap {
env.appExternalConfigMap.Store(k, v)
}
+ env.appExternalConfigMap.Range(func(key, value interface{}) bool {
+ if _, ok := externalMap[key.(string)]; !ok {
+ env.appExternalConfigMap.Delete(key)
+ }
+ return true
+ })
Review comment:
I think those lines could be refactor and extract them to be a new
function
##########
File path: config/config_loader.go
##########
@@ -194,6 +207,17 @@ func loadProviderConfig() {
}
checkRegistries(providerConfig.Registries, providerConfig.Registry)
+ // Write the current configuration to cache file.
+ if providerConfig.CacheFile != "" {
+ if data, err := yaml.MarshalYML(providerConfig); err != nil {
+ logger.Errorf("Marshal provider config err: %s",
err.Error())
+ } else {
+ if err := ioutil.WriteFile(providerConfig.CacheFile,
data, 0666); err != nil {
+ logger.Errorf("Write provider config cache file
err: %s", err.Error())
+ }
+ }
+ }
Review comment:
See my above comment
##########
File path: config/config_loader.go
##########
@@ -138,6 +140,17 @@ func loadConsumerConfig() {
ref.Implement(rpcService)
}
+ // Write current configuration to cache file.
+ if consumerConfig.CacheFile != "" {
+ if data, err := yaml.MarshalYML(consumerConfig); err != nil {
+ logger.Errorf("Marshal consumer config err: %s",
err.Error())
+ } else {
+ if err := ioutil.WriteFile(consumerConfig.CacheFile,
data, 0666); err != nil {
+ logger.Errorf("Write consumer config cache file
err: %s", err.Error())
+ }
+ }
+ }
Review comment:
I am not sure whether we should do this like:
```go
(c *consumerConfig) SaveToCacheFile() {
// ...
}
// in this funtion
consumerConfig.SaveToFile()
```
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
