feat: enable cache for external tables using a cache control on metrics views #6265
Conversation
runtime/resolvers/metrics.go
Outdated
| return nil, false, nil | ||
| } | ||
|
|
||
| hasher := md5.New() |
There was a problem hiding this comment.
This is already being hashed in runtime/resolver.go so could we return the full pointer here?
There was a problem hiding this comment.
The values were already being hashed in the older implementation :
rill/runtime/resolvers/metrics.go
Line 94 in 9061a24
Since the key is now composed of two things (cache_key and query) I thought of just using a md5 hasher.
Do you mean we create a new struct and just return any from CacheKey instead of bytes ?
There was a problem hiding this comment.
I just meant if one way or another it could use the same hasher – e.g. it might return a combined []byte or a *drivers.Result, or accept a io.Writer, or something like that.
It's not an important comment, certainly you shouldn't do anything weird to achieve this (like returning a struct would probably be weird). Just wondered if there was a clean way to have just one hasher.
There was a problem hiding this comment.
Okay. Added another way to compute key without hasher.
Co-authored-by: Benjamin Egelund-Müller <[email protected]>
Co-authored-by: Benjamin Egelund-Müller <[email protected]>
No description provided.