----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36757/#review92966 -----------------------------------------------------------
3rdparty/libprocess/src/http.cpp (line 707) <https://reviews.apache.org/r/36757/#comment147245> We should do s/getSocket/createSocket/ to match the fact that you're calling 'Socket::create' under the covers, and the variable below is called 'create', etc. Also, any reason to name the function at all? We'd like to move to a static initialization world where we do stuff like this but it'll just look like: Try<Socket> create = [&url]() -> Try<Socket> { if (url.scheme == "http") { return Socket::create(Socket::POLL); } #ifdef USE_SSL_SOCKET if (url.scheme == "https") { return Socket::create(Socket::SSL); } #endif return Error("Unsupported URL scheme"); }(); 3rdparty/libprocess/src/tests/ssl_tests.cpp (line 1020) <https://reviews.apache.org/r/36757/#comment147243> Do you want to check that this payload and content-type actually make their way to the socket by doing a read and parsing into a request? 3rdparty/libprocess/src/tests/ssl_tests.cpp (lines 1025 - 1029) <https://reviews.apache.org/r/36757/#comment147241> Why do you need a std::stringstream for this? We can simplify with just a string, and also construct it like the protocol exposes: string buffer = "HTTP/1.1 200 OK\r\n" + "Content-Length : " + stringify(data.length()) + "\r\n" + "\r\n" + data; Same for test above too. 3rdparty/libprocess/src/tests/ssl_tests.cpp (line 1030) <https://reviews.apache.org/r/36757/#comment147244> Moot point if we just use a string (see comment above) but you should not have needed the `c_str()` here. Also, looks like we need a const& Future::get in our future (pun intended). - Benjamin Hindman On July 24, 2015, 5:49 p.m., Jojy Varghese wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36757/ > ----------------------------------------------------------- > > (Updated July 24, 2015, 5:49 p.m.) > > > Review request for mesos, Joris Van Remoortere and Timothy Chen. > > > Bugs: MESOS-3093 > https://issues.apache.org/jira/browse/MESOS-3093 > > > Repository: mesos > > > Description > ------- > > Current http implementation lacks a https interface. This change exposes > SSL socket for "https" URL scheme. > > JIRA: MESOS-3093 > > > Diffs > ----- > > 3rdparty/libprocess/src/http.cpp d1685799f4c53e067d0812e037e171324ee7195f > 3rdparty/libprocess/src/tests/ssl_tests.cpp > 2fe50601615b0bee57bd3e05dc9c932f93ca7477 > > Diff: https://reviews.apache.org/r/36757/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jojy Varghese > >
