Hello everyone, This is my review of the Aedis library. This is my first attempt at a Boost review, so I hope that it is helpful. Also, I'm attempting to send this from Gmail, so I apologize if the formatting doesn't come out right.
Additionally, stating your knowledge of redis and asio would be helpful for me.
I am using Redis in a project, so I am familiar with it, though nothing too advanced. I have looked at the Asio documentation before, but this is my first time actually trying it out (I've been meaning to).
Does this library bring real benefit to C++ developers for real world use-case?
Yes, absolutely, I think this is a great addition to the Asio ecosystem.
Do you have an application for this library?
Yes, I'd like to replace the hiredis code in the aforementioned project with something C++ with asynchronous support, but none of the existing options seemed great, and I want to try to move to using Asio in the future, so this library seems like a perfect fit.
Does the API match with current best practices?
I'm not that familiar with Asio best practices. I don't have a problem with the raw strings for this API; a more strongly-typed API could be added on top later.
Is the documentation helpful and clear?
Overall I thought it was a bit sparse. I think the other reviews have made some great suggestions, in particular Ruben Perez's comments on what could be added, such as a discussion of design decisions. I was initially put off by the lack of C++17 examples (since I'm not using it yet for work), but thinking about it with a long-term view I think the C++20 style is a good thing. However, I thought the use of the "common" files in the examples was less than helpful; I think it would be easier to follow if the examples were each complete (maybe some "common" code could be added to the library?). Is it important to send "HELLO"? I don't see that explained. In the Redis documentation for "QUIT", it says "Clients should not use this command", so maybe you should explain why it is used. It was not immediately obvious why a `tuple` is used for the response, so maybe some explanation of that is warranted the first time it is shown, or at least a reference to the "Responses" section (e.g. "response type will be explained later in the 'Responses' section"). I'd also like to see an explanation of why the `adapt` function call is required. Why can't I just pass the response type? Most of the examples show multiple commands, and it's great that it can handle that, but it seems like a common case would be to execute a single command and get a response. It seems like there should be some shortcut included in the library, for example: std::optionalstd::string reply; co_await conn.async_exec(resp3::request("GET", "FOO"), adapt(reply));
Did you try to use it? What problems or surprises did you encounter?
Yes, I played with the examples. I mostly tried using it in the C++17 style. I had errors compiling the cpp17_intro.cpp example on GCC 10 (gcc-toolset-10 on RHEL8),so I had to switch to GCC 12 ("no matching function call to from_chars..."). I initially tried to use the library via CMake but Boost::asio was not a valid target for me. The first time I tried to modify one of the examples to do something different, I got a SIGSEGV with no obvious indication of what went wrong. It was an object lifetime issue - obvious in retrospect that the request and reply objects need to live past the async_exec call, but I guess the lambdas make it easy to forget how things are actually flowing. More an Asio/C++ issue than Aedis. This may also be due to my unfamiliarity with Asio, but when I tried the "use_future" completion token, get() never returned: resp3::request req; req.push("QUIT"); auto f = conn.async_exec(req, adapt(), net::use_future); std::cout << "QUIT: " << f.get() << std::endl;
What is your evaluation of the implementation?
I didn't look very deeply into the implementation.
Please explicitly state that you either *accept* or *reject* the inclusion of this library into boost. Also please indicate the time & effort spent on the evaluation and give the reasons for your decision.
I think the explanation from Ruben's review seems reasonable, so I will also say REJECT in the current state, but I very much would like this library to be included in Boost in the future. It sounds like the error handling may need some more consideration, and I'd like to see the documentation expanded. I would also like to see the name changed to Boost.Redis if accepted, to make it easier to identify and find. I spent around 5-6 hours over a few days evaluating the library. Thanks very much to Marcelo for working on this library! - Sam Hartsfield