Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-12 Thread Timothy Chen
On May 11, 2015, 11:07 p.m., Timothy Chen wrote: Thanks Jay the changes looks reasonable to me, will wait for Jie to comment. Jay Buffington wrote: Great, thanks! When you commit can you add the three separate commits that are here:

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/#review83265 --- src/slave/containerizer/docker.cpp

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Jay Buffington
On May 11, 2015, 12:12 p.m., Timothy Chen wrote: src/tests/slave_tests.cpp, line 325 https://reviews.apache.org/r/33249/diff/7/?file=955468#file955468line325 I don't really understand this comment? And if you simply pause the clock your await won't work right? I'm starting to

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Jay Buffington
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/ --- (Updated May 11, 2015, 3:06 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Jay Buffington
On May 11, 2015, 4:07 p.m., Timothy Chen wrote: Thanks Jay the changes looks reasonable to me, will wait for Jie to comment. Great, thanks! When you commit can you add the three separate commits that are here:

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/#review83310 --- Thanks Jay the changes looks reasonable to me, will wait for Jie to

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Jay Buffington
On May 8, 2015, 11:48 p.m., Timothy Chen wrote: src/slave/containerizer/mesos/containerizer.cpp, line 567 https://reviews.apache.org/r/33249/diff/6/?file=954330#file954330line567 No tests for mesos containerizer change? The changes to mesos containerizer were to support the

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/#review83233 --- include/mesos/containerizer/containerizer.proto

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Jay Buffington
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/ --- (Updated May 11, 2015, 10:10 a.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-07 Thread Jay Buffington
On May 7, 2015, 10:02 a.m., Timothy Chen wrote: src/examples/docker_no_executor_framework.cpp, line 109 https://reviews.apache.org/r/33249/diff/4/?file=952333#file952333line109 is this something you like to commit? Whops! That snuck in. I'll fix. On May 7, 2015, 10:02 a.m.,

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-07 Thread Jay Buffington
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/ --- (Updated May 7, 2015, 11:57 a.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-07 Thread Jay Buffington
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/ --- (Updated May 7, 2015, 9:17 a.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

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

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-07 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/#review82832 --- Do we have any tests to check for the change in mesos containerizer

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-06 Thread Timothy Chen
On April 21, 2015, 11:25 p.m., Jie Yu wrote: src/slave/slave.cpp, lines 3065-3078 https://reviews.apache.org/r/33249/diff/3/?file=938221#file938221line3065 Instead of doing that in your way, can we just try to make sure `containerizer-wait` here will return a failure (or a

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-05 Thread Timothy Chen
On April 21, 2015, 11:25 p.m., Jie Yu wrote: src/slave/slave.cpp, lines 3065-3078 https://reviews.apache.org/r/33249/diff/3/?file=938221#file938221line3065 Instead of doing that in your way, can we just try to make sure `containerizer-wait` here will return a failure (or a

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-05 Thread Jie Yu
On April 21, 2015, 11:25 p.m., Jie Yu wrote: src/slave/slave.cpp, lines 3065-3078 https://reviews.apache.org/r/33249/diff/3/?file=938221#file938221line3065 Instead of doing that in your way, can we just try to make sure `containerizer-wait` here will return a failure (or a

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-05 Thread Jay Buffington
On April 21, 2015, 4:25 p.m., Jie Yu wrote: src/slave/slave.cpp, lines 3065-3078 https://reviews.apache.org/r/33249/diff/3/?file=938221#file938221line3065 Instead of doing that in your way, can we just try to make sure `containerizer-wait` here will return a failure (or a

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-04 Thread Jay Buffington
On April 21, 2015, 4:25 p.m., Jie Yu wrote: src/slave/slave.cpp, lines 3065-3078 https://reviews.apache.org/r/33249/diff/3/?file=938221#file938221line3065 Instead of doing that in your way, can we just try to make sure `containerizer-wait` here will return a failure (or a

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-01 Thread Jie Yu
On April 21, 2015, 11:25 p.m., Jie Yu wrote: src/slave/slave.cpp, lines 3065-3078 https://reviews.apache.org/r/33249/diff/3/?file=938221#file938221line3065 Instead of doing that in your way, can we just try to make sure `containerizer-wait` here will return a failure (or a

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-04-30 Thread Jay Buffington
On April 21, 2015, 4:25 p.m., Jie Yu wrote: src/slave/slave.cpp, lines 3065-3078 https://reviews.apache.org/r/33249/diff/3/?file=938221#file938221line3065 Instead of doing that in your way, can we just try to make sure `containerizer-wait` here will return a failure (or a

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-04-22 Thread Jay Buffington
On April 21, 2015, 4:25 p.m., Jie Yu wrote: src/slave/slave.cpp, lines 3065-3078 https://reviews.apache.org/r/33249/diff/3/?file=938221#file938221line3065 Instead of doing that in your way, can we just try to make sure `containerizer-wait` here will return a failure (or a

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-04-22 Thread Jie Yu
On April 21, 2015, 11:25 p.m., Jie Yu wrote: src/slave/slave.cpp, lines 3065-3078 https://reviews.apache.org/r/33249/diff/3/?file=938221#file938221line3065 Instead of doing that in your way, can we just try to make sure `containerizer-wait` here will return a failure (or a