Conversation
stevenh
left a comment
There was a problem hiding this comment.
Thanks for looking at this.
I've done a quick pass and adding some initial feedback
See commit 4a86003. |
|
Is this working for XREAD or XREADGROUP? I wonder this is not working well for them because the nested structure is different, sorry if this is out of scope. |
You raise a valid question @tk42 it does look like this will only work for Any thoughts @smotes ? |
|
Hi @stevenh. I propose we keep things as they stand. The Redis docs distinguish between entries and stream names in return types, so I felt it on the caller to parse out the reply in parts and know where to find the nested entries within. Consider the
And here's a little code snippet showing how to parse that as is (adapted from some code in a personal project). reply, err := sc.conn.Do("XREADGROUP", "GROUP", "some-group-name", "some-consumer-name",
"BLOCK", 3000, "COUNT", 10, "STREAMS", "some-stream-name", ">")
vs, err := redis.Values(reply, err)
if err != nil {
if errors.Is(err, redis.ErrNil) {
continue
}
return err
}
// Get the first and only value in the array since we're only
// reading from one stream "some-stream-name" here.
vs, err = redis.Values(vs[0], nil)
if err != nil {
return err
}
// Ignore the stream name as the first value as we already have
// that in hand! Just get the second value which is guaranteed to
// exist per the docs, and parse it as some stream entries.
entries, err := ParseEntries(vs[1], nil)
if err != nil {
return fmt.Errorf("error parsing entries: %w", err)
} |
|
That makes sense to me @tk42 does that answer address your concerns? |
|
Hi @smotes @stevenh
I’m feeling that users for type StreamEntry struct {
Stream string
Entries []Entry
}and its corresponded parsing function like this func StreamEntries(reply interface{}, err error) ([]StreamEntry, error) {
vss, err := redis.Values(reply, err)
if err != nil {
// error handling
return nil, fmt.Errorf("error parsing reply: %w", err)
}
var streamEntries []StreamEntry
for _, vs := range vss {
name, err := redis.String(vs[0], nil)
if err != nil {
// error handling
streamEntries = append(streamEntries, new(StreamEntry))
}
entries, err := ParseEntries(vs[1], nil)
if err != nil {
// error handling
streamEntries = append(streamEntries, new(StreamEntry))
}
streamEntries = append(streamEntries, StreamEntry{
Stream: name,
Entries: entries,
})
}
return streamEntries, nil
} |
This PR is related to issue #375 and adds a first pass at parsing replies containing stream entries into a struct.
I tested things against a Redis server v5.0.7, which is the first version containing the streams feature: https://redislabs.com/blog/redis-5-0-is-here/.