Re: Review Request 32664: Add port mapping isolator statistics tests

2015-05-27 Thread Paul Brett

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

(Updated May 27, 2015, 9:04 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.


Bugs: mesos-2332
https://issues.apache.org/jira/browse/mesos-2332


Repository: mesos


Description
---

Add port mapping isolator statistics tests


Diffs
-

  src/tests/port_mapping_tests.cpp b8c2db6d0a02f79d38a21c227575299880980502 

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


Testing
---

make check


Thanks,

Paul Brett



Re: Review Request 32664: Add port mapping isolator statistics tests

2015-05-27 Thread Mesos ReviewBot

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


Bad patch!

Reviews applied: [32664]

Failed command: ./support/apply-review.sh -n -r 32664

Error:
 2015-05-28 05:19:18 URL:https://reviews.apache.org/r/32664/diff/raw/ 
[42205/42205] - 32664.patch [1]
error: patch failed: src/tests/port_mapping_tests.cpp:2015
error: src/tests/port_mapping_tests.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On May 27, 2015, 9:04 p.m., Paul Brett wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/32664/
 ---
 
 (Updated May 27, 2015, 9:04 p.m.)
 
 
 Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
 
 
 Bugs: mesos-2332
 https://issues.apache.org/jira/browse/mesos-2332
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add port mapping isolator statistics tests
 
 
 Diffs
 -
 
   src/tests/port_mapping_tests.cpp b8c2db6d0a02f79d38a21c227575299880980502 
 
 Diff: https://reviews.apache.org/r/32664/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Paul Brett
 




Re: Review Request 32664: Add port mapping isolator statistics tests

2015-05-22 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [34321, 34426, 34428, 34431, 34432, 32660, 34436, 34558, 32664]

All tests passed.

- Mesos ReviewBot


On May 22, 2015, 11:45 p.m., Paul Brett wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/32664/
 ---
 
 (Updated May 22, 2015, 11:45 p.m.)
 
 
 Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
 
 
 Bugs: mesos-2332
 https://issues.apache.org/jira/browse/mesos-2332
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add port mapping isolator statistics tests
 
 
 Diffs
 -
 
   src/tests/port_mapping_tests.cpp b8c2db6d0a02f79d38a21c227575299880980502 
 
 Diff: https://reviews.apache.org/r/32664/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Paul Brett
 




Re: Review Request 32664: Add port mapping isolator statistics tests

2015-05-22 Thread Paul Brett

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

(Updated May 22, 2015, 11:45 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.


Changes
---

Update to address review comments.


Bugs: mesos-2332
https://issues.apache.org/jira/browse/mesos-2332


Repository: mesos


Description
---

Add port mapping isolator statistics tests


Diffs (updated)
-

  src/tests/port_mapping_tests.cpp b8c2db6d0a02f79d38a21c227575299880980502 

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


Testing
---

make check


Thanks,

Paul Brett



Re: Review Request 32664: Add port mapping isolator statistics tests

2015-05-21 Thread Paul Brett


 On April 8, 2015, 7:46 p.m., Ian Downes wrote:
  src/tests/port_mapping_tests.cpp, line 1788
  https://reviews.apache.org/r/32664/diff/1/?file=911866#file911866line1788
 
  Did you consider using iperf3 which makes all features available 
  through a library?

I looked at iperf3 but it would require creating an external helper that can be 
launched on host or in a container, then generating and parsing the output. I 
don't believe the benefit is worth the extra effort required.


 On April 8, 2015, 7:46 p.m., Ian Downes wrote:
  src/tests/port_mapping_tests.cpp, line 1831
  https://reviews.apache.org/r/32664/diff/1/?file=911866#file911866line1831
 
  that's a scary regex! I'm guessing a lot of this would be vastly 
  simpler if we used the iper3 library. Did you investigate that?

Using the iperf3 library requires that we create a helper function that we 
spawn to execute the tests (possible in a container), and therefore we would 
have to output  parse data in some format. However, I can make the regex a lot 
more readable.


 On April 8, 2015, 7:46 p.m., Ian Downes wrote:
  src/tests/port_mapping_tests.cpp, lines 1874-1881
  https://reviews.apache.org/r/32664/diff/1/?file=911866#file911866line1874
 
  Why do this in a shell? why not do this in code - signal the pid as 
  desired.

I've reduced but not eliminated the use of shell script to operations that have 
to sometimes run inside containers.  This allows the use of the same mechanisms 
to stop the helper programs as used to start the program without having to 
create a separate helper program.


- Paul


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


On April 1, 2015, 3:45 p.m., Paul Brett wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/32664/
 ---
 
 (Updated April 1, 2015, 3:45 p.m.)
 
 
 Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
 
 
 Bugs: mesos-2332
 https://issues.apache.org/jira/browse/mesos-2332
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add port mapping isolator statistics tests
 
 
 Diffs
 -
 
   src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 
 
 Diff: https://reviews.apache.org/r/32664/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Paul Brett