redis / lettuce

Advanced Java Redis client for thread-safe sync, async, and reactive usage. Supports Cluster, Sentinel, Pipelining, and codecs.
https://lettuce.io
MIT License
5.3k stars 947 forks source link

Add support for CLIENT TRACKINGINFO #2862

Open tishun opened 1 month ago

tishun commented 1 month ago

Closes #2761

Make sure that:

tishun commented 1 month ago

This is what I initially went with, but the existing CommandOutput abstractions could not handle a Map result with nested Array. Both the MapOutput and the GenericMapOutput could not handle this.

The only place we had a similar parsing done is with the CLUSTER SLOTS and I replicated this approach.

@mp911de do we have a way to handle this or should I create a new CommandOutput?

mp911de commented 3 weeks ago

@mp911de do we have a way to handle this or should I create a new CommandOutput?

Have you tried ObjectOutput? An alternative could be ArrayOutput. If both do not work, then a custom CommandOutput would be the last alternative.

tishun commented 3 weeks ago

Sooo folks I'd love to hear from you what you think about the latest solution.

I already had a discussion with @uglide on that, but would love to hear any comments.

Obviously it has some downsides, but I personally find it a good compromise between what we have as a solution right now and having an actual mapping to a domain object. The latter would also incur some more heavy duty maintenance on our part.

mp911de commented 3 weeks ago

The new output is pretty complex and its result seems pretty complex to understand as well. A Object or List<Object> return is typically much easier to consume as it would contain Redis primitives (Lists, numbers, strings) that are more common than the approach used here.

tishun commented 3 weeks ago

The new output is pretty complex and its result seems pretty complex to understand as well. A Object or List<Object> return is typically much easier to consume as it would contain Redis primitives (Lists, numbers, strings) that are more common than the approach used here.

I agree. It is a tradeoff.

The proposed solution is supposed to be used (mainly) in conjunction with the parser logic. Further down the line the parser could be extended to generate stubs based on some schema definition. For a strongly typed language such as Java this is the only working way to consume an API of a service, that has complex types and could be changed down the line.

Jedis has a model object for complex return types, but this makes the design tightly coupled with the current API. Any change to the API would break the existing model.

I was aiming for a middle ground where we have a utility to help the user consume the logic in a simple way (see the integration test), but also allow them to parse the information themselves if the model changes.

A good example for this is if another value is added to the Map. The consumers of the driver could start polling the new value without having to wait until we release a new version of the parser.

So we gat a bit of more stability for the price of having to deal with this interim object that is a bit complex to use.

tishun commented 3 weeks ago

We should return Map<String, String> to make the response usable even without a parser.

@uglide what is your take on this solution?