> On July 19, 2016, 8:51 p.m., Joerg Schad wrote:
> > src/launcher/fetcher.cpp, line 381
> > <https://reviews.apache.org/r/50199/diff/1/?file=1447503#file1447503line381>
> >
> >     Does it make sense to add a check here to document and check that 
> > precondition?

Yep! :)


> On July 19, 2016, 8:51 p.m., Joerg Schad wrote:
> > src/launcher/fetcher.cpp, line 437
> > <https://reviews.apache.org/r/50199/diff/1/?file=1447503#file1447503line437>
> >
> >     Not an issue: just making sure I understood this correctly :-): if 
> > there is no user we do not need to create directory i.e., a global/non 
> > user-specific cache dir is created upfront?

While I do think this was a correct assumption (that the directory will already 
exist if no task user is specified), the `FetcherInfo` passed to the fetcher 
binary should always contain a cache directory if the cache is going to be used 
(see 
https://github.com/apache/mesos/blob/dc4a4ae9b264e66ff891c6b00181d8d501514642/src/launcher/fetcher.cpp#L368-L370),
 so I think it makes more sense to follow the original behavior found here, 
where we call `os::mkdir` whether or not a task user has been supplied.


- Greg


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50199/#review142829
-----------------------------------------------------------


On July 19, 2016, 11:10 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50199/
> -----------------------------------------------------------
> 
> (Updated July 19, 2016, 11:10 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
> 
>

Reply via email to