----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52311/#review152088 -----------------------------------------------------------
src/slave/slave.cpp (line 4197) <https://reviews.apache.org/r/52311/#comment220815> Period at the end of all comments. The cpp linter should be catching this for you (a pre-commit git hook), so make sure you've run `./bootstrap` in your development directory. src/slave/slave.cpp (lines 4209 - 4210) <https://reviews.apache.org/r/52311/#comment220816> This custom executor user should only be set if it isn't already set. Also, you need to guard the `loggerUser.get()` with a `loggerUser.isSome()`. src/slave/slave.cpp (line 4221) <https://reviews.apache.org/r/52311/#comment220818> Same here, you need to guard the `.get()` with a `.isSome()`. src/slave/slave.cpp (lines 4358 - 4365) <https://reviews.apache.org/r/52311/#comment220817> It is now possible replace this block with: ``` executor.mutable_command()->add_arguments("--user=" + loggerUser.get()); ``` Considering the variable's name and how it is used in this function, you should consider renaming it `executorUser` and changing the type from `Option<string>` to `string`. Then, each place it needs to be set, you check `if (flags.switch_user)`. This is clearer than guarding it with `if (executorUser.isSome)`. - Joseph Wu On Oct. 5, 2016, 8:30 p.m., Sivaram Kannan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52311/ > ----------------------------------------------------------- > > (Updated Oct. 5, 2016, 8:30 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 value from executor of switch_user flag is set. > > > Diffs > ----- > > src/slave/slave.cpp d30001bc0d1798311ba3966f67aadc6be2c92306 > > Diff: https://reviews.apache.org/r/52311/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 > >
