> On Aug. 11, 2014, 3:53 a.m., Jie Yu wrote: > > src/docker/docker.hpp, lines 96-99 > > <https://reviews.apache.org/r/24464/diff/9/?file=657101#file657101line96> > > > > Hum, this interface is weird to me. What if in the future we switch to > > use the REST api, what does the return value mean? > > > > Ideally, docker.logs should return a 'stream' object, something like > > this: > > > > class Docker { > > class LogStream { > > int out(); // return the out pipe. > > int err(); // return the err pipe. > > void close(); // terminate the subprocess. > > ... > > }; > > > > Future<LogStream> logs(const string& containerName); > > }; > > > > docker.logs(containerName).then(lambda::bind(&_logs, lambda::_1); > > > > void _logs(const Docker::LogStream& stream) > > { > > io::redirect(stream.out(), "stdout"); > > io::redirect(stream.err(), "stderr"); > > }
I was debating about this too, And was thinking to hide it all completely by holding state to docker abstraction but it was a bit too complicated than I like to so end up doing this. I think wrapping the log stream object sounds like a good middle road. I'll do this tonight > On Aug. 11, 2014, 3:53 a.m., Jie Yu wrote: > > src/docker/docker.cpp, lines 84-87 > > <https://reviews.apache.org/r/24464/diff/9/?file=657102#file657102line84> > > > > Looks like these changes are not necessary given your recent changes? Sounds good > On Aug. 11, 2014, 3:53 a.m., Jie Yu wrote: > > src/tests/docker_containerizer_tests.cpp, lines 888-889 > > <https://reviews.apache.org/r/24464/diff/9/?file=657104#file657104line888> > > > > Can you expand here: what extra log will be there if you run the above > > command: > > > > /bin/sh -c 'echo out!; echo err! 1>&2' I'm redirecting the command executor log output to stdout and stderr, so it also has glog init messages. - Timothy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24464/#review50134 ----------------------------------------------------------- On Aug. 10, 2014, 6:51 a.m., Timothy Chen wrote: > > ----------------------------------------------------------- > 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 and Jie Yu. > > > Repository: mesos-git > > > Description > ------- > > Review: https://reviews.apache.org/r/24464 > > > Diffs > ----- > > src/docker/docker.hpp 98b2d6099988f51f12e7b108e73dcfd0143adc48 > src/docker/docker.cpp 1cba381118c6bd2ac7fcf5a8a229602e2c65c571 > src/slave/containerizer/docker.cpp 904cdd32362591777aecaa58e723af36419f011c > src/tests/docker_containerizer_tests.cpp > a559836dd11a9a97e5939364c4b35a8dbb6a503d > > Diff: https://reviews.apache.org/r/24464/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Timothy Chen > >
