feat: Add WithMinimumCacheTTL option#176
Conversation
3946491 to
fdc60bc
Compare
The WithMinimumCacheTTL option is used to control when cache entries are removed from the cache. The existing code sets this value to 15 seconds, and if this option is not passed, that is the fallback value. Signed-off-by: Marcelo E. Magallon <marcelo.magallon@grafana.com>
fdc60bc to
f45a2ff
Compare
|
|
||
| // WithMinimumCacheTTL allows setting the minimum amount of time that a cache | ||
| // entry must be valid for in order for it to be reused. | ||
| func WithMinimumCacheTTL(ttl time.Duration) ExchangeClientOpts { |
There was a problem hiding this comment.
I'm curious to hear your use case for making this configurable. The 15-second leeway acts as way to enforce that a token is purged from the cache a few seconds before its actual expiration. Would you actually like it to be purged sooner/later?
There was a problem hiding this comment.
I spoke to @mem offline and got some context around this. I understand the reasoning behind it and think it's a reasonable change.
I did share some minor concern about the naming here as WithMinimumCacheTTL doesn't quite convey what this is meant to achieve. That said, I'm not sure I can provide a substantially better alternative. Some ideas:
- WithExpirationBuffer
- WithEvictionThreshold
cinaglia
left a comment
There was a problem hiding this comment.
Functionality-wise this looks good to me. I understand the need for it.
My only suggestions here would be to rename the option to something hopefully more self-explanatory. I'd also suggest providing more context around why this is needed, perhaps by providing a real-life example.
|
@mem is this still something you'd like to wrap up? |
The WithMinimumCacheTTL option is used to control when cache entries are removed from the cache. The existing code sets this value to 15 seconds, and if this option is not passed, that is the fallback value.