Fwd: aedis review starts today

From: Vinnie Falco (vinnie.falco_at_[hidden])
Date: 2023-01-25 19:27:50

The following review came in after the review period ended. I am
reposting it with permission. A copy was also sent by the reviewer to
the review manager.

---------- Forwarded message ---------
From: M P <miguelportilla64_at_[hidden]>
Date: Wed, Jan 25, 2023 at 11:14 AM
Subject: RE: [boost] aedis review starts today
To: <klemensdavidmorgenstern_at_[hidden]>, Vinnie Falco <vinnie.falco_at_[hidden]>

> Please explicitly state that you either *accept* or *reject* the inclusion of this library into boost.

I ACCEPT this library into boost. My only concern is that although
Redis may be the flavor of the day, project popularities fade. If
Redis is supplanted, I wonder how effective boost maintainers down the
road will be to remove it.

> Also please indicate the time & effort spent on the evaluation and give the reasons for your decision.

> Additionally, stating your knowledge of redis and asio would be helpful for Me.

I had no prior knowledge of Redis before reviewing Aedis, but I am
familiar with Boost and Asio. Before diving into Aedis, I began by
studying Redis, its RESP protocol, and real-world use to help me
understand why the Aedis project might be useful to the c++ community.
It quickly became apparent that Redis is very popular. It is the most
popular database used on Amazon Web Services, the world’s largest
cloud service provider. It is also ranked first in its category and
the top ten of all categories by DB-Engines. In today’s full-stack web
applications, users expect responsiveness, and Redis is an important
component in scaling architecture to meet demands. Aedis grants c++
developers Redis access, empowering their software to participate in
this evolving sphere of technologies.

I spent about three days learning about Redis, RESP3, and Aedis. While
evaluating Aedis, I built the project in Linux and Windows and
performed tests using Visual Studio 2022. The project examples provide
a variety of scenarios that may be useful as starting points for folks
with different requirements.

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

Absolutely. Any c++ project with heavy utilization of Relational
Databases can benefit from the cache performance Redis provides.
Projects that do use a Relational Database may also be able to use
Redis as a mechanism to share data across servers. There are many
real-world use cases ranging from web, multi-user games, fintech, and
blockchain.

> - Do you have an application for this library?

I do not have a software application at the moment that uses Redis. I
am working on a project that might end up using Redis and if so, would
likely use Aedis.

> - Does the API match with current best practices?

Yes, the API is well-matched with current practices. Aedis is based on
Asio and it does a good job of keeping in line with it.

> - Is the documentation helpful and clear?

The documentation within the sources (Doxygen) is helpful. The project
readme.md file can be improved. Having steps to build the project for
various operating systems would be nice. Providing cmake examples to
generate project solutions for Visual Studio might be a good idea.

Eg.

cmake -G "Visual Studio 17 2022" -A x64 -B bin64
-DCMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake

Although seemingly trivial to some, it helps reduce friction (and
frustration) to newcomers and increase adoption.

> - Did you try to use it? What problems or surprises did you encounter?

Yes, I built the included examples and tested them with a Redis
server. It may not be obvious to new users that most functions can
throw an exception. Some exception messages should be elaborated. For
instance, upon a failure to a server that refused a connection, the
exception message was "multiple exceptions".

Small nits…

May want to consider renaming the function 'healthy_checker' to 'health_check'.

Rename the abbreviated response variable 'resp' in the examples to
`response`. `resp` can be confusing in this context considering RESP
is an acronym for the communication protocol.

It might be helpful to use an enumeration (or something else) for
request commands. Not only would it provide type safety but it also
aids in knowing the commands without having to reference them in the
docs.

> - What is your evaluation of the implementation?

Overall I believe Aedis is well-designed and written, Marcelo has done
an excellent job. Unfortunately, this library requires c++ 17 with an
emphasis on c++ 20. This likely eliminates a large population of c++
users.

Best,
Mickey

-- 
Regards,
Vinnie
Follow me on GitHub: https://github.com/vinniefalco