consul-api icon indicating copy to clipboard operation
consul-api copied to clipboard

Implement CAS support for delete method

Open etki opened this issue 8 years ago • 3 comments

Consul (at least of version 0.6) returns boolean value for delete operation that marks it's success (normal deletion) or failure (e.g. cas parameter set to invalid value). It would be quite cool to add same support (ability to pass cas parameter and return boolean value) for client implementation.

etki avatar Mar 16 '16 13:03 etki

For what it's worth, I was able to find a workaround by using RawConsulClient directly.

    public void deleteKey(final String key, final long checkAndSetIndex) {
        consulRawClient.makeDeleteRequest(
            "/v1/kv/" + key,
            (UrlParameters) () -> Collections.singletonList("cas=" + checkAndSetIndex)
        );
    }

Take a look at ConsulRawClient.java to see how to construct one.

jglamine avatar Apr 24 '19 23:04 jglamine

The problem is that all the different overloaded methods of deleteKVValue() return Response<Void>. In order to indicate whether the delete succeeded or not, we would need to return Response<Boolean>. I see a few ways forward:

  1. The most minimal change is to just add the support for the cas parameter, but don't return the boolean. If callers want to know whether their delete succeeded, they'll have to do a KV get.
  2. A partial change would be to add more deleteKVValue() methods that support the cas parameter and return Response<Boolean>, but leave the others as-is. So deleteKVValue() would return a mix of Response<Void> and Response<Boolean> depending on use case.
  3. A more gradual change is to add new methods, probably with a different name, that return Response<Boolean>. Leave the old ones in place and @Deprecate them.
  4. The most major change would be to just change all the existing methods to Response<Boolean>, probably after a major version number change.

Anusien avatar Jan 07 '20 22:01 Anusien

@vgv Do you have a preference for the fix?

Anusien avatar Jan 13 '20 18:01 Anusien