Should Rueidis store implementation return string?
rueian opened this issue · 3 comments
Thank you for adding rueidis integration. It is my honor that rueidis can be integrated into this popular cache library.
Just wondering why doesn't Rueidis store implementation return a simple string instead of a rueidis.RedisResult
? Are there some considerations I missed? I thought It would be much friendly if it returns a string to users like what go-redis integration does.
For example:
func (s *RueidisStore) Get(ctx context.Context, key any) (any, error) {
cmd := s.client.B().Get().Key(key.(string)).Cache()
str, err := s.client.DoCache(ctx, cmd, s.options.ClientSideCacheExpiration).ToString()
if rueidis.IsRedisNil(err) {
err = lib_store.NotFoundWithCause(err)
}
return str, err
}
Also, I noticed that there are some room for improvement:
- Each tag operation in
setTags
can be manually pipelined:
func (s *RueidisStore) setTags(ctx context.Context, key any, tags []string) {
ttl := 720 * time.Hour
for _, tag := range tags {
tagKey := fmt.Sprintf(RueidisTagPattern, tag)
s.client.DoMulti(ctx,
s.client.B().Sadd().Key(tagKey).Member(key.(string)).Build(),
s.client.B().Expire().Key(tagKey).Seconds(int64(ttl.Seconds())).Build(),
)
}
}
- I am going to release a new
CacheTTL()
method this weekend. It can retrieve the remaining client-side TTL of a cached message. Then, theGetWithTTL
can be just like this:
func (s *RueidisStore) GetWithTTL(ctx context.Context, key any) (any, time.Duration, error) {
cmd := s.client.B().Get().Key(key.(string)).Cache()
res := s.client.DoCache(ctx, cmd).Cache(), s.options.ClientSideCacheExpiration)
ttl := res.CacheTTL()
str, err := res.ToString()
if rueidis.IsRedisNil(err) {
err = lib_store.NotFoundWithCause(err)
}
return str, time.Duration(ttl) * time.Second, err
}
What do you think? I am happy to open a PR for the above changes. 😊
Hello @rueian,
I agree with your proposals, if you want to open a PR you're mostly welcome, elsewhere I could find some time on next week to do these changes.
Thank you for this and to have built Rueidis :)
Thank you for this implementation.
Closing this PR for now but feel free to reopen if you still see anything that can be improved.