Re: aedis review starts today

From: Дмитрий Архипов (grisumbras_at_[hidden])
Date: 2023-01-23 18:36:30

This is my review of Boost.Aedis.

> Are you knowledgeable about the problem domain?
I have moderate knowledge of ASIO. I have used Redis in a minor
capacity in the past.

> How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I've spent a few hours reading RESP3 spec and Redis documentation,
then a couple more hours reading Aedis documentation, a little bit of
its implementation, and comparing its API with the Python redis
library.

> Does this library bring real benefit to C++ developers for real world use-case?
Certainly.

> Do you have an application for this library?
No.

> Does the API match with current best practices?
The Asio part seems fairly standard.

As was pointed out by Peter Dimov, to/from_bulk is way to generic for
ADL customisation points. They should be more specific like
aedis_to_bulk or use tag_invoke (like JSON does).

Unlike some previous reviewers I am not so concerned with
"stringly-typed" request API. The full type safe approach would
require implementing 462 commands as separate types. Several would
require accompanying types for optional parameters. The commands are
sometimes extended to have more optional parameters. And can result in
different response types depending on the context (e.g. because of
transactions). This sounds like quite a lot of work.

In addition, Redis can load third-party modules which add more
commands. This means that the current API is still necessary, and a
type-safe one could only be added in addition to, not instead of it.

When thinking of how I would use Aedis, I envisioned writing several
(possibly, a few dozen) of my own helpers that support the commands I
would need and ensure that the proper arguments are used with each
command. I do not envision a situation where a significant part of my
code becomes string with commands. As such, I do not think the current
API creates too much of a hurdle.

> Is the documentation helpful and clear?
The documentation is very sparse, even though mostly serviceable. In
particular, there needs to be a more thorough explanation of type
requirements to arguments of aedis::adapt and aedis::resp3::node.
There also needs to be a discussion of what types to/from_bulk
supports out of the box.

> Did you try to use it? What problems or surprises did you encounter?
I have not.

> What is your evaluation of the implementation?
I haven't spent much time looking through the implementation.

> Do you think the library should be accepted as a Boost library?
I think the library should be ACCEPTED, on several conditions. First
of all, it has to improve its documentation. Second, it has to switch
to less generic customization point function names. And finally, It
appears from reading documentation and cursory looking through the
implementation that only fundamental and standard types are supported
by functions like `aedis::adapt`. I think Boost libraries should be
interoperable.

If these conditions are met, I believe the library would work very
well with several existing Boost libraries, and make it very easy to
create network applications with the help of Boost.

вс, 15 янв. 2023 г. в 18:17, Klemens Morgenstern via Boost
<boost_at_[hidden]>:

>
> The formal review of the aedis starts today (15th) and ends next week on
> Tuesday (24th).
>
> The library was developed & submitted by Marcelo.
>
> Aedis
> =================
>
> Aedis is a Redis client library built on top of Boost.Asio that
> implements the latest version of the Redis communication protocol
> RESP3.
>
> Some of its distinctive features are
> * A connection class that provides high-level functions to execute Redis
> commands and receive server pushes concurrently.
> * Automatic pipelining of commands.
> * Support for STL containers and serialization of user-defined data types.
> * Low-latency: Responses can be read in their final data-structure without
> creating copies.
>
> Aedis links
>    Github: https://github.com/mzimbres/aedis
>    Homepage: https://mzimbres.github.io/aedis/
>    Git tag: v1.4.1
>    Tarball:
> https://github.com/mzimbres/aedis/archive/refs/tags/v1.4.1.tar.gz
>
> Redis Links:
>    Redis: https://redis.io/
>    RESP3:
> https://github.com/redis/redis-specifications/blob/master/protocol/RESP3.md
>    Pipelines: https://redis.io/docs/manual/pipelining/
>    Redis commands: https://redis.io/commands/
>
> Local Redis
> ===========
> Redis can be also easily installed on your local machine, for example,
> on Debian run "apt install redis". You might also want to use docker
> https://www.docker.com/blog/how-to-use-the-redis-docker-official-image/
>
> Redis server for the Boost review
> =================================
> We also have a public aedis server available for testing,
> please contact me or Marcelo for the details.
>
>
> Review
> ========
>
> 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.
>
> The review is open to anyone who is prepared to put in the work of
> evaluating and reviewing the library. Prior experience in contributing to
> Boost reviews is not a requirement.
>
> Some questions to consider:
>
>  - Does this library bring real benefit to C++ developers for real world
> use-case?
>  - Do you have an application for this library?
>  - Does the API match with current best practices?
>  - Is the documentation helpful and clear?
>  - Did you try to use it? What problems or surprises did you encounter?
>  - What is your evaluation of the implementation?
>
> Additionally, stating your knowledge of redis and asio would be helpful for
> me.
>
> More information about the Boost Formal Review Process can be found
> at: http://www.boost.org/community/reviews.html
>
> Reviews can also be submitted privately to me, and I will not disclose who
> sent them.
> I might however cite passages from those reviews in my final decision.
>
> Thanks to Marcelo for providing us with a new library & thanks for all the
> reviews in advance.
>
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost