Ecwid / consul-api

Java client for Consul HTTP API
Apache License 2.0
417 stars 175 forks source link

Implement CAS support for delete method #49

Open etki opened 8 years ago

etki commented 8 years ago

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.

jglamine commented 5 years ago

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.

Anusien commented 4 years ago

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 commented 4 years ago

@vgv Do you have a preference for the fix?