go-redis / cache

Cache library with Redis backend for Golang
https://redis.uptrace.dev/guide/go-redis-cache.html
BSD 2-Clause "Simplified" License
757 stars 95 forks source link

Return error explicitly when TTL = - 1 #84

Open r-novel opened 2 years ago

r-novel commented 2 years ago

Hello! 👋🏻

I've tried to use go-cache library for some permanently caching use cases and a little bit confused with Set function behaviour using ttl < 0 and found similar issue.

I suggest to return error value explicitly to avoid undefined behaviour on client applications.

I would appreciate for your reply. Best regard, Roman.

jslang commented 2 years ago

I also ran into issues with the behavior around TTL as it differs from the TTL behavior of the underlying redis client. It was surprising to say the least. The current behavior of just caching to the local cache and not sending to redis at all makes very little sense to me and lead to false positives during testing.

The reasoning explained in linked issue is not very illuminating either:

This library assumes that you always want some expiration time for cached items. I think it is a fine assumption in most cases and we should not change it.

I don't understand why this assumption is made, and doesn't match with the behavior of the library as is. Why would I ever want to store the value only in the local cache? If the assumption on the part of the cache is that some expiry is required, then agree with @r-novel that setting anything <= 0 should be treated as error.

But I'd also like to put my vote in for allowing permanent key setting through the cache API by allowing a TTL of 0 for no-expiry keys.