Allow to diff two configurations and return the difference in the keys.#93
Allow to diff two configurations and return the difference in the keys.#93urso merged 3 commits intoelastic:masterfrom ph:feature/allow-to-diff-configuration-keys
Conversation
|
@urso When working on the implicit keystore usage in beats, I needed something like this. |
diff.go
Outdated
| } | ||
| } else { | ||
| // We should not get here | ||
| panic("unknown config type") |
There was a problem hiding this comment.
I believe this should not happen but I think it could.
|
+1 In go-ucfg, the top-level package is mostly the core for packing/unpacking/querying the config object. All extra functionality is in sub-packages. As getters/setters can operate on complete settings paths, adding the |
|
@urso I have applied the changes we were talking about. |
| "fmt" | ||
| "strings" | ||
|
|
||
| ucfg "github.com/elastic/go-ucfg" |
There was a problem hiding this comment.
Note: This line is equivalent to "github.com/elastic/go-ucfg".
goimports normaly adds this kind of line. If - is used in package names, the later part of the name makes the package.
diff/keys.go
Outdated
| } | ||
|
|
||
| // Keys takes two configuration and return the difference between the defined keys | ||
| func Keys(old, new *ucfg.Config, opts ...ucfg.Option) Diff { |
There was a problem hiding this comment.
This is basically the constructor. How about naming it one of (Make/Build/Create)Diff, Compare, CompareConfigs or Changes?
| // FlattenedKeys return a flattened views of the set keys in the configuration | ||
| func (c *Config) FlattenedKeys(opts ...Option) []string { | ||
| var keys []string | ||
| normalizedOptions := makeOptions(opts) |
There was a problem hiding this comment.
Not sure, but pathSep might be empty here. If so add:
if normalizedOptions.pathSep == "" {
normalizedOptions.pathSep = "."
}
ucfg.go
Outdated
| } | ||
| } else { | ||
| // We should not get here | ||
| panic("unknown config type") |
There was a problem hiding this comment.
hm... not sure about 'empty' config objects.
| panic("unknown config type") | ||
| } | ||
|
|
||
| sort.Strings(keys) |
There was a problem hiding this comment.
We need to sort here? If so the godoc should reflect the list returned is always being sorted.
|
@urso I choose |
| sort.Strings(results) | ||
|
|
||
| assert.Equal(t, expected, results) | ||
| } |
There was a problem hiding this comment.
tests are quite similar. Rather introduce a table-driven test + use Run like:
func TestFlattenKeys(t *testing.T) {
tests := struct {
name string
pathSep string
}{
{ "withDot", "." },
{ "emptySep", "" },
}
sorted := func(s []string) []string {
sort.Strings(s)
return s
}
cfg := map[string]interface{}{ ... }
expected := sorted([]string { ... })
for _, test := range tests {
sep := test.pathSep
t.Run(test.name, func(t *testing.T) {
opts := []Option{}
if sep != "" {
opts = append(opts, PathSep(sep))
}
c, err := NewFrom(cfg, opts...)
if err != nil { t.Fatal(err) }
assert.Equal(t, expected, sorted(c.FlattenKeys(opts...)))
})
}
}
|
@urso, I've moved the two similar test into a table driven test. |
|
seems to be a problem with the ci, I see that you are working on it on #95. |
|
rebased with master, the ci should run. |
Pass 2 difference Config object to DiffKeys and it will return a Diff type that will contains the following: - Added keys - Keep keys - Removed keys Add a few helpers method on the type to know if something is different between the 2 configuration.
Pass 2 difference Config object to DiffKeys and it will return a Diff
type that will contains the following:
Add a few helpers method on the type to know if something is different
between the 2 configuration.