You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Paul Brett <pa...@twopensource.com> on 2015/06/16 22:27:10 UTC
Review Request 35536: Replace adhoc JSON conversion functions for
ResourceStatistics with a protocol buffer to JSON converter.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35536/
-----------------------------------------------------------
Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
Bugs: MESOS-2874
https://issues.apache.org/jira/browse/MESOS-2874
Repository: mesos
Description
-------
Replace adhoc JSON conversion functions for ResourceStatistics with a protocol buffer to JSON converter.
Diffs
-----
src/slave/containerizer/isolators/network/port_mapping.hpp 4c719b186b519fad0c3869dbdae8b60c3a2c20cc
src/slave/containerizer/isolators/network/port_mapping.cpp 432b05ce5a99c8239fafc47a6b65d46a0fbac26e
src/tests/port_mapping_tests.cpp f8372df74cd71df37de4a2438069ef0ea8878512
Diff: https://reviews.apache.org/r/35536/diff/
Testing
-------
Thanks,
Paul Brett
Re: Review Request 35536: Replace adhoc JSON conversion functions for
ResourceStatistics with a protocol buffer to JSON converter.
Posted by Paul Brett <pa...@twopensource.com>.
> On June 16, 2015, 10:33 p.m., Jie Yu wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 713
> > <https://reviews.apache.org/r/35536/diff/1/?file=985924#file985924line713>
> >
> > I don't think you need to set this.
It is a required field. Without it, the conversion to JSON does not generate a timestamp and hence the conversion back fails.
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35536/#review88147
-----------------------------------------------------------
On June 16, 2015, 8:27 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35536/
> -----------------------------------------------------------
>
> (Updated June 16, 2015, 8:27 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
>
>
> Bugs: MESOS-2874
> https://issues.apache.org/jira/browse/MESOS-2874
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Replace adhoc JSON conversion functions for ResourceStatistics with a protocol buffer to JSON converter.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolators/network/port_mapping.hpp 4c719b186b519fad0c3869dbdae8b60c3a2c20cc
> src/slave/containerizer/isolators/network/port_mapping.cpp 432b05ce5a99c8239fafc47a6b65d46a0fbac26e
> src/tests/port_mapping_tests.cpp f8372df74cd71df37de4a2438069ef0ea8878512
>
> Diff: https://reviews.apache.org/r/35536/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 35536: Replace adhoc JSON conversion functions for
ResourceStatistics with a protocol buffer to JSON converter.
Posted by Jie Yu <yu...@gmail.com>.
> On June 16, 2015, 10:33 p.m., Jie Yu wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 713
> > <https://reviews.apache.org/r/35536/diff/1/?file=985924#file985924line713>
> >
> > I don't think you need to set this.
>
> Paul Brett wrote:
> It is a required field. Without it, the conversion to JSON does not generate a timestamp and hence the conversion back fails.
Then, we should use a dummy value here and call clear_timestamp() in the host namespace. Otherwise, it will overwrite the final timestamp set in the containerizer. I'll fix this for you.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35536/#review88147
-----------------------------------------------------------
On June 16, 2015, 11:44 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35536/
> -----------------------------------------------------------
>
> (Updated June 16, 2015, 11:44 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
>
>
> Bugs: MESOS-2874
> https://issues.apache.org/jira/browse/MESOS-2874
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Replace adhoc JSON conversion functions for ResourceStatistics with a protocol buffer to JSON converter.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolators/network/port_mapping.hpp 4c719b186b519fad0c3869dbdae8b60c3a2c20cc
> src/slave/containerizer/isolators/network/port_mapping.cpp e55e7b62bc29125458f9c0fb5477057ecc5a90df
> src/tests/port_mapping_tests.cpp f8372df74cd71df37de4a2438069ef0ea8878512
>
> Diff: https://reviews.apache.org/r/35536/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 35536: Replace adhoc JSON conversion functions for
ResourceStatistics with a protocol buffer to JSON converter.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35536/#review88147
-----------------------------------------------------------
src/slave/containerizer/isolators/network/port_mapping.cpp (line 706)
<https://reviews.apache.org/r/35536/#comment140553>
I don't think you need to set this.
- Jie Yu
On June 16, 2015, 8:27 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35536/
> -----------------------------------------------------------
>
> (Updated June 16, 2015, 8:27 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
>
>
> Bugs: MESOS-2874
> https://issues.apache.org/jira/browse/MESOS-2874
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Replace adhoc JSON conversion functions for ResourceStatistics with a protocol buffer to JSON converter.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolators/network/port_mapping.hpp 4c719b186b519fad0c3869dbdae8b60c3a2c20cc
> src/slave/containerizer/isolators/network/port_mapping.cpp 432b05ce5a99c8239fafc47a6b65d46a0fbac26e
> src/tests/port_mapping_tests.cpp f8372df74cd71df37de4a2438069ef0ea8878512
>
> Diff: https://reviews.apache.org/r/35536/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 35536: Replace adhoc JSON conversion functions for
ResourceStatistics with a protocol buffer to JSON converter.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35536/#review88146
-----------------------------------------------------------
src/tests/port_mapping_tests.cpp (line 1587)
<https://reviews.apache.org/r/35536/#comment140552>
Oops! I'll let you find the bug. Please be careful because those errors are hard to cache in review too.
- Jie Yu
On June 16, 2015, 8:27 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35536/
> -----------------------------------------------------------
>
> (Updated June 16, 2015, 8:27 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
>
>
> Bugs: MESOS-2874
> https://issues.apache.org/jira/browse/MESOS-2874
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Replace adhoc JSON conversion functions for ResourceStatistics with a protocol buffer to JSON converter.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolators/network/port_mapping.hpp 4c719b186b519fad0c3869dbdae8b60c3a2c20cc
> src/slave/containerizer/isolators/network/port_mapping.cpp 432b05ce5a99c8239fafc47a6b65d46a0fbac26e
> src/tests/port_mapping_tests.cpp f8372df74cd71df37de4a2438069ef0ea8878512
>
> Diff: https://reviews.apache.org/r/35536/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 35536: Replace adhoc JSON conversion functions for
ResourceStatistics with a protocol buffer to JSON converter.
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35536/
-----------------------------------------------------------
(Updated June 16, 2015, 11:44 p.m.)
Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
Changes
-------
Address review comments.
Bugs: MESOS-2874
https://issues.apache.org/jira/browse/MESOS-2874
Repository: mesos
Description
-------
Replace adhoc JSON conversion functions for ResourceStatistics with a protocol buffer to JSON converter.
Diffs (updated)
-----
src/slave/containerizer/isolators/network/port_mapping.hpp 4c719b186b519fad0c3869dbdae8b60c3a2c20cc
src/slave/containerizer/isolators/network/port_mapping.cpp e55e7b62bc29125458f9c0fb5477057ecc5a90df
src/tests/port_mapping_tests.cpp f8372df74cd71df37de4a2438069ef0ea8878512
Diff: https://reviews.apache.org/r/35536/diff/
Testing
-------
sudo make check
Thanks,
Paul Brett
Re: Review Request 35536: Replace adhoc JSON conversion functions for
ResourceStatistics with a protocol buffer to JSON converter.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35536/#review88145
-----------------------------------------------------------
Ship it!
Great cleanup! Thanks Paul!
src/slave/containerizer/isolators/network/port_mapping.cpp (lines 2768 - 2769)
<https://reviews.apache.org/r/35536/#comment140549>
Not yours, but can you fix the format:
```
return Failure(
"Failed to ... "
"network statistics: " + object.error());
```
src/slave/containerizer/isolators/network/port_mapping.cpp (line 2772)
<https://reviews.apache.org/r/35536/#comment140551>
s/isolatorStats/statistics/
src/slave/containerizer/isolators/network/port_mapping.cpp (lines 2775 - 2776)
<https://reviews.apache.org/r/35536/#comment140550>
Ditto.
- Jie Yu
On June 16, 2015, 8:27 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35536/
> -----------------------------------------------------------
>
> (Updated June 16, 2015, 8:27 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
>
>
> Bugs: MESOS-2874
> https://issues.apache.org/jira/browse/MESOS-2874
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Replace adhoc JSON conversion functions for ResourceStatistics with a protocol buffer to JSON converter.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolators/network/port_mapping.hpp 4c719b186b519fad0c3869dbdae8b60c3a2c20cc
> src/slave/containerizer/isolators/network/port_mapping.cpp 432b05ce5a99c8239fafc47a6b65d46a0fbac26e
> src/tests/port_mapping_tests.cpp f8372df74cd71df37de4a2438069ef0ea8878512
>
> Diff: https://reviews.apache.org/r/35536/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 35536: Replace adhoc JSON conversion functions for
ResourceStatistics with a protocol buffer to JSON converter.
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35536/
-----------------------------------------------------------
(Updated June 16, 2015, 8:27 p.m.)
Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
Bugs: MESOS-2874
https://issues.apache.org/jira/browse/MESOS-2874
Repository: mesos
Description
-------
Replace adhoc JSON conversion functions for ResourceStatistics with a protocol buffer to JSON converter.
Diffs
-----
src/slave/containerizer/isolators/network/port_mapping.hpp 4c719b186b519fad0c3869dbdae8b60c3a2c20cc
src/slave/containerizer/isolators/network/port_mapping.cpp 432b05ce5a99c8239fafc47a6b65d46a0fbac26e
src/tests/port_mapping_tests.cpp f8372df74cd71df37de4a2438069ef0ea8878512
Diff: https://reviews.apache.org/r/35536/diff/
Testing (updated)
-------
sudo make check
Thanks,
Paul Brett