Add validation for n_val and other riak_kv bucket props #667
Conversation
ghost
assigned 
|
This seems quite sane, despite needing to check atom() and binary() versions of the same option, oh well. +1. |
| -spec is_quorum(term()) -> true | false. | ||
| is_quorum(Q) when is_integer(Q), Q > 0 -> | ||
| true; | ||
| is_quorum(Q) when is_atom(Q), Q =:= quorum |
There was a problem hiding this comment.
There is no need for the is_atom here since you are matching exactly (=:=) against atoms anyway. The same with binaries below.
|
Addressed comments. |
| validate([Prop|T], ValidProps, Errors) -> | ||
| validate(T, [Prop|ValidProps], Errors). | ||
|
|
||
|
|
||
| -spec is_quorum(term()) -> true | false. | ||
| is_quorum(Q) when is_integer(Q), Q > 0 -> |
There was a problem hiding this comment.
The default for pr and pw is zero, which doesn't pass this validation. So, for example, getting the default properties and setting them again, maybe with a modification, will fail. I'm not sure why it's zero. The code seems to make sure that zero is never used I believe.
There was a problem hiding this comment.
yea it can be zero https://github.com/basho/riak_kv/blob/develop/src/riak_kv_app.erl#L83-L97
There was a problem hiding this comment.
Do you think other quora can be zero? I mean can you zero R? W? DW? RW?
|
Ping @engelsanchez addressed comments |
|
Bump? |
|
re-re-bump ping bueller, bueller |
|
Oh, I assumed this no longer merges cleanly but it looks like it does :). I'm doing some separate |
|
Pulling PRs out of the freezer ... re-re-re-bump? |
|
@slfritchie see riak_kv#747… @jrwest has said he'll merge this with that. |
|
closing for rebased/merged version in #770 |
Addresses issue #666 test at basho/riak_test#389