Re: Review Request 24464: Redirect docker logs

2014-08-13 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/#review50491 --- Patch looks great! Reviews applied: [24656, 24464] All tests passe

Re: Review Request 24464: Redirect docker logs

2014-08-13 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/ --- (Updated Aug. 13, 2014, 5:33 p.m.) Review request for mesos, Benjamin Hindman a

Re: Review Request 24464: Redirect docker logs

2014-08-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/#review50426 --- Just realized. What's the recover story? 1) what if the slave exits,

Re: Review Request 24464: Redirect docker logs

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

Re: Review Request 24464: Redirect docker logs

2014-08-12 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/ --- (Updated Aug. 12, 2014, 10:28 p.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 24464: Redirect docker logs

2014-08-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/#review50333 --- Ship it! Looks good! Thanks for the cleanup! src/docker/docker.hp

Re: Review Request 24464: Redirect docker logs

2014-08-12 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/ --- (Updated Aug. 12, 2014, 7:44 a.m.) Review request for mesos, Benjamin Hindman a

Re: Review Request 24464: Redirect docker logs

2014-08-11 Thread Timothy Chen
> On Aug. 11, 2014, 10:46 p.m., Jie Yu wrote: > > src/slave/containerizer/docker.cpp, line 446 > > > > > > You probably want to do a 'logs.wait()' here: > > > > logs.wait() > > .onAny(defer(self(), &Sel

Re: Review Request 24464: Redirect docker logs

2014-08-11 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/#review50206 --- Synced with BenH on this patch. Here is our comments. Let us know if

Re: Review Request 24464: Redirect docker logs

2014-08-11 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/ --- (Updated Aug. 11, 2014, 8:19 a.m.) Review request for mesos, Benjamin Hindman a

Re: Review Request 24464: Redirect docker logs

2014-08-10 Thread Timothy Chen
> On Aug. 11, 2014, 3:53 a.m., Jie Yu wrote: > > src/docker/docker.hpp, lines 96-99 > > > > > > Hum, this interface is weird to me. What if in the future we switch to > > use the REST api, what does the return value me

Re: Review Request 24464: Redirect docker logs

2014-08-10 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/#review50134 --- src/docker/docker.hpp

Re: Review Request 24464: Redirect docker logs

2014-08-09 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/ --- (Updated Aug. 10, 2014, 6:51 a.m.) Review request for mesos, Benjamin Hindman a

Re: Review Request 24464: Redirect docker logs

2014-08-09 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/ --- (Updated Aug. 10, 2014, 6:08 a.m.) Review request for mesos, Benjamin Hindman a

Re: Review Request 24464: Redirect docker logs

2014-08-09 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/ --- (Updated Aug. 10, 2014, 5:50 a.m.) Review request for mesos, Benjamin Hindman a

Re: Review Request 24464: Redirect docker logs

2014-08-08 Thread Timothy Chen
> On Aug. 8, 2014, 11:22 p.m., Jie Yu wrote: > > Can you add a test to test the docker.logs. For example, run: > > > > /bin/sh -c 'echo out; echo err 1>&2' > > > > using docker and make sure that you get 'out' from stdout and 'err' from > > stderr:) ya I was exactly thinking about that! - T

Re: Review Request 24464: Redirect docker logs

2014-08-08 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/#review50090 --- Can you add a test to test the docker.logs. For example, run: /bin/

Re: Review Request 24464: Redirect docker logs

2014-08-08 Thread Jie Yu
> On Aug. 8, 2014, 11:13 p.m., Jie Yu wrote: > > > > Timothy Chen wrote: > Ah sorry the docker tests passed, but I don't have tests in place for > testing the logs. I'll be adding some tests later. Thanks Tim! > On Aug. 8, 2014, 11:13 p.m., Jie Yu wrote: > > src/slave/containerizer/docke

Re: Review Request 24464: Redirect docker logs

2014-08-08 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/ --- (Updated Aug. 8, 2014, 11:19 p.m.) Review request for mesos, Benjamin Hindman a

Re: Review Request 24464: Redirect docker logs

2014-08-08 Thread Timothy Chen
> On Aug. 8, 2014, 11:13 p.m., Jie Yu wrote: > > Ah sorry the docker tests passed, but I don't have tests in place for testing the logs. I'll be adding some tests later. - Timothy --- This is an automatically generated e-mail. To reply

Re: Review Request 24464: Redirect docker logs

2014-08-08 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/#review50085 --- src/slave/containerizer/docker.cpp

Re: Review Request 24464: Redirect docker logs

2014-08-08 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/ --- (Updated Aug. 8, 2014, 9:20 p.m.) Review request for mesos, Benjamin Hindman an

Re: Review Request 24464: Redirect docker logs

2014-08-08 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/ --- (Updated Aug. 8, 2014, 9:14 p.m.) Review request for mesos, Benjamin Hindman an

Re: Review Request 24464: Redirect docker logs

2014-08-08 Thread Jie Yu
Yeah, you are right. Having a continuation to check the status is the right way. On Fri, Aug 8, 2014 at 10:09 AM, Timothy Chen wrote: >This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24464/ > > On August 8th, 2014, 4:41 p.m. UTC, *Jie Yu* wrote: >

Re: Review Request 24464: Redirect docker logs

2014-08-08 Thread Timothy Chen
> On Aug. 8, 2014, 4:41 p.m., Jie Yu wrote: > > src/docker/docker.cpp, line 512 > > > > > > return checkError(cmd, s.get()); I don't think I want to checkError since it already reads stderr, however docker logs pipes

Re: Review Request 24464: Redirect docker logs

2014-08-08 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/#review50043 --- src/slave/containerizer/docker.cpp

Re: Review Request 24464: Redirect docker logs

2014-08-08 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/#review50042 --- src/docker/docker.hpp

Re: Review Request 24464: Redirect docker logs

2014-08-07 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/ --- (Updated Aug. 7, 2014, 9:56 p.m.) Review request for mesos and Benjamin Hindman

Re: Review Request 24464: Redirect docker logs

2014-08-07 Thread Timothy Chen
> On Aug. 7, 2014, 6:41 p.m., Jie Yu wrote: > > src/slave/containerizer/docker.cpp, line 583 > > > > > > Who is gonna stop the log subprocess? Worth commenting on that? It will automatically be stopped when the docker

Re: Review Request 24464: Redirect docker logs

2014-08-07 Thread Timothy Chen
> On Aug. 7, 2014, 6:41 p.m., Jie Yu wrote: > > src/docker/docker.cpp, line 512 > > > > > > What if 'docker logs' fails? For example, the containerName is not > > valid? How are we gonna detect that? Right now I'm as

Re: Review Request 24464: Redirect docker logs

2014-08-07 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/#review49936 --- src/docker/docker.cpp

Review Request 24464: Redirect docker logs

2014-08-07 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/ --- Review request for mesos and Benjamin Hindman. Repository: mesos-git Descript