> On Feb. 1, 2017, 3:51 p.m., James Peach wrote: > > src/tests/containerizer/rootfs.cpp, line 90 > > <https://reviews.apache.org/r/53791/diff/8/?file=1589112#file1589112line90> > > > > What are you asking for here? This can fail for a number of reasons and > > `errno` describes them. Squashing that to a fixed string would be > > misleading. > > Jiang Yan Xu wrote: > I am saying we know "This can fail for a number of reasons and errno > describes them" because we are breaking the abstraction here. Perhaps my > arugment seems silly because `os::exists()` has a 9-line implementation and > we can look at the source. But if it's 900 lines or we can't see the source > the same principle still applies right? > > `os::exists()` current exposes a `bool` and expects it to be used to > interpret what's happened. If we peek inside the implementation and write > code based on the internals, it's likely to break when the implementation > changes. > > Perhaps `bool os::exists()` is currently lacking in ability to reveal > errors, should we then change it to `Try<bool> exists()`? If that's more > suitable as a TODO or we are not sure, I think we should just leave it as it > is right now. There are hundreds of other uses in the codebase which may be > susceptible to conditions like "the path does exist but we are not permitted > to access it" so maybe it's worth a JIRA to fix them anyways? (Of course that > shouldn't block our change here)
Filed https://issues.apache.org/jira/browse/MESOS-7052 for this. - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53791/#review163834 ----------------------------------------------------------- On Feb. 1, 2017, 3:51 p.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53791/ > ----------------------------------------------------------- > > (Updated Feb. 1, 2017, 3:51 p.m.) > > > Review request for mesos, Gilbert Song, Jie Yu, Kevin Klues, and Jiang Yan Xu. > > > Bugs: MESOS-6588 > https://issues.apache.org/jira/browse/MESOS-6588 > > > Repository: mesos > > > Description > ------- > > The dependencies for the programs we need in the Linux root > filesystem vary over time and across distributions. Since Stout > has support for parsing the library dependencies of ELF binaries, > use this to collect the necessary dependencies when constructing > a root filesystem for the tests. > > > Diffs > ----- > > src/tests/containerizer/rootfs.hpp 9648638c1d4b1f1c49708e058e9b930c5ae32138 > src/tests/containerizer/rootfs.cpp f11f126fa4f3130b6778bfe7e7d0f025a638f06e > > Diff: https://reviews.apache.org/r/53791/diff/ > > > Testing > ------- > > sudo make check (Fedora 24) > > > Thanks, > > James Peach > >
