----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42374/#review115100 -----------------------------------------------------------
Ship it! src/tests/environment.cpp (line 365) <https://reviews.apache.org/r/42374/#comment175964> We should really add our own `os::which` so that we can easily differentiate the error cases here. In particular, what if just `os::system` fails? or `which` fails but `logrotate` is still accesible on the path? Having something like a `os::which` which returns a `Result`, either some (it exists on the path), none (it doesn't exist on the path), or `Error` (there was an error determining that information) would be great. While an `Error` or none would still do the same thing here, it would probably give a better error message. Feel free to add these kinds of primitives on the journey in the future! - Benjamin Hindman On Jan. 16, 2016, 12:54 a.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42374/ > ----------------------------------------------------------- > > (Updated Jan. 16, 2016, 12:54 a.m.) > > > Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Artem > Harutyunyan. > > > Bugs: MESOS-4136 > https://issues.apache.org/jira/browse/MESOS-4136 > > > Repository: mesos > > > Description > ------- > > The `logrotate` binary is used by the non-default `RotatingContainerLogger` > module. On some systems, `logrotate` is not provided by default. > > > Diffs > ----- > > src/tests/environment.cpp 20218a086baefcefb310eb45ed9024e5425ce787 > > Diff: https://reviews.apache.org/r/42374/diff/ > > > Testing > ------- > > See later reviews. > > > Thanks, > > Joseph Wu > >
