> On July 20, 2016, 8:14 a.m., Joerg Schad wrote: > > src/launcher/fetcher.cpp, line 514 > > <https://reviews.apache.org/r/50199/diff/2/?file=1447745#file1447745line514> > > > > I personally would still use this cacheDirectory as parameter to > > createCacheDirectory for the following reason: we need to extract it anyhow > > and otherwise we do that at two different places. Feel free to drop if you > > have a different opinion. > > Greg Mann wrote: > With the current diff, the only extra work done in `createCacheDirectory` > is `if (fetcherInfo.has_cache_directory())`, which seems OK to me. I think > that we could definitely refactor the various fetching functions in this file > to clean things up, but perhaps we could do that in separate patches.
It is no issue :-), it just felt weird when reading the code that you first create the directory and afterwards extract the directory name from the protobuf. - Joerg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50199/#review142913 ----------------------------------------------------------- On July 20, 2016, 7:50 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50199/ > ----------------------------------------------------------- > > (Updated July 20, 2016, 7:50 p.m.) > > > Review request for mesos, Gilbert Song, Jie Yu, and Joerg Schad. > > > Bugs: MESOS-5845 > https://issues.apache.org/jira/browse/MESOS-5845 > > > Repository: mesos > > > Description > ------- > > The fetcher's launcher creates the fetcher cache > directory for each user immediately before an artifact > is fetched. In order to allow this directory to be > created by a different user than the user doing the > fetching, this patch factors out this directory > creation and places it before all fetches occur. > > > Diffs > ----- > > src/launcher/fetcher.cpp 0539b0182bd4a7178f103dddd1ab4fee8fc79eda > > Diff: https://reviews.apache.org/r/50199/diff/ > > > Testing > ------- > > `sudo make check` on both OSX and CentOS 7 was done at the end of this patch > chain. > > > Thanks, > > Greg Mann > >
