zio-redis icon indicating copy to clipboard operation
zio-redis copied to clipboard

Introduce all Redis operations as type-safe methods of RedisExecutor trait

Open jdegoes opened this issue 3 years ago • 1 comments

In order to facilitate better use by layers, which want to consume other services by their interfaces, it would be ideal to add to RedisExecutor typed methods for all Redis operations, which simply encode/decode as appropriate.

This way, a service can depend on RedisExecutor, and use its operations, without also directly adding any dependencies into ZIO environment (which would have to be implemented for a service which is calling RedisExecutor).

jdegoes avatar Feb 15 '22 10:02 jdegoes

Recently, we have changed service structure and i suppose now it should be Redis trait. Am i right?

anatolysergeev avatar Feb 23 '22 10:02 anatolysergeev

Looking into this while migrating a codebase to ZIO 2, I have some questions:

  1. Is this the desired result, moving the following from the zio.redis package object:
trait Redis extends api.Connection
  with api.Geo 
  with api.Hashes
  ...
{
  def codec: Codec
  def executor: RedisExecutor
}
  1. Should accessor methods remain in the package object, or should they be moved to the zio.redis.Redis companion object?

  2. Where should options.XXX be placed?

rfmejia avatar Oct 29 '22 06:10 rfmejia

@rfmejia The idea is to move the api functions to Redis trait so that it "plays nice" with the rest of the ecosystem as @jdegoes described above. Eventually, we'll hide codec and executor methods, but I'd say it's not a concern of this particular issue. Helper packages, like options, should not be touched.

mijicd avatar Oct 30 '22 08:10 mijicd

i'm going to take this one but let me check if i understand what the end result should look like. Goal is to have Redis trait (and probably companion object as well) with all the api method declarations, and those declarations probably shouldn't "leak" dependencies i.e. something like

trait Redis {
  def auth(password: String): IO[RedisError, Unit]
  ...
}

which would require changes in api traits and the way commands are run. Or Redis trait could be just mixed in with api traits but all the declarations should change to not to "depend" on Redis. Or it could be i'm missing something, it's first time i'm looking at this codebase.

m-kalai avatar Jan 08 '23 20:01 m-kalai

@m-kalai That is correct: a single "service" trait without dependency leaks. You are free to reuse / redefine / introduce traits if you think they'd improve the current state. The issue mentions RedisExecutor as it was the only trait at that time.

mijicd avatar Jan 08 '23 21:01 mijicd