Conversation
This saves a roundtrip when you set/add an item that you'll want to modify later. This is a backward-incompatible change, but it seems important enough and seems to be a coding oversight as most of the work was already done (but thrown away) in store_callback().
|
I notice https://github.com/orgs/chitika/people is empty now, might be time to start using my repo as the trunk 😂 |
|
@wcummings in the original PR (#62) you suggested merging the optional arguments together into a map (or proplist or record or...), I think that would work well and we can do this fairly elegantly: % @deprecated
store(PoolPid, Op, Key, Value, TranscoderOpts, Exp, Cas) ->
execute(PoolPid, {store, Op, Key, Value, #{trancoder => TranscoderOpts, exp => Exp, cas => Cas}).
% New default function
store(PoolPid, Op, Key, Value, Opts) ->
execute(PoolPid, {store, Op, Key, Value, Opts}).
% Simple version
store(PoolPid, Op, Key, Value) ->
execute(PoolPid, {store, Op, Key, Value, #{}}).
% Some functions require some where trickery but shouldn't need a name change
append(PoolPid, Cas, Key, Value) when is_integer(Cas) ->
store(PoolPid, append, Key, Value, {cas => Cas});
append(PoolPid, Key, Value, Opts) ->
store(PoolPid, append, Key, Value, Opts).
append(PoolPid, Key, Value) ->
store(PoolPid, append, Key, Value, #{}).From there on, the API should be a bit more future-proof and it'll be easy to add a |
|
@vincentdephily 👍 I can make this happen |
|
@vincentdephily So i think this basically works, but the problem I've noticed is situations like this: add(PoolPid, Key, Value) ->
add(PoolPid, Key, Value, []).
add(PoolPid, Key, Value, Opts) when is_list(Opts) ->
store(PoolPid, add, Key, Value, Opts);
%% @equiv add(PoolPid, Key, Exp, Value, standard)
%% @deprecated
add(PoolPid, Key, Exp, Value) ->
store(PoolPid, add, Key, Value, [{exp, Exp}]).If the key is a string (list of chars) the guard won't save us. Similarly, a value can be an integer, just like an expiration. I think the best solution is to simply remove the newly deprecated functions, but I'm open to suggestions. Since a value can be basically anything it's pretty hard to resolve this "clash". Even a tagged tuple could potentially be a valid value, depending on the transcoder being used. The options I see are:
I'll push my changes shortly. |
|
OK after some deliberation I'm going with the original change, it breaks compatibility but it seems that was unavoidable and this is just a lot simpler and probably an easier change for people. It's in my repo w/ a tagged release. https://github.com/wcummings/cberl |
Tests pass, dialyzer is happy
This is a breaking change, but I agree w/ @vincentdephily that it is important enough. Because it affects so many functions, the alternatives would severely muddy the interface (for ex we could create a new set of store functions in a
cberl_v2module, or a new set of functions w/ a different arity, essential a "return a cas" flag). This should be accompanied by a version bump.#86
cc @vasumahesh1
P.S.
""doesnt give you the default bucket anymore, this includes a commit to pass"default"instead