----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36402/#review91843 -----------------------------------------------------------
Have not gone through the whole review yet. It would be worth discussing this with @bmahler more on what he thinks about my comment on refactoring acceptMediaType and then go over this again based on his feedback. 3rdparty/libprocess/src/http.cpp (line 125) <https://reviews.apache.org/r/36402/#comment145496> Whoops ! Let's return false as earlier. Also add a test-case for empty check. ( Accept-Encoding ) 3rdparty/libprocess/src/http.cpp <https://reviews.apache.org/r/36402/#comment145495> Can we get these comments back ? 3rdparty/libprocess/src/http.cpp (lines 135 - 136) <https://reviews.apache.org/r/36402/#comment145510> Can we put these comments back ? 3rdparty/libprocess/src/http.cpp <https://reviews.apache.org/r/36402/#comment145511> Same here. 3rdparty/libprocess/src/http.cpp (line 163) <https://reviews.apache.org/r/36402/#comment145509> How do we intend to use this method ? Let's say we have 2 media types that we support "application/json" and "application/x-protobuf". I would need to call acceptMediaType twice ( in a loop ): - acceptMediaType("application/json") , if it returns false , call again with - acceptMediaType("application/x-protobuf"). Return back a 415 if both of these return false. Why not we change the method signature to: Try<std::string> Request::findFirstAcceptedType(const std::vector<std::string>& acceptedMediaTypes) that just returns a string with the first match and an Error if none of the options in acceptedMediaTypes match. We did not face the same issue with Accept-Encoding may be because gzip is the only encoding we support ( and not deflate ? ) 3rdparty/libprocess/src/http.cpp (line 196) <https://reviews.apache.org/r/36402/#comment145512> Nit-pick : Modify the comment string here, gzip;q=0.0 does not hold good here. - Anand Mazumdar On July 15, 2015, 11:54 p.m., Isabel Jimenez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36402/ > ----------------------------------------------------------- > > (Updated July 15, 2015, 11:54 p.m.) > > > Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and > Vinod Kone. > > > Repository: mesos-incubating > > > Description > ------- > > Adding a method for Accept header in request + refactor of Accept-Encoding > > > Diffs > ----- > > 3rdparty/libprocess/include/process/http.hpp 72b6d27 > 3rdparty/libprocess/src/http.cpp d168579 > 3rdparty/libprocess/src/tests/http_tests.cpp 01f243c > > Diff: https://reviews.apache.org/r/36402/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Isabel Jimenez > >
