----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30037/#review69514 -----------------------------------------------------------
More thoughts on the protobufs. include/mesos/fetcher/fetcher.proto <https://reviews.apache.org/r/30037/#comment114218> Maybe it would be better to call this BYPASS_CACHE, and then the other two could be combined into USE_CACHE. At that point it almost sounds like a boolean. Is there a discussion somewhere about what this variable should express? include/mesos/fetcher/fetcher.proto <https://reviews.apache.org/r/30037/#comment114217> How does this fail for an item not present in the cache? Why wouldn't you just want to check for presence in the cache, retrieve if present and download&cache if absent? include/mesos/fetcher/fetcher.proto <https://reviews.apache.org/r/30037/#comment114219> Why make this required, instead of optional with a default action? include/mesos/fetcher/fetcher.proto <https://reviews.apache.org/r/30037/#comment114220> You could probably get away with `cache_filename` instead include/mesos/fetcher/fetcher.proto <https://reviews.apache.org/r/30037/#comment114216> Why introduce this in the previous patch if you're just going to move/change it in this one? We should either do the protobuf changes in backwards-compatible iterations, or all in one patch, once we know the desired format. include/mesos/fetcher/fetcher.proto <https://reviews.apache.org/r/30037/#comment114211> As mentioned in the previous patch, this must be optional if you're updating FetcherInfo compatibly, and it still makes sense optional even in a new message for the reason you mention here. include/mesos/fetcher/fetcher.proto <https://reviews.apache.org/r/30037/#comment114213> New fields must take new/unused field ids, so this should be `repeated Item items = 7;` include/mesos/fetcher/fetcher.proto <https://reviews.apache.org/r/30037/#comment114214> Where did this go? - Adam B On Jan. 22, 2015, 1:07 a.m., Bernd Mathiske wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30037/ > ----------------------------------------------------------- > > (Updated Jan. 22, 2015, 1:07 a.m.) > > > Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and > Timothy Chen. > > > Bugs: MESOS-2069 > https://issues.apache.org/jira/browse/MESOS-2069 > > > Repository: mesos-git > > > Description > ------- > > Extends the fetcher info protobuf with "actions" (fetch directly, through > cache, retrieve from cache). Switches the basis for dealing with uris to > "items", which contain the uri, the action, and potentially and cache file > name. Refactors fetch() and run(), so there is only one of each. Introduces > about half of the actual cache logic, including a hashmap of cache file > objects for bookkeeping and basic operations on it. > > > Diffs > ----- > > include/mesos/fetcher/fetcher.proto > facb87b92bf3194516f636dcc348e136af537721 > src/launcher/fetcher.cpp fed0105946da579a38357a30e7ae56e646e05b89 > src/slave/containerizer/fetcher.hpp > 1db0eaf002c8d0eaf4e0391858e61e0912b35829 > src/slave/containerizer/fetcher.cpp > d290f95251def3952c5ee34f600e1d71467f6293 > > Diff: https://reviews.apache.org/r/30037/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Bernd Mathiske > >
