Make the List method homogeneous between all stores #32
Conversation
|
@nmengin Changes LGTM, although tests are failing because of boltdb and redis which have the same behavior of including the "directory" in the result. We may have to modify both backends to also exclude the "directory" on |
nmengin
changed the title
|
@abronan BoltDB and Redis are now homogeneous too 😉 (and by commits need to be squashed 😜 ) |
.gitignore
Outdated
| @@ -1 +1,4 @@ | |||
| dump.rdb | |||
| .idea/ | |||
| Gopkg.* | |||
| vendor/ | |||
There was a problem hiding this comment.
Is it possible to remove these lines from the .gitignore? Because they are either IDE specific or depending on other PRs 😸
|
@nmengin Is it possible to squash your commits? Also it would be perfect if you can write a full description for this change that impact all stores and what it fixes (not using the |
…ing an empty list if the key given in entry has no child. The modifications are done on ETCD V2,V3, BoltDB and Redis stores.
nmengin
force-pushed
the
feature/make-etcd-list-homogeneous
branch
from
b8a243e
to
b7ad9bc
Compare
|
@abronan squash done. Let me know if the description is OK for you. |
store/boltdb/boltdb.go
Outdated
| @@ -302,6 +301,8 @@ func (b *BoltDB) List(keyPrefix string) ([]*store.KVPair, error) { | |||
| }) | |||
| if len(kv) == 0 { | |||
| return nil, store.ErrKeyNotFound | |||
| } else if len(kv) == 1 && kv[0].Key == keyPrefix { | |||
| return []*store.KVPair{}, err | |||
There was a problem hiding this comment.
Unfortunately this is not going to work here: if we have an array of > 1 element, we're still going return the directory key like before. I think we should do like other stores and filter in the loop above before appending to the array of pairs, when key == directory we skip that entry and continue looping over keys.
| if assert.NotNil(t, pairs) { | ||
| assert.Equal(t, 0, len(pairs)) | ||
| } | ||
| } |
There was a problem hiding this comment.
We should add a test case where the list is non-empty with a few elements and check we don't return the directory key on List. We should test for all cases (otherwise we would have missed the bad case for boltdb).
There was a problem hiding this comment.
@abronan OK, I'll crate the parent key in the testList() function.
In this way, 3 keys will be inserted (1 parent and 2 children) but only 2 Pairs have to been returned.
…ly the children are returned
When the
List()method is called for the etcd provider, a Pair with the given key is returned in the result.This behavior is different than the others stores which only return the children
Pairs.The PR allows filtering this root
Pairfrom the result for ETCD V2 and V3.