Feat: Align go-redis TimeseriesCmdable
#422
Conversation
…SDeleteRule, TSGet, TSInfo, TSInfoWithArgs, TSMAdd, TSQueryIndex, TSRevRange
…any]any WORKAROUND: manually convert map[string]any to map[any]any to make go-redis test PASS we don't use (*RedisMessage).ToAny() on map type because it will convert ele to map[string]any
Because tests in go-redis use both of them.
| } | ||
| cmds.TsMrangeTotimestamp(_cmd).Filter(filterExpr...) | ||
| if options.GroupByLabel != nil { | ||
| // FIXME: Wrong API definition: REDUCE |
There was a problem hiding this comment.
Note that our builder API TsMrevrange is wrong (same as below)
| } | ||
| cmds.TsMrevrangeTotimestamp(_cmd).Filter(filterExpr...) | ||
| if options.GroupByLabel != nil { | ||
| // FIXME: Wrong API definition: REDUCE |
There was a problem hiding this comment.
Note that our builder API TsMrevrange is wrong,
according to the doc, it should be:
cmds.TsMrevrangeFilter(_cmd).Groupby(str(options.GroupByLabel).Reduce(str(options.Reducer))
Not:
cmds.TsMrevrangeFilter(_cmd).Groupby(str(options.GroupByLabel), "REDUCE", str(options.Reducer))
unknowntpo
force-pushed
the
feat-align-go-redis-timeseries
branch
from
4a32f6b to
5f22cd6
Compare
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #422 +/- ##
==========================================
- Coverage 97.03% 96.07% -0.96%
==========================================
Files 76 76
Lines 31445 32162 +717
==========================================
+ Hits 30512 30899 +387
- Misses 788 1083 +295
- Partials 145 180 +35 ☔ View full report in Codecov by Sentry. |
rueidiscompat/command.go
Outdated
| } | ||
| strIntMap := make(map[string]any, len(m)) | ||
| for k, ele := range m { | ||
| var v any = rueidis.Nil |
There was a problem hiding this comment.
This rueidis.Nil is added to make tests pass, should we use rueidis.Nil or let it be nil if ele.IsNil() is true ?
There was a problem hiding this comment.
I think nil is more reasonable.
rueidiscompat/command.go
Outdated
| } | ||
| strIntMap := make(map[string]any, len(m)) | ||
| for k, ele := range m { | ||
| var v any = rueidis.Nil |
There was a problem hiding this comment.
I think nil is more reasonable.
message.go
Outdated
| // AsAnyAnyMap converts RedisMessage to map[any]any. | ||
| // we don't use (*RedisMessage).ToAny() on map type because it will convert ele | ||
| // to map[string]any | ||
| func (m *RedisMessage) AsAnyAnyMap() (map[any]any, error) { |
There was a problem hiding this comment.
map[any]any is too hard to use. I don't think we should have this function.
There was a problem hiding this comment.
Okay, I'll remove it.
This PR implemented adapter for
go-redisTimeseriesCmdable.