----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32351/#review78111 -----------------------------------------------------------
Ship it! 3rdparty/libprocess/include/process/http.hpp <https://reviews.apache.org/r/32351/#comment126747> Can you put a TODO here you wrote in the description of this review: ``` Long term I'd like to just have a http::request(..., Request) function rather than the overloads. ``` 3rdparty/libprocess/src/http.cpp <https://reviews.apache.org/r/32351/#comment126562> I would rather reorder the helpers so that I don't need to do forward declarations. 3rdparty/libprocess/src/http.cpp <https://reviews.apache.org/r/32351/#comment126740> Can you handle the DISCARDED case as well (instead of a CHECK). We never know if the impl of the socket recv will change or not. ``` if (!data.isReady()) { // XXX responses = decoder->decode("", 0); } else { ... } ``` 3rdparty/libprocess/src/http.cpp <https://reviews.apache.org/r/32351/#comment126736> Why change the name to `internal::_recv`? This function tries to decode the content received from the socket. Using the name `internal::decode` seems to be more appropriate. - Jie Yu On March 23, 2015, 11 p.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32351/ > ----------------------------------------------------------- > > (Updated March 23, 2015, 11 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 > ------- > > The caller must make the decision as to whether it expects to stream the > response via PIPE or get the full BODY response, otherwise they must > implement both cases, which is pretty hacky. > > So the approach here is to introduce a streaming:: namespace for this. Long > term I'd like to just have a http::request(..., Request) function rather than > the overloads. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/http.hpp > 2b366986b1f617e95bda94e07f2a0e532f5626f6 > 3rdparty/libprocess/src/http.cpp 276cecd17364989093e6eed8e97ff3a02fb0d0ef > 3rdparty/libprocess/src/tests/http_tests.cpp > 17fb092a851e128c137152fdce57e9fb10a08bf7 > > Diff: https://reviews.apache.org/r/32351/diff/ > > > Testing > ------- > > Added a test. > > > Thanks, > > Ben Mahler > >
