Conversation
…ypes other than 'default'
There was a problem hiding this comment.
Could you please match the brace style in this file? Thank you!
There was a problem hiding this comment.
Sure. Should have done it, sorry
There was a problem hiding this comment.
It's no problem, I'm just being pedantic. Thank you!
There was a problem hiding this comment.
@lukebakken
You're absolutely right, I have to stick to the style in use.
|
Thank you for this pull request 😄 There should also be a unit test confirming this fix here (I think): @alexmoore - is that the right place? |
|
@lukebakken |
|
Great! |
|
Yep, that would be the folder to put a unit test in. |
|
Fixed, added unit tests. Please review. BTW, I think that copying the bucket type from the command context into response is wrong, and it should be returned from the backend. See com.basho.riak.client.api.commands.buckets.ListBuckets#executeAsync. As a side note, it posed a challenge when writing the unit tests, because validating an expected bucket type against a response does not make sense as the type is always the one from request. |
|
Yeah, our messages only return the bucket names: https://github.com/basho/riak_pb/blob/develop/src/riak_kv.proto#L127-L130 I think the original authors did this to save message space, otherwise we'd send a lot of duplicate info back with every bucket. This applies to the RpbListKeysResp message as well. :/ |
|
Looks good, thanks for the PR! |
3 participants
ListBuckets ignored the bucket type member when building a request, which made it work with the default bucket type only. Passing bucket type if not null to enable listing non-empty buckets of types other than 'default'.