> On Aug. 17, 2016, 6:01 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/launcher.hpp, lines 87-89 > > <https://reviews.apache.org/r/51167/diff/3/?file=1476441#file1476441line87> > > > > Returning a relative path here sounds weird to me. The caller needs to > > do another path::join to get the final result. > > > > I'd suggest we made this method pure virtual. Still keep the common > > protected method in Launcher. Finally, implement this pure virtual method > > in both LinuxLauncher and PosixLauncher. > > > > LinuxLauncher and PosixLauncher can have a field member called > > `runtimeDirectory` (or `flags` directly, up to you). In the > > `LinuxLauncher::create`, you can do some validation if needed. > > > > You don't need the `name()` pure virtual method any more. I'd use a > > constexpr in LinuxLauncher and PosixLauncher for that.
No problem, I made it a relative path on BenH's suggestions. I agree that it should be absolute though. Pushing a fix soon. - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51167/#review146016 ----------------------------------------------------------- On Aug. 17, 2016, 2:30 p.m., Kevin Klues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51167/ > ----------------------------------------------------------- > > (Updated Aug. 17, 2016, 2:30 p.m.) > > > Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone. > > > Bugs: MESOS-6051 > https://issues.apache.org/jira/browse/MESOS-6051 > > > Repository: mesos > > > Description > ------- > > The functions added in this commit are not currently used anywhere in > the code base. However, they are required for some changes we are > making in order to persist container state in cases where an agent > crashes. The changes to use these function will come in subsequent > commits. > > > Diffs > ----- > > src/slave/containerizer/mesos/launcher.hpp > bf435e3a9c150648336a1becf2f075fa183428bd > src/slave/containerizer/mesos/launcher.cpp > 9efe8474a2210957ce256fc08cb35694194213c3 > src/slave/containerizer/mesos/linux_launcher.hpp > c1852226c74bc611d045be721e284141e59adcd9 > src/slave/containerizer/mesos/linux_launcher.cpp > 95dee95c5e6e613e526c92d8729ae5583c8b58f1 > src/tests/containerizer/launcher.hpp > 7e5c243efad11d04e70b36876b2ed4db82666d31 > > Diff: https://reviews.apache.org/r/51167/diff/ > > > Testing > ------- > > GTEST_FILTER="" make -j check > src/mesos-tests > sudo src/mesos-tests > > > Thanks, > > Kevin Klues > >
