feat: Ensure new configurations from the file are applied #894
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #894 +/- ##
==========================================
- Coverage 46.08% 45.90% -0.19%
==========================================
Files 62 62
Lines 3669 3684 +15
==========================================
Hits 1691 1691
- Misses 1856 1871 +15
Partials 122 122 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
weichou1229
force-pushed
the
config-migrate
branch
2 times, most recently
from
90e916a to
226cd21
Compare
bootstrap/config/config.go
Outdated
| if hasSubConfig { | ||
| continue | ||
| } |
There was a problem hiding this comment.
Add a comment to indicate the rationale why skip the key when it has subConfig
bootstrap/config/config.go
Outdated
| continue | ||
| } | ||
| if err = configClient.PutConfigurationValue(key, []byte(fmt.Sprint(value))); err != nil { | ||
| return fmt.Errorf("failed to push common configuration key %s with value %v: %s", key, value, err.Error()) |
There was a problem hiding this comment.
From the error log, does the code here only handle common config? How about the private config?
bootstrap/config/config.go
Outdated
| // buildKeyValues is a helper function to parse the configuration yaml file contents | ||
| func buildKeyValues(data map[string]interface{}, kv map[string]interface{}, origKey string) map[string]interface{} { | ||
| key := origKey | ||
| for k, v := range data { | ||
| if len(key) == 0 { | ||
| key = fmt.Sprint(k) | ||
| } else { | ||
| key = fmt.Sprintf("%s/%s", key, k) | ||
| } | ||
|
|
||
| vdata, ok := v.(map[string]interface{}) | ||
| if !ok { | ||
| kv[key] = v | ||
| key = origKey | ||
| continue | ||
| } | ||
|
|
||
| kv = buildKeyValues(vdata, kv, key) | ||
| key = origKey | ||
| } | ||
|
|
||
| return kv | ||
| } |
There was a problem hiding this comment.
The current function comment doesn’t clearly convey the purpose of this helper, making it hard to judge whether its recursive logic is correct.
From the code, I can only guess that the function’s intent is to flatten a nested map into a single-level map? If that’s correct, please update the comment to explicitly describe this purpose. Also, since this is a utility function, consider to move this func to bootstrap/utils/utils.go and this func should have unit tests to verify that its behavior matches the intended goal.
There was a problem hiding this comment.
I found the config provider client has already done the key flattening and put config when the keyValue not exist or override is set, so I updated the code to keep it simple.
https://github.com/edgexfoundry/go-mod-configuration/blob/main/internal/pkg/keeper/client.go#L96
weichou1229
force-pushed
the
config-migrate
branch
from
226cd21 to
8310e4f
Compare
After upgrading EdgeX, new configurations may be introduced. Ensure new configurations from the YAML file are pushed to the core-keeper. Signed-off-by: bruce <[email protected]>
weichou1229
force-pushed
the
config-migrate
branch
from
8310e4f to
b92b2bf
Compare
After upgrading EdgeX, new configurations may be introduced. Ensure new configurations from the YAML file are pushed to the core-keeper.
If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/go-mod-bootstrap/blob/main/.github/Contributing.md
PR Checklist
Please check if your PR fulfills the following requirements:
BREAKING CHANGE:describing the break)Testing Instructions
New Dependency Instructions (If applicable)