----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/#review100953 -----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 621) <https://reviews.apache.org/r/38342/#comment158240> Not yours, but while you're editing here, can you `.reserve()` please? 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 638) <https://reviews.apache.org/r/38342/#comment158241> ditto 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (lines 760 - 761) <https://reviews.apache.org/r/38342/#comment158239> I've seen your discussion with @Jan above, here is what I think regarding `ABORT` vs. `Try<>`. First off, some background. We tend to use `ABORT()` and `CHECK_*` macros when so-called "internal invariant" is violated, for example an object is being used without proper initialization, or a change is being made to an instance, that does not exists any more. On the other side `Try<>` and `Error<>` family is used when our expectation about the outer world does not hold, or, in other words, when an action cannot be completed, but no internal invariant is broken. User input, I/O, network operations are good examples of the latter case. Let's try to figure out what we have here. I would say, it's an internal invariant, and here is why. JSON is less strict as Protobuf, therefore conversion Protobuf->JSON always exists (note that this is not symmetrical, JSON->Protobuf is not always possible, hence we use `Try<>` in e.g. `protobuf::parse<>()`). The only reason it may fail (remember we do not handle OOM exceptions) is because we convert a protobuf message of a yet unknown format (most probably future proto versions), which is a violation of an internal invariant! Therefore I would suggest we keep `ABORT()` but document in the preambular comment why we use `ABORT()` and not `Try<>`, explaining what internal invariant is in this case. - Alexander Rukletsov On Sept. 27, 2015, 1:34 a.m., Klaus Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38342/ > ----------------------------------------------------------- > > (Updated Sept. 27, 2015, 1:34 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 > ----- > > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 2285ce9 > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 68328a2 > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 8ebb798 > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 34eb6d0 > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 920f5c9 > > Diff: https://reviews.apache.org/r/38342/diff/ > > > Testing > ------- > > cd 3rdparty/libprocess/3rdparty/stout > ./boostrap > ./configure > make > > > Thanks, > > Klaus Ma > >
