Improve config center #1030
Merged
Improve config center #1030
Conversation
This file contains 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
luckyxiaoqiang
force-pushed
the
feature/improve_config_center
branch
2 times, most recently
from
e7e1006
to
f4be162
Compare
AlexStocks
requested review from
zouyx,
hxmhlt and
flycash
and removed request for
zouyx and
hxmhlt
|
pls change the target branch to 1.5 |
luckyxiaoqiang
force-pushed
the
feature/improve_config_center
branch
from
f4be162
to
e703ce9
Compare
Codecov Report
@@ Coverage Diff @@
## 1.5 #1030 +/- ##
==========================================
+ Coverage 59.53% 59.65% +0.11%
==========================================
Files 259 261 +2
Lines 12737 12943 +206
==========================================
+ Hits 7583 7721 +138
- Misses 4199 4248 +49
- Partials 955 974 +19
Continue to review full report at Codecov.
|
Updated. |
flycash
reviewed
Feb 1, 2021
Comment on lines
+144
to
+152
| 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()) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I am not sure whether we should do this like:
(c *consumerConfig) SaveToCacheFile() {
// ...
}
// in this funtion
consumerConfig.SaveToFile()
Comment on lines
+210
to
+219
| // 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()) | ||
| } | ||
| } | ||
| } |
zouyx
reviewed
Feb 20, 2021
| 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 { |
There was a problem hiding this comment.
I can not get this point. why will you copy this config to a new file?
There was a problem hiding this comment.
Please see the issue comment: #729 (comment).
AlexStocks
approved these changes
Mar 4, 2021
AlexStocks
force-pushed
the
1.5
branch
2 times, most recently
from
415e245
to
698908d
Compare
AlexStocks
approved these changes
Mar 6, 2021
flycash
approved these changes
Mar 7, 2021
| @@ -50,6 +50,9 @@ type BaseConfig struct { | |||
| EventDispatcherType string `default:"direct" yaml:"event_dispatcher_type" json:"event_dispatcher_type,omitempty"` | |||
| MetricConfig *MetricConfig `yaml:"metrics" json:"metrics,omitempty"` | |||
| fileStream *bytes.Buffer | |||
|
|
|||
| // cache file used to store the current used configurations. | |||
| CacheFile string `yaml:"cache_file" json:"cache_file,omitempty" property:"cache_file"` | |||
There was a problem hiding this comment.
I can't find where to use this field.
There was a problem hiding this comment.
Please see config_loader.go file.
There was a problem hiding this comment.
Yes, you write provider config to CacheFile, but I want to know where to use it.
There was a problem hiding this comment.
Please see the issue comment: #729 (comment).
LaurenceLiZhixin
approved these changes
Mar 15, 2021
AlexStocks
added a commit
that referenced
this pull request
Apr 14, 2021
…enter Improve config center
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What this PR does:
Which issue(s) this PR fixes:
Fixes #729
Special notes for your reviewer:
Does this PR introduce a user-facing change?: