Re: Review Request 51097: Added a `PortMapper` class.

2016-09-03 Thread Avinash sridharan
> On Sept. 3, 2016, 9:36 p.m., Jie Yu wrote: > > I made a bunch of edit while committing since I probably don't have time to > > do review back and forth. Please do take a look at the final patch. > > > > Please follow up with patches if you don't agree on some of the changes. > > > > Also,

Re: Review Request 51097: Added a `PortMapper` class.

2016-09-03 Thread Avinash sridharan
> On Sept. 3, 2016, 9:36 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/isolators/network/cni/spec.hpp, line 35 > > > > > > This is not in the CNI spec. Why we have it here? Most CNI plugins return an error

Re: Review Request 51097: Added a `PortMapper` class.

2016-09-03 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review147619 --- Fix it, then Ship it! I made a bunch of edit while committing

Re: Review Request 51097: Added a `PortMapper` class.

2016-09-02 Thread Avinash sridharan
> On Sept. 2, 2016, 11:46 p.m., haosdent huang wrote: > > src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp, > > line 36 > > > > > > Should put before `using std::cout`; you mean

Re: Review Request 51097: Added a `PortMapper` class.

2016-09-02 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review147713 --- Patch looks great! Reviews applied: [51095, 51096, 51097]

Re: Review Request 51097: Added a `PortMapper` class.

2016-09-02 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review147711 ---

Re: Review Request 51097: Added a `PortMapper` class.

2016-09-02 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/ --- (Updated Sept. 2, 2016, 10:33 p.m.) Review request for mesos, Jie Yu and Qian

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-29 Thread Qian Zhang
> On Aug. 29, 2016, 9:59 a.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/network/cni/spec.cpp, line 63 > > > > > > I would suggest to switch `msg` and `code`. > > Avinash sridharan wrote: >

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-29 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review147166 --- Patch looks great! Reviews applied: [51095, 51096, 51097]

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-29 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/ --- (Updated Aug. 29, 2016, 4 p.m.) Review request for mesos, Jie Yu and Qian

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-29 Thread Avinash sridharan
> On Aug. 29, 2016, 1:59 a.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp, > > line 49 > > > > > > The indent should be 4 spaces, please see the

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-28 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review147113 ---

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-28 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review147108 --- Patch looks great! Reviews applied: [51095, 51096, 51097]

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-28 Thread Avinash sridharan
> On Aug. 27, 2016, 9:12 a.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp, > > lines 143-144 > > > > > > I do not see the code to check the

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-28 Thread Avinash sridharan
> On Aug. 26, 2016, 9:23 a.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp, > > line 169 > > > > > > A newline before this line. > > Avinash

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-28 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/ --- (Updated Aug. 28, 2016, 7:14 p.m.) Review request for mesos, Jie Yu and Qian

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-27 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review147073 ---

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-27 Thread Qian Zhang
> On Aug. 26, 2016, 5:23 p.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp, > > line 81 > > > > > > I think we need to print `result.get()` on success. > >

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-26 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review147067 --- Patch looks great! Reviews applied: [51095, 51096, 51097]

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-26 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/ --- (Updated Aug. 26, 2016, 11:01 p.m.) Review request for mesos, Jie Yu and Qian

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-26 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/ --- (Updated Aug. 26, 2016, 10:51 p.m.) Review request for mesos, Jie Yu and Qian

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-26 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review147034 --- Patch looks great! Reviews applied: [51095, 51096, 51097]

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-26 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review147019 ---

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-26 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/ --- (Updated Aug. 26, 2016, 8:24 p.m.) Review request for mesos, Jie Yu and Qian

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-26 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review146986 --- Patch looks great! Reviews applied: [51095, 51096, 51097]

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-26 Thread Avinash sridharan
> On Aug. 26, 2016, 9:23 a.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp, > > lines 191-192 > > > > > > I think we also need to make sure the

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-26 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/ --- (Updated Aug. 26, 2016, 2:42 p.m.) Review request for mesos, Jie Yu and Qian

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-26 Thread Avinash sridharan
> On Aug. 26, 2016, 9:23 a.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp, > > line 44 > > > > > > A newline before this line, and a space after

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-26 Thread Avinash sridharan
> On Aug. 26, 2016, 9:23 a.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp, > > lines 39-41 > > > > > > I am not sure why we need two classes here, I

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-26 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review146956 ---

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-26 Thread Avinash sridharan
> On Aug. 26, 2016, 9:23 a.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp, > > line 97 > > > > > > I think in our context, we will always set this

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-26 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review146782 ---

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-25 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/ --- (Updated Aug. 26, 2016, 5:21 a.m.) Review request for mesos, Jie Yu and Qian

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-25 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review146913 --- Bad patch! Reviews applied: [51097, 51096, 51095] Failed

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-25 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/ --- (Updated Aug. 25, 2016, 10:04 p.m.) Review request for mesos, Jie Yu and Qian

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-22 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review146381 --- Patch looks great! Reviews applied: [51095, 51096, 51097]

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-22 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/ --- (Updated Aug. 22, 2016, 3:50 p.m.) Review request for mesos, Jie Yu and Qian

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-22 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review146325 --- Patch looks great! Reviews applied: [51095, 51096, 51097]

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-22 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/ --- (Updated Aug. 22, 2016, 7:22 a.m.) Review request for mesos, Jie Yu and Qian

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review146295 --- Patch looks great! Reviews applied: [51095, 51096, 51097]

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-21 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/ --- (Updated Aug. 22, 2016, 5:35 a.m.) Review request for mesos, Jie Yu and Qian

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-21 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/ --- (Updated Aug. 22, 2016, 3:53 a.m.) Review request for mesos, Jie Yu and Qian

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review146257 --- Patch looks great! Reviews applied: [51095, 51096, 51097]

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-21 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/ --- (Updated Aug. 21, 2016, 6:01 a.m.) Review request for mesos, Jie Yu and Qian

Re: Review Request 51097: Added a `PortMapper` class.

2016-08-15 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review145837 --- Patch looks great! Reviews applied: [51095, 51096, 51097]