Re: Review Request 34137: Add support for container image provisioners.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/#review92462 --- Ship it! Ship It! - Jie Yu On July 12, 2015, 4:47 a.m., Ian Down

Re: Review Request 34137: Add support for container image provisioners.

> On July 17, 2015, 1:36 a.m., Jie Yu wrote: > > src/slave/containerizer/mesos/containerizer.cpp, lines 637-643 > > > > > > No test for checkpointing??? I decided to punt this in this review. Will follow up with a t

Re: Review Request 34137: Add support for container image provisioners.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/#review92339 --- src/slave/containerizer/mesos/containerizer.cpp (lines 185 - 186) <

Re: Review Request 34137: Add support for container image provisioners.

> On July 17, 2015, 1:36 a.m., Jie Yu wrote: > > src/slave/containerizer/mesos/containerizer.cpp, line 630 > > > > > > Hum, looks like a bug since, for example, slaveId is a reference and > > will be invalid when th

Re: Review Request 34137: Add support for container image provisioners.

> On July 17, 2015, 1:36 a.m., Jie Yu wrote: > > src/slave/containerizer/provisioner.cpp, line 24 > > > > > > Where is this? This won't compile! You're right it won't, wierd it compiled for me :( let's fix this! -

Re: Review Request 34137: Add support for container image provisioners.

> On July 16, 2015, 6:36 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/containerizer.cpp, line 630 > > > > > > Hum, looks like a bug since, for example, slaveId is a reference and > > will be invalid when th

Re: Review Request 34137: Add support for container image provisioners.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/#review92000 --- Tim, I probably wound wait for Vinod's shipit before committing this

Re: Review Request 34137: Add support for container image provisioners.

> On May 28, 2015, 9:49 p.m., Paul Brett wrote: > > src/slave/containerizer/mesos/containerizer.hpp, line 245 > > > > > > To many underscores - can we come up with a better name? We can refactor later. - Timothy -

Re: Review Request 34137: Add support for container image provisioners.

> On July 14, 2015, 7:41 p.m., Vinod Kone wrote: > > src/slave/containerizer/provisioner.cpp, lines 43-56 > > > > > > Why do you need foreach loop here if you were going to return error > > anyway? > > Timothy Chen

Re: Review Request 34137: Add support for container image provisioners.

> On July 8, 2015, 11:34 p.m., Vinod Kone wrote: > > src/slave/containerizer/mesos/containerizer.cpp, line 418 > > > > > > Is it possible for rootfs to not exist when we are here? If not, there > > should be a CHECK a

Re: Review Request 34137: Add support for container image provisioners.

> On July 14, 2015, 7:41 p.m., Vinod Kone wrote: > > can you fix the "depends on" please? I think the two listed are the correct dependencies for this rb? - Timothy --- This is an automatically generated e-mail. To reply, visit: https:/

Re: Review Request 34137: Add support for container image provisioners.

> On July 14, 2015, 7:41 p.m., Vinod Kone wrote: > > src/slave/containerizer/provisioner.cpp, lines 43-56 > > > > > > Why do you need foreach loop here if you were going to return error > > anyway? We need the forea

Re: Review Request 34137: Add support for container image provisioners.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/#review91662 --- can you fix the "depends on" please? src/slave/containerizer/provi

Re: Review Request 34137: Add support for container image provisioners.

> On July 8, 2015, 11:34 p.m., Vinod Kone wrote: > > can you please set the dependency for this review correctly? it's hard to > > understand which reviews introduced the code that this review depends on. none of the dropped issues have comments. did you forget to hit publish on your comments?

Re: Review Request 34137: Add support for container image provisioners.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/#review91422 --- Ship it! Ship It! - Timothy Chen On July 12, 2015, 4:47 a.m., Ia

Re: Review Request 34137: Add support for container image provisioners.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/ --- (Updated July 11, 2015, 9:47 p.m.) Review request for mesos, Chi Zhang, Paul Br

Re: Review Request 34137: Add support for container image provisioners.

> On June 29, 2015, 12:41 p.m., Jiang Yan Xu wrote: > > src/slave/containerizer/mesos/containerizer.hpp, lines 319-320 > > > > > > Why not store ContainerInfo directly? It's optional so I decided to store the required

Re: Review Request 34137: Add support for container image provisioners.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/ --- (Updated July 10, 2015, 5:05 p.m.) Review request for mesos, Chi Zhang, Paul Br

Re: Review Request 34137: Add support for container image provisioners.

> On June 29, 2015, 7:41 p.m., Jiang Yan Xu wrote: > > src/slave/containerizer/provisioner.cpp, lines 46-47 > > > > > > Seems like this belongs to a later patch. AppcProvisioner is not > > introduced yet. +1 - Vinod

Re: Review Request 34137: Add support for container image provisioners.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/#review91014 --- can you please set the dependency for this review correctly? it's ha

Re: Review Request 34137: Add support for container image provisioners.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/ --- (Updated July 7, 2015, 12:42 p.m.) Review request for mesos, Chi Zhang, Paul Br

Re: Review Request 34137: Add support for container image provisioners.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/#review89975 --- src/slave/containerizer/mesos/containerizer.cpp (line 626)

Re: Review Request 34137: Add support for container image provisioners.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/#review89293 --- src/slave/containerizer/mesos/containerizer.hpp (lines 319 - 320) <

Re: Review Request 34137: Add support for container image provisioners.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/#review89227 --- include/mesos/slave/isolator.hpp (line 71)

Re: Review Request 34137: Add support for container image provisioners.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/ --- (Updated June 22, 2015, 9:44 a.m.) Review request for mesos, Chi Zhang, Paul Br

Re: Review Request 34137: Add support for container image provisioners.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/#review85624 --- src/slave/containerizer/mesos/containerizer.hpp

Re: Review Request 34137: Add support for container image provisioners.

> On May 14, 2015, 12:41 p.m., Timothy Chen wrote: > > src/slave/containerizer/mesos/containerizer.cpp, line 193 > > > > > > Not sure if I follow this logic or if I'm missing something, but the > > provisioners hashma

Re: Review Request 34137: Add support for container image provisioners.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/#review83824 --- src/slave/containerizer/mesos/containerizer.cpp

Review Request 34137: Add support for container image provisioners.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/ --- Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.