Re: Review Request 38335: Add JSON::protobuf for google::protobuf::RepeatedPtrField

2015-11-04 Thread Klaus Ma 

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

(Updated Nov. 4, 2015, 9:21 p.m.)


Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.


Changes
---

rebase the code & address comments


Bugs: MESOS-3405
https://issues.apache.org/jira/browse/MESOS-3405


Repository: mesos


Description
---

Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
converts a `google::protobuf::Message` into a `JSON::Object`.
We should add the support for `google::protobuf::RepeatedPtrField` by 
introducing overloaded functions.


Diffs (updated)
-

  src/common/http.cpp f56d8a1 
  src/docker/executor.cpp d4c05c2 
  src/examples/persistent_volume_framework.cpp 176ac3d 
  src/launcher/executor.cpp 50b3c6e 
  src/master/contender.cpp c641305 
  src/master/http.cpp 9d20346 
  src/master/maintenance.cpp 5fe9358 
  src/master/registrar.cpp 1117232 
  src/slave/containerizer/fetcher.cpp e0d02d5 
  src/slave/containerizer/mesos/containerizer.cpp 9fd69c1 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 1911ba6 
  src/slave/monitor.cpp aa6e958 
  src/tests/containerizer/launch_tests.cpp de655ec 
  src/tests/containerizer/port_mapping_tests.cpp ae2c0e6 
  src/tests/fault_tolerance_tests.cpp f78a291 
  src/tests/master_contender_detector_tests.cpp 1da7f91 
  src/tests/master_maintenance_tests.cpp e89ce3b 
  src/tests/master_tests.cpp 8564405 
  src/tests/mesos.cpp ab2d85b 
  src/tests/monitor_tests.cpp 583e711 
  src/tests/reservation_endpoints_tests.cpp f5f9c48 
  src/tests/resources_tests.cpp 6584fc6 
  src/tests/scheduler_http_api_tests.cpp b6f6e91 
  src/tests/script.cpp bcc1fab 
  src/tests/slave_tests.cpp 91dbdba 
  src/usage/main.cpp 86fd796 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 38335: Add JSON::protobuf for google::protobuf::RepeatedPtrField

2015-11-04 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [38342, 38335]

All tests passed.

- Mesos ReviewBot


