You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Klaus Ma <kl...@cguru.net> on 2015/09/13 18:19:10 UTC
Review Request 38335: Add JSON::protobuf for
google::protobuf::RepeatedPtrField
-----------------------------------------------------------
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<T>` 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
Posted by Klaus Ma <kl...@cguru.net>.
> On Sept. 29, 2015, 3:48 p.m., Alexander Rukletsov wrote:
> > src/tests/mesos.hpp, line 1844
> > <https://reviews.apache.org/r/38335/diff/3/?file=1084054#file1084054line1844>
> >
> > 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<T>` 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
Posted by Michael Park <mc...@gmail.com>.
> On Sept. 29, 2015, 3:48 p.m., Alexander Rukletsov wrote:
> > src/tests/mesos.hpp, line 1844
> > <https://reviews.apache.org/r/38335/diff/3/?file=1084054#file1084054line1844>
> >
> > 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<Resource>`. 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<ResourcesProtobufType>(dynamicallyReserved)));
```
could be:
```cpp
auto protobuf = JSON::protobuf(
static_cast<const RepeatedPtrField<Resource>&>(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<T>` 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
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38335/#review100961
-----------------------------------------------------------
src/master/http.cpp (lines 261 - 262)
<https://reviews.apache.org/r/38335/#comment158251>
Fits in one line.
src/tests/mesos.hpp (line 1844)
<https://reviews.apache.org/r/38335/#comment158249>
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)
<https://reviews.apache.org/r/38335/#comment158250>
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<T>` 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
Posted by Klaus Ma <kl...@gmail.com>.
> On Oct. 23, 2015, 4:53 a.m., Jan Schlicht wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 800
> > <https://reviews.apache.org/r/38335/diff/4/?file=1101469#file1101469line800>
> >
> > 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<T>` 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
Posted by Michael Park <mc...@gmail.com>.
> On Oct. 22, 2015, 8:53 p.m., Jan Schlicht wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 800
> > <https://reviews.apache.org/r/38335/diff/4/?file=1101469#file1101469line800>
> >
> > 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<T>` 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
Posted by Michael Park <mc...@gmail.com>.
> On Oct. 22, 2015, 8:53 p.m., Jan Schlicht wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 800
> > <https://reviews.apache.org/r/38335/diff/4/?file=1101469#file1101469line800>
> >
> > 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<T>` 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
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/38335/#comment161712>
Not yours, but please s/push_back/emplace_back
src/tests/reservation_endpoints_tests.cpp (line 830)
<https://reviews.apache.org/r/38335/#comment161711>
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<T>` 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
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38335/#review103231
-----------------------------------------------------------
Bad patch!
Reviews applied: [38342, 38335]
Failed command: ./support/apply-review.sh -n -r 38335
Error:
2015-10-20 08:29:53 URL:https://reviews.apache.org/r/38335/diff/raw/ [26873/26873] -> "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<T>` by introducing overloaded functions.
Review: https://reviews.apache.org/r/38335
Checking 24 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
src/tests/reservation_endpoints_tests.cpp:830: Lines should be <= 80 characters long [whitespace/line_length] [2]
Total errors found: 1
Failed to commit patch
- Mesos ReviewBot
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<T>` 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
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/38335/#comment163318>
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<T>` 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
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
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<T>` 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
Posted by Klaus Ma <kl...@gmail.com>.
-----------------------------------------------------------
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<T>` 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
Posted by Klaus Ma <kl...@gmail.com>.
-----------------------------------------------------------
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<T>` 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
Posted by Klaus Ma <kl...@cguru.net>.
-----------------------------------------------------------
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<T>` 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
Posted by Klaus Ma <kl...@cguru.net>.
-----------------------------------------------------------
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<T>` 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
Re: Review Request 38335: Add JSON::protobuf for
google::protobuf::RepeatedPtrField
Posted by Klaus Ma <kl...@cguru.net>.
-----------------------------------------------------------
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<T>` 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
Re: Review Request 38335: Add JSON::protobuf for
google::protobuf::RepeatedPtrField
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
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<T>` 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<T>` 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
>
>