Re: Review Request 24776: Add destroy tests for docker containerizer.

2014-11-07 Thread Benjamin Hindman

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/#review60470
---

Ship it!



src/slave/containerizer/docker.hpp
https://reviews.apache.org/r/24776/#comment101871

How come you don't need to do 'process::Owned' here? There is probably some 
bad include file that is including all of the 'process' namespace, but we 
should really be doing process::Owned in headers (just like you do 
process::Shared above).



src/slave/containerizer/docker.hpp
https://reviews.apache.org/r/24776/#comment101872

process::Owned



src/slave/containerizer/docker.hpp
https://reviews.apache.org/r/24776/#comment101873

process::PID (here and everywhere else please).



src/slave/containerizer/docker.hpp
https://reviews.apache.org/r/24776/#comment101874

For the code that you have actually changed please go ahead and s/ // 
(but just for the code you've changed, not for all the code in all the files 
you've changed).



src/slave/containerizer/docker.cpp
https://reviews.apache.org/r/24776/#comment101875

Please update all of the 'dispatch' calls that need to be wrapped to have 
consistent wrapping style in this file. Thanks!



src/tests/docker_containerizer_tests.cpp
https://reviews.apache.org/r/24776/#comment101876

Esoteric style nit: we've only used 1 newline between things 
declared/defined inside of classes, but two newlines for things declared at the 
top level. So please kill the double newline here and throughout the review. 
Thank you Tim!



src/tests/docker_containerizer_tests.cpp
https://reviews.apache.org/r/24776/#comment101877

Did you want a  here? And no need to use the _ prefix here.


- Benjamin Hindman


On Nov. 5, 2014, 7:26 a.m., Timothy Chen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24776/
 ---
 
 (Updated Nov. 5, 2014, 7:26 a.m.)
 
 
 Review request for mesos, Benjamin Hindman and Jie Yu.
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Review: https://reviews.apache.org/r/24776
 
 
 Diffs
 -
 
   src/slave/containerizer/docker.hpp ec6b9cd308e9a16e05f016e8aeadbe77646d1621 
   src/slave/containerizer/docker.cpp a6689203adbdcb0ad12583389eaeb83329e4ef6b 
   src/tests/docker_containerizer_tests.cpp 
 9d4ccc57f58d61c62aab5cdc79a129e987920bf6 
 
 Diff: https://reviews.apache.org/r/24776/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Timothy Chen
 




Re: Review Request 24776: Add destroy tests for docker containerizer.

2014-11-05 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/#review59939
---


Patch looks great!

Reviews applied: [24776]

All tests passed.

- Mesos ReviewBot


On Nov. 5, 2014, 7:26 a.m., Timothy Chen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24776/
 ---
 
 (Updated Nov. 5, 2014, 7:26 a.m.)
 
 
 Review request for mesos, Benjamin Hindman and Jie Yu.
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Review: https://reviews.apache.org/r/24776
 
 
 Diffs
 -
 
   src/slave/containerizer/docker.hpp ec6b9cd308e9a16e05f016e8aeadbe77646d1621 
   src/slave/containerizer/docker.cpp a6689203adbdcb0ad12583389eaeb83329e4ef6b 
   src/tests/docker_containerizer_tests.cpp 
 9d4ccc57f58d61c62aab5cdc79a129e987920bf6 
 
 Diff: https://reviews.apache.org/r/24776/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Timothy Chen
 




Re: Review Request 24776: Add destroy tests for docker containerizer.

2014-11-04 Thread Timothy Chen

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/
---

(Updated Nov. 5, 2014, 7:26 a.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

Rebased and also update the tests to call destroy directly.


Summary (updated)
-

Add destroy tests for docker containerizer.


Repository: mesos-git


Description
---

Review: https://reviews.apache.org/r/24776


Diffs (updated)
-

  src/slave/containerizer/docker.hpp ec6b9cd308e9a16e05f016e8aeadbe77646d1621 
  src/slave/containerizer/docker.cpp a6689203adbdcb0ad12583389eaeb83329e4ef6b 
  src/tests/docker_containerizer_tests.cpp 
9d4ccc57f58d61c62aab5cdc79a129e987920bf6 

Diff: https://reviews.apache.org/r/24776/diff/


Testing
---

make check


Thanks,

Timothy Chen