----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52309/#review151271 -----------------------------------------------------------
Looks like you've mastered the ReviewBoard tools :) This patch is almost good to go. src/slave/container_loggers/lib_logrotate.cpp (line 142) <https://reviews.apache.org/r/52309/#comment219622> I'd recommend adding this here: `Option<string> loggerUser = None();` And do the CommandInfo.user() checking once right below. Then there won't be duplicated logic when assigning the `user` field in the logger flags. src/slave/container_loggers/lib_logrotate.cpp (lines 174 - 177) <https://reviews.apache.org/r/52309/#comment219619> Spacing nit: ``` if (executorInfo.has_command() && executorInfo.command().has_user()) { outFlags.user = executorInfo.command().user(); } ``` This spacing applies (for the most part) even if you move it upwards (a.la. my previous comment). src/slave/container_loggers/lib_logrotate.cpp (line 237) <https://reviews.apache.org/r/52309/#comment219620> Spacing nit: four spaces here, not five :) Of course, you'll want to delete this if-statement anyway. src/slave/container_loggers/lib_logrotate.cpp (line 238) <https://reviews.apache.org/r/52309/#comment219621> Spacing nit: two spaces, not five. - Joseph Wu On Sept. 30, 2016, 6:33 p.m., Sivaram Kannan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52309/ > ----------------------------------------------------------- > > (Updated Sept. 30, 2016, 6:33 p.m.) > > > Review request for mesos and Joseph Wu. > > > Bugs: MESOS-5856 > https://issues.apache.org/jira/browse/MESOS-5856 > > > Repository: mesos > > > Description > ------- > > Pass the user variable from library to binary. > > > Diffs > ----- > > src/slave/container_loggers/lib_logrotate.cpp > 0ca2b3d7dbb57c11c0740aed3914a6b75329af99 > > Diff: https://reviews.apache.org/r/52309/diff/ > > > Testing > ------- > > 1. Run the mesos-logrotate-logger with un-priviledged user and verify whether > the logs are getting rotated. > 2. Run the mesos-logrotate-logger as root user and verify whether the logs > are getting rotated. > > > Thanks, > > Sivaram Kannan > >
