> On March 18, 2015, 11:49 p.m., Jie Yu wrote: > > 3rdparty/libprocess/src/process.cpp, lines 1149-1151 > > <https://reviews.apache.org/r/31930/diff/2/?file=896916#file896916line1149> > > > > Does this comment still apply? Looks like we cannot write empty string. > > Maybe a CHECK? > > Ben Mahler wrote: > Chatted with Jie, this is the clunkiness that is induced from introducing > Option<string> per benh and joris' comments. > > Callers **still** have to think about the empty string case because the > type can express it. The alternative is we add an implicit invariant that > empty reads are never returned, which seems pretty hacky (all callers will > CHECK against it). > > Will keep this as a special case for now. But we may want to circle back > to Future<string>, as it seems clearer now that we'll probably not want to > use a typed Stream<T> abstraction for this case. Rather, for raw byte streams > we probably want to stick with a specialized abstraction (like Pipe). > > Jie Yu wrote: > Chatted with BenM on this. We probably want two abstractions here for > streaming. > 1) Typed object stream. I.e., Stream<T>. And there is a 1:1 mapping > between reads and writes. > 2) Untyped byte stream. I.e., Pipe. No 1:1 mapping between reads and > writes (i.e., we can combine writes). > > For 1), it does makes sense to return an Future<Option<T>> when read. In > fact, we have to because we need a way to tell if it's an end-of-file or an > error. > > For 2), I don't think returning Future<Option<string>> is a good idea. > The stream is untyped bytes, returning an empty string in the non EOF case > shouldn't be possible because it should instead be blocked. Therefore, as > benM said, intead of relying on caller to do the CHECK every time, why not > just use Future<string> and use empty string to signify the EOF.
It looks like [Future<string> Socket::recv()](https://github.com/apache/mesos/blob/0.22.0-rc4/3rdparty/libprocess/include/process/socket.hpp#L87) takes the empty string <-> EOF approach as well, although it's not documented ;) ;) I'll circle back to Future<string> for now per the original approach. If we want to follow up to use Option<string> we should probably look at the Socket API as well to make these consistent. :) - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31930/#review76931 ----------------------------------------------------------- On March 19, 2015, 9:33 p.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31930/ > ----------------------------------------------------------- > > (Updated March 19, 2015, 9:33 p.m.) > > > Review request for mesos, Benjamin Hindman, Jie Yu, and Joris Van Remoortere. > > > Bugs: MESOS-2438 > https://issues.apache.org/jira/browse/MESOS-2438 > > > Repository: mesos > > > Description > ------- > > See MESOS-2438 for the motivation. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/http.hpp > 10143fdbd5eb43f968c41957a2335f96a097d82e > 3rdparty/libprocess/src/http.cpp 7c0cee404645e1dd1a9253f85883aefa29a5f999 > 3rdparty/libprocess/src/process.cpp > e7b029ba97e640c2102548c190ba62b30602f43d > 3rdparty/libprocess/src/tests/http_tests.cpp > 800752a57230d7768d3d741fef87f6283757e424 > > Diff: https://reviews.apache.org/r/31930/diff/ > > > Testing > ------- > > * added tests > * make check > > > Thanks, > > Ben Mahler > >