On Nov. 4, 2015, 1:21 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38335/
> ---
> 
> (Updated Nov. 4, 2015, 1:21 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp f56d8a1 
>   src/docker/executor.cpp d4c05c2 
>   src/examples/persistent_volume_framework.cpp 176ac3d 
>   src/launcher/executor.cpp 50b3c6e 
>   src/master/contender.cpp c641305 
>   src/master/http.cpp 9d20346 
>   src/master/maintenance.cpp 5fe9358 
>   src/master/registrar.cpp 1117232 
>   src/slave/containerizer/fetcher.cpp e0d02d5 
>   src/slave/containerizer/mesos/containerizer.cpp 9fd69c1 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 1911ba6 
>   src/slave/monitor.cpp aa6e958 
>   src/tests/containerizer/launch_tests.cpp de655ec 
>   src/tests/containerizer/port_mapping_tests.cpp ae2c0e6 
>   src/tests/fault_tolerance_tests.cpp f78a291 
>   src/tests/master_contender_detector_tests.cpp 1da7f91 
>   src/tests/master_maintenance_tests.cpp e89ce3b 
>   src/tests/master_tests.cpp 8564405 
>   src/tests/mesos.cpp ab2d85b 
>   src/tests/monitor_tests.cpp 583e711 
>   src/tests/reservation_endpoints_tests.cpp f5f9c48 
>   src/tests/resources_tests.cpp 6584fc6 
>   src/tests/scheduler_http_api_tests.cpp b6f6e91 
>   src/tests/script.cpp bcc1fab 
>   src/tests/slave_tests.cpp 91dbdba 
>   src/usage/main.cpp 86fd796 
> 
> Diff: https://reviews.apache.org/r/38335/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38335: Add JSON::protobuf for google::protobuf::RepeatedPtrField

2015-11-03 Thread Michael Park 


> On Oct. 22, 2015, 8:53 p.m., Jan Schlicht wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 800
> > 
> >
> > Not yours, but please s/push_back/emplace_back
> 
> Michael Park wrote:
> I'm curious as to what your reasoning is?
> 
> Klaus Ma wrote:
> AFAIK, that'll avoid temp var copy from `JSON::protobuf(...)`'s return to 
> `commandArray.values`

Not true. `push_back(T&&)` will be used which avoids the copy. My 
recommendation is to use `push_back` if you have an instance of `T`, and use 
`emplace_back` when you have __constructor parameters__ for `T`. Note that 
using `emplace_back` with an instance of `T` will invoke the copy/move 
constructor so the affect is the same, but they're not interchangeable all 
cases because they have different initialization rules.


- Michael


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


On Nov. 3, 2015, 1:38 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38335/
> ---
> 
> (Updated Nov. 3, 2015, 1:38 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp f56d8a1 
>   src/docker/executor.cpp d4c05c2 
>   src/examples/persistent_volume_framework.cpp 176ac3d 
>   src/launcher/executor.cpp 50b3c6e 
>   src/master/contender.cpp c641305 
>   src/master/http.cpp 093f793 
>   src/master/maintenance.cpp 5fe9358 
>   src/master/registrar.cpp 1117232 
>   src/slave/containerizer/fetcher.cpp e0d02d5 
>   src/slave/containerizer/mesos/containerizer.cpp 9fd69c1 
>   src/slave/monitor.cpp aa6e958 
>   src/tests/containerizer/launch_tests.cpp de655ec 
>   src/tests/fault_tolerance_tests.cpp f78a291 
>   src/tests/master_maintenance_tests.cpp e89ce3b 
>   src/tests/master_tests.cpp ee24739 
>   src/tests/mesos.cpp ab2d85b 
>   src/tests/monitor_tests.cpp 583e711 
>   src/tests/reservation_endpoints_tests.cpp f5f9c48 
>   src/tests/resources_tests.cpp 6584fc6 
>   src/tests/scheduler_http_api_tests.cpp b6f6e91 
>   src/tests/script.cpp bcc1fab 
>   src/tests/slave_tests.cpp 91dbdba 
>   src/usage/main.cpp 86fd796 
> 
> Diff: https://reviews.apache.org/r/38335/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38335: Add JSON::protobuf for google::protobuf::RepeatedPtrField

2015-11-03 Thread Michael Park 

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

Ship it!


Please rebase and address the comment below and I can get this committed for 
you!


src/tests/reservation_endpoints_tests.cpp (lines 107 - 108)


Could we take the `JSON::protobuf(static_cast<...>(resources))` out of this 
expression? It makes it quite difficult to read I think.

Here and below.


- Michael Park


On Nov. 3, 2015, 1:38 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38335/
> ---
> 
> (Updated Nov. 3, 2015, 1:38 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp f56d8a1 
>   src/docker/executor.cpp d4c05c2 
>   src/examples/persistent_volume_framework.cpp 176ac3d 
>   src/launcher/executor.cpp 50b3c6e 
>   src/master/contender.cpp c641305 
>   src/master/http.cpp 093f793 
>   src/master/maintenance.cpp 5fe9358 
>   src/master/registrar.cpp 1117232 
>   src/slave/containerizer/fetcher.cpp e0d02d5 
>   src/slave/containerizer/mesos/containerizer.cpp 9fd69c1 
>   src/slave/monitor.cpp aa6e958 
>   src/tests/containerizer/launch_tests.cpp de655ec 
>   src/tests/fault_tolerance_tests.cpp f78a291 
>   src/tests/master_maintenance_tests.cpp e89ce3b 
>   src/tests/master_tests.cpp ee24739 
>   src/tests/mesos.cpp ab2d85b 
>   src/tests/monitor_tests.cpp 583e711 
>   src/tests/reservation_endpoints_tests.cpp f5f9c48 
>   src/tests/resources_tests.cpp 6584fc6 
>   src/tests/scheduler_http_api_tests.cpp b6f6e91 
>   src/tests/script.cpp bcc1fab 
>   src/tests/slave_tests.cpp 91dbdba 
>   src/usage/main.cpp 86fd796 
> 
> Diff: https://reviews.apache.org/r/38335/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38335: Add JSON::protobuf for google::protobuf::RepeatedPtrField

2015-11-03 Thread Klaus Ma 


> On Oct. 23, 2015, 4:53 a.m., Jan Schlicht wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 800
> > 
> >
> > Not yours, but please s/push_back/emplace_back
> 
> Michael Park wrote:
> I'm curious as to what your reasoning is?

AFAIK, that'll avoid temp var copy from `JSON::protobuf(...)`'s return to 
`commandArray.values`


- Klaus


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


On Oct. 20, 2015, 1:37 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38335/
> ---
> 
> (Updated Oct. 20, 2015, 1:37 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp f56d8a1 
>   src/docker/executor.cpp 1e49013 
>   src/examples/persistent_volume_framework.cpp 176ac3d 
>   src/launcher/executor.cpp 50b3c6e 
>   src/master/contender.cpp c641305 
>   src/master/http.cpp 093f793 
>   src/master/maintenance.cpp 5fe9358 
>   src/master/registrar.cpp 1117232 
>   src/slave/containerizer/fetcher.cpp e0d02d5 
>   src/slave/containerizer/mesos/containerizer.cpp d1fc5a4 
>   src/slave/monitor.cpp aa6e958 
>   src/tests/containerizer/launch_tests.cpp de655ec 
>   src/tests/fault_tolerance_tests.cpp f78a291 
>   src/tests/master_maintenance_tests.cpp e89ce3b 
>   src/tests/master_tests.cpp ee24739 
>   src/tests/mesos.hpp 3e58b45 
>   src/tests/mesos.cpp ab2d85b 
>   src/tests/monitor_tests.cpp 583e711 
>   src/tests/reservation_endpoints_tests.cpp f5f9c48 
>   src/tests/resources_tests.cpp 6584fc6 
>   src/tests/scheduler_http_api_tests.cpp d338a1b 
>   src/tests/script.cpp bcc1fab 
>   src/tests/slave_tests.cpp 10a4fa7 
>   src/usage/main.cpp 86fd796 
> 
> Diff: https://reviews.apache.org/r/38335/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38335: Add JSON::protobuf for google::protobuf::RepeatedPtrField

2015-11-03 Thread Klaus Ma 

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

(Updated Nov. 3, 2015, 9:38 p.m.)


Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.


Changes
---

Address comments


Bugs: MESOS-3405
https://issues.apache.org/jira/browse/MESOS-3405


Repository: mesos


Description
---

Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
converts a `google::protobuf::Message` into a `JSON::Object`.
We should add the support for `google::protobuf::RepeatedPtrField` by 
introducing overloaded functions.


Diffs (updated)
-

  src/common/http.cpp f56d8a1 
  src/docker/executor.cpp d4c05c2 
  src/examples/persistent_volume_framework.cpp 176ac3d 
  src/launcher/executor.cpp 50b3c6e 
  src/master/contender.cpp c641305 
  src/master/http.cpp 093f793 
  src/master/maintenance.cpp 5fe9358 
  src/master/registrar.cpp 1117232 
  src/slave/containerizer/fetcher.cpp e0d02d5 
  src/slave/containerizer/mesos/containerizer.cpp 9fd69c1 
  src/slave/monitor.cpp aa6e958 
  src/tests/containerizer/launch_tests.cpp de655ec 
  src/tests/fault_tolerance_tests.cpp f78a291 
  src/tests/master_maintenance_tests.cpp e89ce3b 
  src/tests/master_tests.cpp ee24739 
  src/tests/mesos.cpp ab2d85b 
  src/tests/monitor_tests.cpp 583e711 
  src/tests/reservation_endpoints_tests.cpp f5f9c48 
  src/tests/resources_tests.cpp 6584fc6 
  src/tests/scheduler_http_api_tests.cpp b6f6e91 
  src/tests/script.cpp bcc1fab 
  src/tests/slave_tests.cpp 91dbdba 
  src/usage/main.cpp 86fd796 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 38335: Add JSON::protobuf for google::protobuf::RepeatedPtrField

2015-11-02 Thread Michael Park 


> On Oct. 22, 2015, 8:53 p.m., Jan Schlicht wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 800
> > 
> >
> > Not yours, but please s/push_back/emplace_back

I'm curious as to what your reasoning is?


- Michael


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


On Oct. 20, 2015, 5:37 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38335/
> ---
> 
> (Updated Oct. 20, 2015, 5:37 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp f56d8a1 
>   src/docker/executor.cpp 1e49013 
>   src/examples/persistent_volume_framework.cpp 176ac3d 
>   src/launcher/executor.cpp 50b3c6e 
>   src/master/contender.cpp c641305 
>   src/master/http.cpp 093f793 
>   src/master/maintenance.cpp 5fe9358 
>   src/master/registrar.cpp 1117232 
>   src/slave/containerizer/fetcher.cpp e0d02d5 
>   src/slave/containerizer/mesos/containerizer.cpp d1fc5a4 
>   src/slave/monitor.cpp aa6e958 
>   src/tests/containerizer/launch_tests.cpp de655ec 
>   src/tests/fault_tolerance_tests.cpp f78a291 
>   src/tests/master_maintenance_tests.cpp e89ce3b 
>   src/tests/master_tests.cpp ee24739 
>   src/tests/mesos.hpp 3e58b45 
>   src/tests/mesos.cpp ab2d85b 
>   src/tests/monitor_tests.cpp 583e711 
>   src/tests/reservation_endpoints_tests.cpp f5f9c48 
>   src/tests/resources_tests.cpp 6584fc6 
>   src/tests/scheduler_http_api_tests.cpp d338a1b 
>   src/tests/script.cpp bcc1fab 
>   src/tests/slave_tests.cpp 10a4fa7 
>   src/usage/main.cpp 86fd796 
> 
> Diff: https://reviews.apache.org/r/38335/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38335: Add JSON::protobuf for google::protobuf::RepeatedPtrField

2015-11-02 Thread Michael Park 


> On Sept. 29, 2015, 3:48 p.m., Alexander Rukletsov wrote:
> > src/tests/mesos.hpp, line 1844
> > 
> >
> > We tend not to use `typedef`s in the codebase. However, this looks like 
> > a good idea, but I would place it in `Resources`, e.g. 
> > `Resources::ProtoType`.
> > 
> > However, I'll leave the decision whether to use `typedef` or not to you 
> > and your shepherd.
> 
> Klaus Ma wrote:
> It's related to test only, so keep it in test module for now.

This doesn't seem like an intuitive typedef nor is it all that helpful. I think 
it would be better to use a `using` declaration instead.

`using google::protobuf::RepeatedPtrField;` then we can use 
`RepeatedPtrField`. I would also suggest breaking it out of the 
single expression to reduce the clutterness. For example:

```cpp
string body = "resources=" +
  stringify(JSON::protobuf(
  static_cast(dynamicallyReserved)));
```
could be:
```cpp
auto protobuf = JSON::protobuf(
static_cast&>(dynamicallyReserved));

string body = "resources=" + stringify(protobuf);
```

Sidenote: the `static_cast` to `const &` is to avoid an unnecessary copy.


- Michael


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


On Oct. 20, 2015, 5:37 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38335/
> ---
> 
> (Updated Oct. 20, 2015, 5:37 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp f56d8a1 
>   src/docker/executor.cpp 1e49013 
>   src/examples/persistent_volume_framework.cpp 176ac3d 
>   src/launcher/executor.cpp 50b3c6e 
>   src/master/contender.cpp c641305 
>   src/master/http.cpp 093f793 
>   src/master/maintenance.cpp 5fe9358 
>   src/master/registrar.cpp 1117232 
>   src/slave/containerizer/fetcher.cpp e0d02d5 
>   src/slave/containerizer/mesos/containerizer.cpp d1fc5a4 
>   src/slave/monitor.cpp aa6e958 
>   src/tests/containerizer/launch_tests.cpp de655ec 
>   src/tests/fault_tolerance_tests.cpp f78a291 
>   src/tests/master_maintenance_tests.cpp e89ce3b 
>   src/tests/master_tests.cpp ee24739 
>   src/tests/mesos.hpp 3e58b45 
>   src/tests/mesos.cpp ab2d85b 
>   src/tests/monitor_tests.cpp 583e711 
>   src/tests/reservation_endpoints_tests.cpp f5f9c48 
>   src/tests/resources_tests.cpp 6584fc6 
>   src/tests/scheduler_http_api_tests.cpp d338a1b 
>   src/tests/script.cpp bcc1fab 
>   src/tests/slave_tests.cpp 10a4fa7 
>   src/usage/main.cpp 86fd796 
> 
> Diff: https://reviews.apache.org/r/38335/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38335: Add JSON::protobuf for google::protobuf::RepeatedPtrField

2015-10-22 Thread Jan Schlicht 

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

Ship it!



src/slave/containerizer/mesos/containerizer.cpp (line 800)


Not yours, but please s/push_back/emplace_back



src/tests/reservation_endpoints_tests.cpp (line 830)


This is longer than 80 chars.


- Jan Schlicht


On Oct. 20, 2015, 7:37 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38335/
> ---
> 
> (Updated Oct. 20, 2015, 7:37 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp f56d8a1 
>   src/docker/executor.cpp 1e49013 
>   src/examples/persistent_volume_framework.cpp 176ac3d 
>   src/launcher/executor.cpp 50b3c6e 
>   src/master/contender.cpp c641305 
>   src/master/http.cpp 093f793 
>   src/master/maintenance.cpp 5fe9358 
>   src/master/registrar.cpp 1117232 
>   src/slave/containerizer/fetcher.cpp e0d02d5 
>   src/slave/containerizer/mesos/containerizer.cpp d1fc5a4 
>   src/slave/monitor.cpp aa6e958 
>   src/tests/containerizer/launch_tests.cpp de655ec 
>   src/tests/fault_tolerance_tests.cpp f78a291 
>   src/tests/master_maintenance_tests.cpp e89ce3b 
>   src/tests/master_tests.cpp ee24739 
>   src/tests/mesos.hpp 3e58b45 
>   src/tests/mesos.cpp ab2d85b 
>   src/tests/monitor_tests.cpp 583e711 
>   src/tests/reservation_endpoints_tests.cpp f5f9c48 
>   src/tests/resources_tests.cpp 6584fc6 
>   src/tests/scheduler_http_api_tests.cpp d338a1b 
>   src/tests/script.cpp bcc1fab 
>   src/tests/slave_tests.cpp 10a4fa7 
>   src/usage/main.cpp 86fd796 
> 
> Diff: https://reviews.apache.org/r/38335/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38335: Add JSON::protobuf for google::protobuf::RepeatedPtrField

2015-10-19 Thread Klaus Ma 

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

(Updated Oct. 20, 2015, 5:37 a.m.)


Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.


Changes
---

Address comments


Bugs: MESOS-3405
https://issues.apache.org/jira/browse/MESOS-3405


Repository: mesos


Description
---

Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
converts a `google::protobuf::Message` into a `JSON::Object`.
We should add the support for `google::protobuf::RepeatedPtrField` by 
introducing overloaded functions.


Diffs (updated)
-

  src/common/http.cpp f56d8a1 
  src/docker/executor.cpp 1e49013 
  src/examples/persistent_volume_framework.cpp 176ac3d 
  src/launcher/executor.cpp 50b3c6e 
  src/master/contender.cpp c641305 
  src/master/http.cpp 093f793 
  src/master/maintenance.cpp 5fe9358 
  src/master/registrar.cpp 1117232 
  src/slave/containerizer/fetcher.cpp e0d02d5 
  src/slave/containerizer/mesos/containerizer.cpp d1fc5a4 
  src/slave/monitor.cpp aa6e958 
  src/tests/containerizer/launch_tests.cpp de655ec 
  src/tests/fault_tolerance_tests.cpp f78a291 
  src/tests/master_maintenance_tests.cpp e89ce3b 
  src/tests/master_tests.cpp ee24739 
  src/tests/mesos.hpp 3e58b45 
  src/tests/mesos.cpp ab2d85b 
  src/tests/monitor_tests.cpp 583e711 
  src/tests/reservation_endpoints_tests.cpp f5f9c48 
  src/tests/resources_tests.cpp 6584fc6 
  src/tests/scheduler_http_api_tests.cpp d338a1b 
  src/tests/script.cpp bcc1fab 
  src/tests/slave_tests.cpp 10a4fa7 
  src/usage/main.cpp 86fd796 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 38335: Add JSON::protobuf for google::protobuf::RepeatedPtrField

2015-10-19 Thread Klaus Ma 


> On Sept. 29, 2015, 3:48 p.m., Alexander Rukletsov wrote:
> > src/tests/mesos.hpp, line 1844
> > 
> >
> > We tend not to use `typedef`s in the codebase. However, this looks like 
> > a good idea, but I would place it in `Resources`, e.g. 
> > `Resources::ProtoType`.
> > 
> > However, I'll leave the decision whether to use `typedef` or not to you 
> > and your shepherd.

It's related to test only, so keep it in test module for now.


- Klaus


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


On Oct. 20, 2015, 5:37 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38335/
> ---
> 
> (Updated Oct. 20, 2015, 5:37 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp f56d8a1 
>   src/docker/executor.cpp 1e49013 
>   src/examples/persistent_volume_framework.cpp 176ac3d 
>   src/launcher/executor.cpp 50b3c6e 
>   src/master/contender.cpp c641305 
>   src/master/http.cpp 093f793 
>   src/master/maintenance.cpp 5fe9358 
>   src/master/registrar.cpp 1117232 
>   src/slave/containerizer/fetcher.cpp e0d02d5 
>   src/slave/containerizer/mesos/containerizer.cpp d1fc5a4 
>   src/slave/monitor.cpp aa6e958 
>   src/tests/containerizer/launch_tests.cpp de655ec 
>   src/tests/fault_tolerance_tests.cpp f78a291 
>   src/tests/master_maintenance_tests.cpp e89ce3b 
>   src/tests/master_tests.cpp ee24739 
>   src/tests/mesos.hpp 3e58b45 
>   src/tests/mesos.cpp ab2d85b 
>   src/tests/monitor_tests.cpp 583e711 
>   src/tests/reservation_endpoints_tests.cpp f5f9c48 
>   src/tests/resources_tests.cpp 6584fc6 
>   src/tests/scheduler_http_api_tests.cpp d338a1b 
>   src/tests/script.cpp bcc1fab 
>   src/tests/slave_tests.cpp 10a4fa7 
>   src/usage/main.cpp 86fd796 
> 
> Diff: https://reviews.apache.org/r/38335/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38335: Add JSON::protobuf for google::protobuf::RepeatedPtrField

2015-09-29 Thread Alexander Rukletsov 

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



src/master/http.cpp (lines 261 - 262)


Fits in one line.



src/tests/mesos.hpp (line 1844)


We tend not to use `typedef`s in the codebase. However, this looks like a 
good idea, but I would place it in `Resources`, e.g. `Resources::ProtoType`.

However, I'll leave the decision whether to use `typedef` or not to you and 
your shepherd.



src/tests/reservation_endpoints_tests.cpp (line 105)


How about `static_cast`? It's more explicit. Here and everywhere else.


- Alexander Rukletsov


On Sept. 24, 2015, 2:43 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38335/
> ---
> 
> (Updated Sept. 24, 2015, 2:43 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 99b843a 
>   src/docker/executor.cpp 1e49013 
>   src/examples/persistent_volume_framework.cpp 426d338 
>   src/launcher/executor.cpp 50b3c6e 
>   src/master/contender.cpp 67562f1 
>   src/master/http.cpp cd37c91 
>   src/master/maintenance.cpp 5fe9358 
>   src/master/registrar.cpp d81560a 
>   src/slave/containerizer/fetcher.cpp 2b2298c 
>   src/slave/containerizer/mesos/containerizer.cpp b904b2d 
>   src/slave/monitor.cpp 8d8b422 
>   src/tests/containerizer/launch_tests.cpp de655ec 
>   src/tests/fault_tolerance_tests.cpp c97bc46 
>   src/tests/http_api_tests.cpp 7938bbb 
>   src/tests/master_maintenance_tests.cpp 89ad138 
>   src/tests/master_tests.cpp f26344d 
>   src/tests/mesos.hpp 3e58b45 
>   src/tests/mesos.cpp ab2d85b 
>   src/tests/monitor_tests.cpp 583e711 
>   src/tests/reservation_endpoints_tests.cpp 398a2e1 
>   src/tests/resources_tests.cpp 6584fc6 
>   src/tests/script.cpp bcc1fab 
>   src/tests/slave_tests.cpp dccdbb0 
>   src/usage/main.cpp 86fd796 
> 
> Diff: https://reviews.apache.org/r/38335/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38335: Add JSON::protobuf for google::protobuf::RepeatedPtrField

2015-09-24 Thread Klaus Ma 

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

(Updated Sept. 24, 2015, 2:43 p.m.)


Review request for mesos and Michael Park.


Changes
---

Resolved code conflict


Bugs: MESOS-3405
https://issues.apache.org/jira/browse/MESOS-3405


Repository: mesos


Description
---

Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
converts a `google::protobuf::Message` into a `JSON::Object`.
We should add the support for `google::protobuf::RepeatedPtrField` by 
introducing overloaded functions.


Diffs (updated)
-

  src/common/http.cpp 99b843a 
  src/docker/executor.cpp 1e49013 
  src/examples/persistent_volume_framework.cpp 426d338 
  src/launcher/executor.cpp 50b3c6e 
  src/master/contender.cpp 67562f1 
  src/master/http.cpp cd37c91 
  src/master/maintenance.cpp 5fe9358 
  src/master/registrar.cpp d81560a 
  src/slave/containerizer/fetcher.cpp 2b2298c 
  src/slave/containerizer/mesos/containerizer.cpp b904b2d 
  src/slave/monitor.cpp 8d8b422 
  src/tests/containerizer/launch_tests.cpp de655ec 
  src/tests/fault_tolerance_tests.cpp c97bc46 
  src/tests/http_api_tests.cpp 7938bbb 
  src/tests/master_maintenance_tests.cpp 89ad138 
  src/tests/master_tests.cpp f26344d 
  src/tests/mesos.hpp 3e58b45 
  src/tests/mesos.cpp ab2d85b 
  src/tests/monitor_tests.cpp 583e711 
  src/tests/reservation_endpoints_tests.cpp 398a2e1 
  src/tests/resources_tests.cpp 6584fc6 
  src/tests/script.cpp bcc1fab 
  src/tests/slave_tests.cpp dccdbb0 
  src/usage/main.cpp 86fd796 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Review Request 38335: Add JSON::protobuf for google::protobuf::RepeatedPtrField

2015-09-13 Thread Klaus Ma 

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

Review request for mesos and Michael Park.


Bugs: MESOS-3405
https://issues.apache.org/jira/browse/MESOS-3405


Repository: mesos


Description
---

Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
converts a `google::protobuf::Message` into a `JSON::Object`.
We should add the support for `google::protobuf::RepeatedPtrField` by 
introducing overloaded functions.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp f28138c 
  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 57d5fdf 
  3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 850650c 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d3 
  include/mesos/resources.hpp 6c3a065 
  src/common/http.cpp 9c0d31e 
  src/master/http.cpp 73e8857 
  src/slave/containerizer/mesos/containerizer.cpp 1b83a87 
  src/tests/reservation_endpoints_tests.cpp dfab497 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 38335: Add JSON::protobuf for google::protobuf::RepeatedPtrField

2015-09-13 Thread Mesos ReviewBot 

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


Bad patch!

Reviews applied: [38335]

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

Error:
 2015-09-13 16:39:45 URL:https://reviews.apache.org/r/38335/diff/raw/ 
[27283/27283] -> "38335.patch" [1]
Successfully applied: Add JSON::protobuf for google::protobuf::RepeatedPtrField

Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
converts a `google::protobuf::Message` into a `JSON::Object`.
We should add the support for `google::protobuf::RepeatedPtrField` by 
introducing overloaded functions.


Review: https://reviews.apache.org/r/38335
Checking 9 files using filter 
--filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/operators,+whitespace/semicolon,+whitespace/tab,+whitespace/todo
Total errors found: 0
ERROR: Commit spanning multiple projects.

Please use separate commits for mesos, libprocess and stout.

Paths grouped by project:
mesos:
  include/mesos/resources.hpp
  src/common/http.cpp
  src/master/http.cpp
  src/slave/containerizer/mesos/containerizer.cpp
  src/tests/reservation_endpoints_tests.cpp
stout:
  3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp
  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp
  3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto
Failed to commit patch

- Mesos ReviewBot


On Sept. 13, 2015, 4:19 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38335/
> ---
> 
> (Updated Sept. 13, 2015, 4:19 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp f28138c 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 57d5fdf 
>   3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 850650c 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d3 
>   include/mesos/resources.hpp 6c3a065 
>   src/common/http.cpp 9c0d31e 
>   src/master/http.cpp 73e8857 
>   src/slave/containerizer/mesos/containerizer.cpp 1b83a87 
>   src/tests/reservation_endpoints_tests.cpp dfab497 
> 
> Diff: https://reviews.apache.org/r/38335/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38335: Add JSON::protobuf for google::protobuf::RepeatedPtrField

2015-09-13 Thread Klaus Ma 

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

(Updated Sept. 14, 2015, 5:15 a.m.)


Review request for mesos and Michael Park.


Changes
---

Add dependency to RB-38342, and remove `JSON::Protobuf`


Bugs: MESOS-3405
https://issues.apache.org/jira/browse/MESOS-3405


Repository: mesos


Description
---

Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
converts a `google::protobuf::Message` into a `JSON::Object`.
We should add the support for `google::protobuf::RepeatedPtrField` by 
introducing overloaded functions.


Diffs (updated)
-

  include/mesos/resources.hpp 6c3a065 
  src/common/http.cpp 9c0d31e 
  src/examples/persistent_volume_framework.cpp 426d338 
  src/launcher/executor.cpp 50b3c6e 
  src/master/contender.cpp 67562f1 
  src/master/http.cpp 73e8857 
  src/master/registrar.cpp d81560a 
  src/slave/containerizer/fetcher.cpp 2b2298c 
  src/slave/containerizer/mesos/containerizer.cpp 1b83a87 
  src/slave/monitor.cpp 8d8b422 
  src/tests/containerizer/launch_tests.cpp d211fc0 
  src/tests/fault_tolerance_tests.cpp 061e099 
  src/tests/http_api_tests.cpp 7938bbb 
  src/tests/master_maintenance_tests.cpp fb8dca3 
  src/tests/master_tests.cpp dd65fcc 
  src/tests/mesos.cpp ab2d85b 
  src/tests/reservation_endpoints_tests.cpp dfab497 
  src/tests/script.cpp bcc1fab 
  src/tests/slave_tests.cpp 447c43c 
  src/usage/main.cpp 86fd796 

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


Testing
---

make
make check


Thanks,

Klaus Ma