----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20493/#review48911 -----------------------------------------------------------
Sorry, one note I missed about cleaning up the existing error messaging in os::chmod. 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp <https://reviews.apache.org/r/20493/#comment85689> Sorry, missed this until I was looking at: https://reviews.apache.org/r/20494/ This should only be an ErrnoError(), since in general we like to avoid the caller provided information in the callee error messaging. If the caller then logs the error: LOG(ERROR) << "Failed to chmod 'path': " << chmod.error(); "Failed to chmod 'path': Failed to changed the mode of the path 'path'" We will be including redundant information, so we've opted to only include the error reason without caller provided information. In the case of os::su, we're ok to log the subcommand failures because they compose nicely with the caller: LOG(ERROR) << "Failed to su: " << su.error(); "Failed to su: Failed to getgid: unknown user" Does this all make sense? If so, let's update the error messaging in os::chmod to clean this up. - Ben Mahler On July 28, 2014, 7:12 p.m., Dominic Hamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20493/ > ----------------------------------------------------------- > > (Updated July 28, 2014, 7:12 p.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Bugs: MESOS-1236 > https://issues.apache.org/jira/browse/MESOS-1236 > > > Repository: mesos-git > > > Description > ------- > > see summary > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp > 08251724fbe45431db2c3637c6beec81f5744c4c > > Diff: https://reviews.apache.org/r/20493/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Dominic Hamon > >
