----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56524/#review165176 -----------------------------------------------------------
src/master/master.cpp (lines 2246 - 2247) <https://reviews.apache.org/r/56524/#comment236985> Can you follow the same format at the drop two lines above? ``` LOG(ERROR) << "Dropping " << call.type() << " call" << " from framework " << *framework << ": " << message; ``` Not yours, but just as an aside it seems like all of the drop logging should probably be at the warning instead of error level. Feel free to leave as is for now. src/master/master.cpp (lines 3248 - 3253) <https://reviews.apache.org/r/56524/#comment236986> I would suggest adding a drop overload for Suppress, so that you can just do: ``` drop(framework, suppress, "Suppression role is invalid: " + roleError->message); ``` This could do the `scheduler::Call` construction and call the version you've added in this patch. As it stands the call-sites need to do the `scheduler::Call` which is error prone. For example, the current code in this patch doesn't fill in the suppress portion of the call: ``` scheduler::Call call; call.set_type(scheduler::Call::SUPPRESS); call.mutable_suppress()->CopyFrom(suppress); // This is missed. ``` - Benjamin Mahler On Feb. 10, 2017, 1:42 a.m., Guangya Liu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56524/ > ----------------------------------------------------------- > > (Updated Feb. 10, 2017, 1:42 a.m.) > > > Review request for mesos, Benjamin Mahler and Jay Guo. > > > Bugs: MESOS-6638 > https://issues.apache.org/jira/browse/MESOS-6638 > > > Repository: mesos > > > Description > ------- > > Added `drop` for overload to avoid custom logging. > > > Diffs > ----- > > src/master/master.hpp 767ac48f2a65514a19c4a3f8ca7b35f0259f9aeb > src/master/master.cpp 620919ecfe85367b5c1281afc5216cc20e5e2e3c > > Diff: https://reviews.apache.org/r/56524/diff/ > > > Testing > ------- > > make > make check > > > Thanks, > > Guangya Liu > >
