----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30037/#review68964 -----------------------------------------------------------
src/slave/containerizer/fetcher.hpp <https://reviews.apache.org/r/30037/#comment113513> We don't usually put @return comments in our code base. Seems to be self explantory what it is going to return too. src/slave/containerizer/fetcher.hpp <https://reviews.apache.org/r/30037/#comment113514> Probably comment on what this serial field is for. src/slave/containerizer/fetcher.cpp <https://reviews.apache.org/r/30037/#comment113519> This to me looks like VLOG(1), not LOG(INFO). src/slave/containerizer/fetcher.cpp <https://reviews.apache.org/r/30037/#comment113527> What is the reason you want to get the extension? I see you're using it at the cache file name, but I don't understand why is that necessary? Also If we pass a relative path (valid with MESOS_FRAMEWORK_HOME), then we no longer have a extension too. src/slave/containerizer/fetcher.cpp <https://reviews.apache.org/r/30037/#comment113515> Why do we need to have a distinct cache file name? And also why introduce a PREFIX for it too? Are we expecting to distinguish files that is cache and isn't? And if we are doing so, I don't see why not uris can also start with c if they are relative paths. src/slave/containerizer/fetcher.cpp <https://reviews.apache.org/r/30037/#comment113518> s/std::string/string/g src/slave/containerizer/fetcher.cpp <https://reviews.apache.org/r/30037/#comment113516> CHECK_SOME src/slave/containerizer/fetcher.cpp <https://reviews.apache.org/r/30037/#comment113528> I don't see us take reference as pointer often in our code base, I recommend chaning this to const Promise<string>& promise = cacheFile.get().get()->promise; if you want to save a copy. src/slave/containerizer/fetcher.cpp <https://reviews.apache.org/r/30037/#comment113517> CHECK_PENDING - Timothy Chen On Jan. 21, 2015, 8:54 a.m., Bernd Mathiske wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30037/ > ----------------------------------------------------------- > > (Updated Jan. 21, 2015, 8:54 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 > 5993670f7899233efa1e6acef4b0c7856e32f748 > > Diff: https://reviews.apache.org/r/30037/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Bernd Mathiske > >
