Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for TS.MGET with options. minor refactor and go fmt. #67

Merged
merged 2 commits into from
Jul 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ jobs:
working_directory: /go/src/github.com/RedisTimeSeries/redistimeseries-go
steps:
- checkout
- run: make checkfmt
- run: make get
- run: make coverage
- run: bash <(curl -s https://codecov.io/bash) -t ${CODECOV_TOKEN}
Expand Down
11 changes: 11 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,25 @@ GOCLEAN=$(GOCMD) clean
GOTEST=$(GOCMD) test
GOGET=$(GOCMD) get
GOMOD=$(GOCMD) mod
GOFMT=$(GOCMD) fmt

.PHONY: all test coverage
all: test coverage

checkfmt:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@filipecosta90 While this will work, an easier way of linting in go would be using https://github.com/golangci/golangci-lint

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danni-m I've used this go fmt approach as a quick fix/quick enforcing of fmt. Will revise the required changes to use golangci-lint and move towards that afterwards. wdyt of adding this quick check to be safeguarded and with more time moving to something more correct/sophisticated?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@filipecosta90 sounds good to me, just want to make sure you are aware that this tool exists.

@echo 'Checking gofmt';\
bash -c "diff -u <(echo -n) <(gofmt -d .)";\
EXIT_CODE=$$?;\
if [ "$$EXIT_CODE" -ne 0 ]; then \
echo '$@: Go files must be formatted with gofmt'; \
fi && \
exit $$EXIT_CODE

get:
$(GOGET) -t -v ./...

test: get
$(GOFMT) ./...
$(GOTEST) -race -covermode=atomic ./...

coverage: get test
Expand Down
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,12 @@ func main() {
| [TS.INCRBY/TS.DECRBY](https://oss.redislabs.com/redistimeseries/commands/#tsincrbytsdecrby) | [IncrBy](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.IncrBy) / [DecrBy](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.DecrBy) |
| [TS.CREATERULE](https://oss.redislabs.com/redistimeseries/commands/#tscreaterule) | [CreateRule](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.CreateRule) |
| [TS.DELETERULE](https://oss.redislabs.com/redistimeseries/commands/#tsdeleterule) | [DeleteRule](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.DeleteRule) |
| [TS.RANGE](https://oss.redislabs.com/redistimeseries/commands/#tsrange) | [RangeWithOptions](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.RangeWithOptions) |
| [TS.MRANGE](https://oss.redislabs.com/redistimeseries/commands/#tsmrange) | [MultiRangeWithOptions](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.MultiRangeWithOptions) |
| [TS.RANGE](https://oss.redislabs.com/redistimeseries/commands/#tsrangetsrevrange) | [RangeWithOptions](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.RangeWithOptions) |
| [TS.REVRANGE](https://oss.redislabs.com/redistimeseries/commands/#tsrangetsrevrange) | N/A |
| [TS.MRANGE](https://oss.redislabs.com/redistimeseries/commands/#tsmrangetsmrevrange) | [MultiRangeWithOptions](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.MultiRangeWithOptions) |
| [TS.MREVRANGE](https://oss.redislabs.com/redistimeseries/commands/#tsmrangetsmrevrange) | N/A |
| [TS.GET](https://oss.redislabs.com/redistimeseries/commands/#tsget) | [Get](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.Get) |
| [TS.MGET](https://oss.redislabs.com/redistimeseries/commands/#tsmget) | [MultiGet](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.MultiGet) |
| [TS.MGET](https://oss.redislabs.com/redistimeseries/commands/#tsmget) | <ul><li>[MultiGet](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.MultiGet)</li><li> [MultiGetWithOptions](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.MultiGetWithOptions) </li> </ul> |
| [TS.INFO](https://oss.redislabs.com/redistimeseries/commands/#tsinfo) | [Info](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.Info) |
| [TS.QUERYINDEX](https://oss.redislabs.com/redistimeseries/commands/#tsqueryindex) | [QueryIndex](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.QueryIndex) |

Expand Down
19 changes: 10 additions & 9 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ func NewClient(addr, name string, authPass *string) *Client {
return ret
}


// NewClientFromPool creates a new Client with the given pool and client name
func NewClientFromPool(pool *redis.Pool, name string) *Client {
ret := &Client{
Expand Down Expand Up @@ -267,21 +266,23 @@ func (client *Client) Get(key string) (dataPoint *DataPoint,
// MultiGet - Get the last sample across multiple time-series, matching the specific filters.
// args:
// filters - list of filters e.g. "a=bb", "b!=aa"
func (client *Client) MultiGet(filters ...string) (ranges []Range,
err error) {
func (client *Client) MultiGet(filters ...string) (ranges []Range, err error) {
return client.MultiGetWithOptions(DefaultMultiGetOptions, filters...)
}

// MultiGetWithOptions - Get the last samples matching the specific filters.
// args:
// multiGetOptions - MultiGetOptions options. You can use the default DefaultMultiGetOptions
// filters - list of filters e.g. "a=bb", "b!=aa"
func (client *Client) MultiGetWithOptions(multiGetOptions MultiGetOptions, filters ...string) (ranges []Range, err error) {
filipecosta90 marked this conversation as resolved.
Show resolved Hide resolved
conn := client.Pool.Get()
defer conn.Close()
var reply interface{}
if len(filters) == 0 {
return
}
args := []interface{}{"FILTER"}
for _, filter := range filters {
args = append(args, filter)
}

args := createMultiGetCmdArguments(multiGetOptions, filters)
reply, err = conn.Do("TS.MGET", args...)

if err != nil {
return
}
Expand Down
62 changes: 62 additions & 0 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,68 @@ func TestClient_MultiGet(t *testing.T) {
}
}

func TestClient_MultiGetWithOptions(t *testing.T) {
client.FlushAll()
key1 := "test_TestClient_MultiGet_key1"
key2 := "test_TestClient_MultiGet_key2"
labels1 := map[string]string{
"metric": "cpu",
"country": "US",
}
labels2 := map[string]string{
"metric": "cpu",
"country": "UK",
}

_, err := client.AddWithOptions(key1, 1, 5.0, CreateOptions{Labels: labels1})
if err != nil {
t.Errorf("TestClient_MultiGetWithOptions Add() error = %v", err)
return
}
_, err = client.Add(key1, 2, 15.0)
_, err = client.Add(key1, 3, 15.0)

_, err = client.AddWithOptions(key2, 1, 5.0, CreateOptions{Labels: labels2})

if err != nil {
t.Errorf("TestClient_MultiGetWithOptions Add() error = %v", err)
return
}

type fields struct {
Pool ConnPool
Name string
}
type args struct {
filters []string
}
tests := []struct {
name string
fields fields
args args
wantRanges []Range
wantErr bool
}{
{"multi key", fields{client.Pool, "test"}, args{[]string{"metric=cpu", "country=UK"}}, []Range{Range{key2, map[string]string{"country": "UK", "metric": "cpu"}, []DataPoint{DataPoint{1, 5.0}}}}, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
client := &Client{
Pool: tt.fields.Pool,
Name: tt.fields.Name,
}
gotRanges, err := client.MultiGetWithOptions(*NewMultiGetOptions().SetWithLabels(true), tt.args.filters...)
if (err != nil) != tt.wantErr {
t.Errorf("MultiGet() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(gotRanges, tt.wantRanges) {
t.Errorf("MultiGet() gotRanges = %v, want %v", gotRanges, tt.wantRanges)
}
})
}
}

func TestClient_Range(t *testing.T) {
client.FlushAll()
key1 := "TestClient_Range_key1"
Expand Down
34 changes: 34 additions & 0 deletions multiget.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package redis_timeseries_go

// MultiGetOptions represent the options for querying across multiple time-series
type MultiGetOptions struct {
WithLabels bool
}

// MultiGetOptions are the default options for querying across multiple time-series
var DefaultMultiGetOptions = MultiGetOptions{
WithLabels: false,
}

func NewMultiGetOptions() *MultiGetOptions {
return &MultiGetOptions{
WithLabels: false,
}
}

func (mgetopts *MultiGetOptions) SetWithLabels(value bool) *MultiGetOptions {
mgetopts.WithLabels = value
return mgetopts
}

func createMultiGetCmdArguments(mgetOptions MultiGetOptions, filters []string) []interface{} {
args := []interface{}{}
if mgetOptions.WithLabels == true {
args = append(args, "WITHLABELS")
}
args = append(args, "FILTER")
for _, filter := range filters {
args = append(args, filter)
}
return args
}
28 changes: 28 additions & 0 deletions multiget_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package redis_timeseries_go

import (
"reflect"
"testing"
)

func Test_createMultiGetCmdArguments(t *testing.T) {
type args struct {
mgetOptions MultiGetOptions
filters []string
}
tests := []struct {
name string
args args
want []interface{}
}{
{"default", args{DefaultMultiGetOptions, []string{"labels!="}}, []interface{}{"FILTER", "labels!="}},
{"withlabels", args{*(NewMultiGetOptions().SetWithLabels(true)), []string{"labels!="}}, []interface{}{"WITHLABELS", "FILTER", "labels!="}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := createMultiGetCmdArguments(tt.args.mgetOptions, tt.args.filters); !reflect.DeepEqual(got, tt.want) {
t.Errorf("createMultiGetCmdArguments() = %v, want %v", got, tt.want)
}
})
}
}
1 change: 0 additions & 1 deletion pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ func testOnBorrow(c redis.Conn, t time.Time) (err error) {
return err
}


func (p *MultiHostPool) Close() (err error) {
p.Lock()
defer p.Unlock()
Expand Down
2 changes: 1 addition & 1 deletion reply_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ package redis_timeseries_go
import (
"errors"
"fmt"
"strconv"
"github.com/gomodule/redigo/redis"
"strconv"
)

func toAggregationType(aggType interface{}) (aggTypeStr AggregationType, err error) {
Expand Down
4 changes: 2 additions & 2 deletions reply_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func TestParseRangesSingleDataPoint(t *testing.T) {
false,
},
{"incorrect input ( 2 elements on inner array )",
args{[]interface{}{[]interface{}{[]byte("serie 1"), []interface{}{},}}},
args{[]interface{}{[]interface{}{[]byte("serie 1"), []interface{}{}}}},
[]Range{},
true,
},
Expand Down Expand Up @@ -201,7 +201,7 @@ func TestParseRanges(t *testing.T) {
false,
},
{"incorrect input ( 2 elements on inner array )",
args{[]interface{}{[]interface{}{[]byte("serie 1"), []interface{}{},}}},
args{[]interface{}{[]interface{}{[]byte("serie 1"), []interface{}{}}}},
[]Range{},
true,
},
Expand Down