Re: aedis review starts today

From: Marcelo Zimbres Silva (mzimbres_at_[hidden])
Date: 2023-01-24 19:22:26

On Tue, 24 Jan 2023 at 18:12, Sam Hartsfield via Boost
<boost_at_[hidden]> wrote:
>
> > Is the documentation helpful and clear?
>
> I was initially put off by the lack of C++17 examples (since I'm not
> using it yet for work),

There are three C++17 example in the docs

- https://mzimbres.github.io/aedis/cpp17__intro_8cpp_source.html
- https://mzimbres.github.io/aedis/cpp17__intro__sync_8cpp_source.html

> Is it important to send "HELLO"? I don't see that explained.

Yes it is necessary to upgrade to the protocol version used by the
library. That is why all examples use it.

> In the Redis documentation for "QUIT", it says "Clients should not use this
> command", so maybe you should explain why it is used.

I use QUIT for clarity. If you start having TIME_WAIT problems
server-side, just don't use it. Notice however that in Aedis you will
often need only once connection in your entire application, which
reduces the load on the server.

> 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,

The documentation is focused on how to use Aedis and not much on
explaining design decisions. The response to commands must be stored
somewhere, a tuple provides a simple way to group them in a way that
they can be indexed.

> 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?

Adapt is there to make customization for user data structures easy to
integrate. It also removes some of the complexity from the response
object, for example, it provides a way to specify how much you allow
the implementation to read from the socket for each response and
overall.

I don't document that because it is something 99% of the user won't ever need.

> 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::optional<std::string> reply;
>     co_await conn.async_exec(resp3::request("GET", "FOO"), adapt(reply));

The request object must outlive the asynchronous operation, so this is
not going to work.

> > Did you try to use it? What problems or surprises did you encounter?
> <snip>
>
> 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;

There are many reasons for why this can go wrong. I would have started
with the synchronous example

https://mzimbres.github.io/aedis/cpp17__intro__sync_8cpp_source.html

Thanks for investing time on this,
Marcelo