Re: Review Request 38137: Added Docker provisioner and local store

2015-09-16 Thread Gilbert Song
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/#review99305 --- Read through all the codes. A discussion is needed to determine

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-14 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/#review98965 --- src/slave/containerizer/provisioners/docker/paths.cpp (line 35)

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-12 Thread Jiang Yan Xu
> On Sept. 10, 2015, 3:04 a.m., Jiang Yan Xu wrote: > > src/slave/containerizer/provisioners/docker.cpp, lines 104-107 > > > > > > I guess you are modeling this after the `docker pull` syntax here: > >

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-12 Thread Jiang Yan Xu
> On Sept. 10, 2015, 3:04 a.m., Jiang Yan Xu wrote: > > src/slave/containerizer/provisioners/docker/local_store.cpp, lines 158-160 > > > > > > Can we actually use fetcher here? > > Timothy Chen wrote: > Not

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-11 Thread Timothy Chen
> On Sept. 10, 2015, 12:21 a.m., Jie Yu wrote: > > src/slave/containerizer/provisioners/docker/local_store.cpp, line 19 > > > > > > Please fix the include order please. At one point I think our style guide has

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-11 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/#review98622 --- Bad patch! Reviews applied: [38137] Failed command: make -j3

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-11 Thread Timothy Chen
> On Sept. 10, 2015, 10:04 a.m., Jiang Yan Xu wrote: > > src/slave/containerizer/provisioners/docker.cpp, lines 104-107 > > > > > > I guess you are modeling this after the `docker pull` syntax here: > >

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-11 Thread Timothy Chen
> On Sept. 10, 2015, 10:04 a.m., Jiang Yan Xu wrote: > > src/slave/containerizer/provisioners/docker.hpp, line 59 > > > > > > re Jie's comment, Appc uses a spec.hpp|cpp to break the circular > > dependency and

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-11 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/ --- (Updated Sept. 11, 2015, 7:22 a.m.) Review request for mesos, Jie Yu, Jojy

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-10 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/#review98216 --- Partial review, mostly high level.

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-10 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/ --- (Updated Sept. 10, 2015, 7:31 a.m.) Review request for mesos, Jie Yu, Jojy

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-10 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/#review98391 --- Patch looks great! Reviews applied: [38137] All tests passed. -

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-10 Thread Timothy Chen
> On Sept. 10, 2015, 12:21 a.m., Jie Yu wrote: > > src/slave/containerizer/provisioners/docker.hpp, line 112 > > > > > > Can you just call it Image given it's under docker namespace. I think I'll still name it

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-09 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/#review98247 --- First pass. Haven't looked at LocalStore and MetadataManager in

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-09 Thread Timothy Chen
> On Sept. 9, 2015, 4:57 p.m., Jiang Yan Xu wrote: > > Could you take out the paths.hpp|cpp changes that have already been > > committed? done - Timothy --- This is an automatically generated e-mail. To reply, visit:

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-09 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/ --- (Updated Sept. 9, 2015, 5:03 p.m.) Review request for mesos, Jojy Varghese,

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-09 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/#review98213 --- Could you take out the paths.hpp|cpp changes that have already been

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-09 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/#review98265 --- Bad patch! Reviews applied: [38137] Failed command:

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-08 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/#review98030 --- src/slave/containerizer/provisioner.hpp (line 54)

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-08 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/ --- (Updated Sept. 8, 2015, 7:52 p.m.) Review request for mesos, Jojy Varghese,

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-08 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/#review98078 --- Patch looks great! Reviews applied: [38137] All tests passed. -

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-08 Thread Timothy Chen
> On Sept. 8, 2015, 6:35 p.m., Jojy Varghese wrote: > > src/slave/containerizer/provisioner.hpp, line 54 > > > > > > doxygen style? I'll fix this later with another patch, this isn't related to docker. > On Sept.

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-07 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/#review97944 --- Bad patch! Reviews applied: [38141, 38137] Failed command:

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-07 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/ --- (Updated Sept. 7, 2015, 11:31 p.m.) Review request for mesos, Jojy Varghese,

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-07 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/ --- (Updated Sept. 7, 2015, 9:58 p.m.) Review request for mesos, Jojy Varghese,

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-07 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/ --- (Updated Sept. 7, 2015, 9:32 p.m.) Review request for mesos, Jojy Varghese,