Conversation
hsanjuan
left a comment
hsanjuan
left a comment
There was a problem hiding this comment.
Couple of comments, otherwise LGTM
| return nil, resp.Error | ||
| } | ||
|
|
||
| outchan := make(chan pstore.PeerInfo, 4) |
There was a problem hiding this comment.
make 4 a constant (with a comment), or add comment on why 4
|
|
||
| go func() { | ||
| <-ctx.Done() | ||
| resp.Close() |
There was a problem hiding this comment.
I fail to see why not defer resp.Close() in the previous go-routine but I assume there is a reason?
There was a problem hiding this comment.
Yes, json.NewDecoder(resp.Output) will hang until the resp is closed. So this thing breaks out the NewDecoder.
There was a problem hiding this comment.
Wait, if NewDecoder hangs, then it's going to hang until someone cancels the context, because resp.Close() is not called until then, and the context won't be cancelled because NewDecoder is hanging ?
There was a problem hiding this comment.
We self cancel/exit when responses from the endpoint end. If someone wants to exit early he has to cancel context.
There was a problem hiding this comment.
Sorry, not NewDecoder but decoder.Decode.
|
I'm a bit afraid of the added dependencies -- let's revisit this when we have the first parts of the Core API implemented here. |
|
@lgierth agreed |
|
Hi! Any update on this? Currently, how can I use |
3 participants
No description provided.