search

sewenew / redis-plus-plus

Redis client written in C++
Apache License 2.0
1.58k stars 347 forks source link

[FEATURE] Request ReplyUPtr expose at Redis++ Async response #554

Open lisay-yan opened 5 months ago

lisay-yan commented 5 months ago

Is your feature request related to a problem? Please describe. My use case prefer send all kinds of raw command, such as lua etc, and prefer to parse hiredis redisReply by application itself.

Describe the solution you'd like Make ReplyUPtr = std::unique_ptr<redisReply, ReplyDeleter> possible to access as Async cluster client as Sync client did now. Then, application can parse redisReply per itself.

Describe alternatives you've considered N/A

Additional context I prefer to know if that is feasible, and if doable, the draft schedule. Thanks.

lisay-yan commented 5 months ago

Hi, Dear code officers

Due to this request block my current project, so, I did simple changes to expose hiredis redisReply. Would you like support review and approval? Thanks so much.

https://github.com/sewenew/redis-plus-plus/pull/555

For sure, we will keep the mindset, redisReply after CB, it will be freed by hiredis. THanks.

sewenew commented 5 months ago

Thanks for your PR! However, it's not a good idea to parse the reply to redisReply*. There's an override command interface that returns RedisUPtr, however, it might not fit for async interface. As you mentioned, redisReply is freed by hiredis framework. If you expose redisReply * to end user, they might use it incorrectly, especially the life scope of the raw pointer.

Why not use the generic interface to do the parse work? It works well with most cases, including Lua scripting. If you compile the code with C++17, with the power of std::variant, you can handle all cases of Redis reply.

Regards

lisay-yan commented 5 months ago

@sewenew

Most of traffic in my use case is redisModule, and user already with business logic to decode redisReply in current system. To integrate with Reds-Plus-Plus only for cluster aware, so, if user can guard safe to use redisReply, expose it maybe a good choice for user which doesn't start from zero?? Agree? And My application can't be C++17 for some reason now.

Since my PR had been draft ready, is it possible to gain your approval to provide an option to Redis-Plus-Plus user at least?? Thanks for understanding.

sewenew commented 5 months ago

Instead of exposing redisReply to user, I was planing to expose a ReplyParser interface. So that user can parse redisReply to a user defined data structure. That might solve your problem. However, I'm too busy to start the project.

Regards