Command channel output #499
Conversation
|
Jenkins is yelling at me because one of the intermediate commits doesn't build, should I change the history to fix that? |
@mappum yeah ideally. though feel free to squash commits together if its not super easy. |
| cr.reader = r | ||
| } | ||
|
|
||
| n, err := cr.reader.Read(p) |
There was a problem hiding this comment.
Are you sure? It looks like that would error if len(p) is greater than the length of one of the readers.
There was a problem hiding this comment.
It'll stop on io.EOF and return io.ErrUnexpectedEOF (which we can ignore). The thing i'm worried about is that Read call not reading all it's supposed to. there's no guarantee the network won't just give it a few bytes and return.
Looking closer at this function, looks like if there are any bytes read, we keep the reader, so we just read again. if so then that may be fine (i thought we lost the reader right away)
|
Yay! I'll be able to write ping now! |
| writer.WriteString("HTTP/1.1 200 OK\r\n") | ||
| writer.WriteString(contentTypeHeader + ": application/json\r\n") | ||
| writer.WriteString(transferEncodingHeader + ": chunked\r\n") | ||
| writer.WriteString(channelHeader + ": 1\r\n\r\n") |
There was a problem hiding this comment.
nice :) love how simple http is
didn't know about \r\n. guess that makes sense for the time.
There was a problem hiding this comment.
It was probably chosen just so Tim Berners-Lee could test out his server via telnet :P
There was a problem hiding this comment.
yeah. he didn't have protobufs. :]
|
@mappum solid PR! LGTM after making commits pass, and comments (that one |
core/commands: Refactored command marshalers
…se before request is done
core/commands: Don't use pointers for Type field
mappum
force-pushed
the
command-channels
branch
from
a247a67
to
2816ed0
Compare
|
Sweet, i'm going to merge this! Thanks! 👍 |
This PR lets commands return channels, which allows for streaming object output. Now the add command will show hashes of files as they are added. 😄
It's pretty satisfying to watch it add the whole go-ipfs directory.
Thanks to the sharness tests for catching various edge-case bugs I wouldn't have noticed. 👍