> On Oct. 11, 2016, 12:23 a.m., Joseph Wu wrote: > > src/slave/slave.cpp, lines 4358-4365 > > <https://reviews.apache.org/r/52311/diff/8/?file=1524371#file1524371line4358> > > > > 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)`.
Don't I need to gaurd against empty string for user here?? Also, string is directly used without std here, so, std namespace is imported somewhere right? - Sivaram ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52311/#review152088 ----------------------------------------------------------- On Oct. 6, 2016, 3:30 a.m., Sivaram Kannan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52311/ > ----------------------------------------------------------- > > (Updated Oct. 6, 2016, 3:30 a.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 > >
