node: Read config files from dir even if config file not set via --config
#245
Labels
No labels
P0
P1
P2
P3
badger
frostfs-adm
frostfs-cli
frostfs-ir
frostfs-lens
frostfs-node
good first issue
triage
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: TrueCloudLab/frostfs-node#245
Loading…
Reference in a new issue
No description provided.
Delete branch "acid-ant/frostfs-node:bugfix/238-fix-read-cfg-from-dir"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Close #238
Signed-off-by: Anton Nikiforov an.nikiforov@yadro.com
@ -188,3 +188,3 @@
FROSTFS_TRACING_ENDPOINT="localhost"
FROSTFS_TRACING_EXPORTER="otlp_grpc"
FROSTFS_TRACING_EXPORTER="otlp_grpc"
What has changed here?
New line added.
@ -37,2 +33,2 @@
var cfgFile *os.File
cfgFile, err = os.Open(fileName)
func mergeConfig(v *viper.Viper, fileName string) error {
cv := viper.New()
Have you thought about "just setting empty config map" in the beginning and performing
Merge
everywhere else?The main problem with merge with empty map that viper unable to parse incoming data because we do not have an API to set config type for this data. Only if we set it directly at the beginning viper able to parse data which we provide with Merge. But this will block us from merging data in multiple formats.
They still have an open issue for merge https://github.com/spf13/viper/issues/181
@ -0,0 +10,4 @@
)
func TestConfigDir(t *testing.T) {
dir, err := os.MkdirTemp("", "")
t.TempDir()
Perfect, thanks!
@ -0,0 +16,4 @@
cfgFileName0 := path.Join(dir, "cfg_00.json")
cfgFileName1 := path.Join(dir, "cfg_01.yml")
cfgFile, err := os.Create(cfgFileName0)
Consider simplifying to
Same below.
Fixed.
@ -0,0 +41,4 @@
os.Clearenv() // ENVs have priority over config files, so we do this in tests
return New(p,
inline the value if it's not actually needed:
Removed this method at all, looks redundant. Please review.
@ -0,0 +39,4 @@
func fromDir(dir string) *Config {
var p Prm
os.Clearenv() // ENVs have priority over config files, so we do this in tests
it probably would be better to use t.Setenv() here, to avoid nuking the env for every test and/or conflict with parallel ones.
Removed this method at all, looks redundant.
a5fd41bda0
toe694540f2a
@ -0,0 +15,4 @@
cfgFileName0 := path.Join(dir, "cfg_00.json")
cfgFileName1 := path.Join(dir, "cfg_01.yml")
require.NoError(t, os.WriteFile(cfgFileName0, []byte("{\n \"storage\": {\n \"shard_pool_size\": 15\n }\n}"), 0777))
if you use backticks, you don't need to quote the special characters
i.e.
[]byte{`{ "foo" : "bar" }`}
e694540f2a
to76f5b1f18a
76f5b1f18a
to187eddf69f
187eddf69f
to2e27f567c5
2e27f567c5
to0ed3eebe1c
Fixed CHANGELOG - moved changes in section related to v0.37.0.
0ed3eebe1c
to4c04395ba9
@ -20,4 +21,2 @@
continue
}
ext := path.Ext(entry.Name())
if ext != ".yaml" && ext != ".yml" {
I think we should still have
.yaml
/.json
check -- we don't want to read arbitrary garbage (may be we do, but that is a separate feature).Reverted, added support for json.
4c04395ba9
to696cae396c
696cae396c
to13dd4ec563