----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50959/#review149113 -----------------------------------------------------------
src/Makefile.am (line 854) <https://reviews.apache.org/r/50959/#comment216626> I'd rename this to rkt. The fetcher plugin should be named by their 'method', rather than 'content'. E.g., copy, curl, hadoop, etc. src/uri/fetchers/appc.cpp (lines 99 - 103) <https://reviews.apache.org/r/50959/#comment216628> YOu should do this check in 'create' to fail early. src/uri/fetchers/appc.cpp (line 108) <https://reviews.apache.org/r/50959/#comment216629> `.await` is a blocking call. We cannot use blocking calls as it'll block the worker thread. Please use .then src/uri/fetchers/appc.cpp (line 110) <https://reviews.apache.org/r/50959/#comment216676> All error message should be Capitalized. src/uri/fetchers/appc.cpp (line 116) <https://reviews.apache.org/r/50959/#comment216677> Ditto on await. We cannot use that because it's blocking. We only use 'await' in tests. src/uri/fetchers/appc.cpp (line 118) <https://reviews.apache.org/r/50959/#comment216678> space before and after `+` src/uri/fetchers/appc.cpp (line 122) <https://reviews.apache.org/r/50959/#comment216680> Comments need to be a full sentense. Capitalize please. Please refer to the style guide. src/uri/fetchers/appc.cpp (lines 149 - 151) <https://reviews.apache.org/r/50959/#comment216684> I suggest we create a class called Rkt which abstracts the rkt tool. ``` Rkt rkt = Rkt::create(); rkt->fetch(...); rkt->export(...); ``` You can do the os::which check in Rkt::create. src/uri/fetchers/appc.cpp (line 252) <https://reviews.apache.org/r/50959/#comment216689> Please fix all the space issue. you need space before and after `+` - Jie Yu On Sept. 6, 2016, 8:11 p.m., Srinivas Brahmaroutu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50959/ > ----------------------------------------------------------- > > (Updated Sept. 6, 2016, 8:11 p.m.) > > > Review request for mesos, Gilbert Song and Jie Yu. > > > Bugs: MESOS-4288 > https://issues.apache.org/jira/browse/MESOS-4288 > > > Repository: mesos > > > Description > ------- > > AppcFetcherPlugin returns a '.aci' file of a container image. > > Fetch supports all three modes that 'rkt fetch' supports, fetch with > meta discovery ex: 'coreos.com/hello:latest', fetch from a specific > location ex: 'https://github.com/xxx/hello.aci' or fetch from docker > registry ex: 'docker://hello-world'. > > Added new fetched plugin for Appc fetch algorithm. The new > AppcFetcherPlugin implements two methods. Method 'rktFetch' is used to > fetch a container image into a store specified at a directory location > using 'rkt fetch' command and method 'rktExport' exports the image > fetched into the directory location as a '.aci' file that is in tar > file format, usinf 'rkt image export' command. > > > Diffs > ----- > > src/Makefile.am 15b9a63822eca8d0b428191940026756fba7821e > src/uri/fetcher.hpp 949363ecdb15f5b225db7f3ed67eda52c351bfa1 > src/uri/fetcher.cpp 904fce5a203c57ef1b8fdda09c81efbcf18f5d2c > src/uri/fetchers/appc.hpp PRE-CREATION > src/uri/fetchers/appc.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/50959/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Srinivas Brahmaroutu > >
