----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52234/#review150342 -----------------------------------------------------------
src/slave/containerizer/mesos/paths.hpp (lines 79 - 80) <https://reviews.apache.org/r/52234/#comment218281> ```The helper method to list all container IDs (including nested containers) from the container runtime directory.``` src/slave/containerizer/mesos/paths.hpp (line 81) <https://reviews.apache.org/r/52234/#comment218282> What's the differnence between Try<{}> and None()? Should we just use Try<vector> here? src/slave/containerizer/mesos/paths.cpp (lines 128 - 130) <https://reviews.apache.org/r/52234/#comment218283> YOu probably want to add a comment about what 'directory' is in this helper. src/slave/containerizer/mesos/paths.cpp (line 129) <https://reviews.apache.org/r/52234/#comment218285> Looks like `runtimeDir` never changes. Why we need to pass in that as a parameter? Can we just capture it? src/slave/containerizer/mesos/paths.cpp (lines 143 - 145) <https://reviews.apache.org/r/52234/#comment218284> If we rely on this ordering, Please make sure we commented on the interface in the header that the list returned is a result of pre-ordering walk (i.e., parent is inserted before its children). src/slave/containerizer/mesos/paths.cpp (lines 156 - 169) <https://reviews.apache.org/r/52234/#comment218286> The logic here is a little hard to follow. I am wondering if we should let the recursive helper takes the following parameter: ``` // Returns all the container IDs under the // given parent container, in pre-order. Try<vector<ContainerID>> helper( const ContainerID& parentContainerId) ``` You can use `getRuntimePath` to get the parent container runtime path first, do an `ls`, and construct the child container ID, then recursively dive into the even nested container. - Jie Yu On Sept. 24, 2016, 6:50 p.m., Gilbert Song wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52234/ > ----------------------------------------------------------- > > (Updated Sept. 24, 2016, 6:50 p.m.) > > > Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph > Wu, Kevin Klues, and Vinod Kone. > > > Repository: mesos > > > Description > ------- > > This is a helper function to collect all containerIds (include top > level and nested) from the runtime directory. > > > Diffs > ----- > > src/slave/containerizer/mesos/paths.hpp PRE-CREATION > src/slave/containerizer/mesos/paths.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/52234/diff/ > > > Testing > ------- > > > Thanks, > > Gilbert Song > >
