----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35919/#review89854 -----------------------------------------------------------
Ship it! Minor points, but otherwise shippable. I guess I'm still a little confused by the motivation. When would the firewall filter rule have more information about how to respond than the caller would? Can you give an example? 3rdparty/libprocess/include/process/firewall.hpp (line 57) <https://reviews.apache.org/r/35919/#comment142703> s/and/an/ s/option/Option/ Maybe worth clarifying that you do not return "200 OK" for rules that apply cleanly. 3rdparty/libprocess/include/process/firewall.hpp (line 59) <https://reviews.apache.org/r/35919/#comment142707> So if the caller wants to override the Response, they should check for a specific response type, extract the error message, and then handle it appropriately? 3rdparty/libprocess/include/process/firewall.hpp (line 86) <https://reviews.apache.org/r/35919/#comment142711> How about `Forbidden("Endpoint '" + request.path + "' is disabled")` so you don't start the message with a path. 3rdparty/libprocess/src/process.cpp (lines 2019 - 2020) <https://reviews.apache.org/r/35919/#comment142712> You're not logging the Response body anymore? - Adam B On June 29, 2015, 3:21 a.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35919/ > ----------------------------------------------------------- > > (Updated June 29, 2015, 3:21 a.m.) > > > Review request for mesos, Adam B and Till Toenshoff. > > > Bugs: MESOS-2877 > https://issues.apache.org/jira/browse/MESOS-2877 > > > Repository: mesos > > > Description > ------- > > see summary. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/firewall.hpp > f71d6541e48e2481c69f401b388977b153503962 > 3rdparty/libprocess/src/process.cpp > 52649fb90cdbefb495b68d0beb8c7f7e5ef6888e > > Diff: https://reviews.apache.org/r/35919/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Alexander Rojas > >
