RESP3 icon indicating copy to clipboard operation
RESP3 copied to clipboard

Attribute type

Open BridgeAR opened this issue 6 years ago • 3 comments

I just went ahead and implemented RESP3 for the Node.js redis parser and I struggle to see the how this can be delivered to the user in a useful way.

It is not clear if the attribute alone should be delivered to the user or if it should be delivered in combination with the actual corresponding data (which IMHO would make sense). The latter however requires the parsing logic to become more complex in cases for aggregated data types such as arrays, maps and sets.

It is also not clear when exactly the data should be delivered as in: immediately after the attribute is parsed or after the top level is done parsing? (This is again about aggregated data types)

Would it not be better to just provide the very same information as push information that just contains the actual data on top of the attribute? It's of course extra data to parse but it would simplify the parser logic. Right now the attributes somewhat seem out of line for me compared to the rest of the spec.

BridgeAR avatar Feb 05 '19 18:02 BridgeAR

It may perhaps be a good idea to stipulate that the attributes won't ever contain information that you really shouldn't chuck away and can't get another way so that simpler libraries can exist that ignore it completely without missing something really important.

For the higher-level languages like Python or JS, it might be clearer to return a response as a class structure that contains the attributes, rather than working out some way of representing them with native datatypes like dicts (so response.items, response.attributes and response.items[0]['somekey'].attributes, as examples)

AngusP avatar Jul 03 '19 15:07 AngusP

@AngusP that would work but it does not sound like a good solution to me. In most cases the data type would be the plain response, so wrapping the return type requires more memory, slows down the parser due to creating more objects and requires to check for the attribute property for all returned values.

@antirez it would be great if you could have a look at this. Is there a real need for the attribute type? It seems very difficult to implement this in a useful way.

BridgeAR avatar Feb 06 '20 19:02 BridgeAR

In particular, it is very awkward for nested data; most clients would be able to add some kind of "and the attributes associated with the last payload are ...", as per the key popularity example shown in the spec (r.attribs) - but it is not obvious how to efficiently do this for sub-data, like the {ttl:3600} example in the spec

mgravell avatar Feb 13 '20 10:02 mgravell