> On Sept. 29, 2015, 12:14 p.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, lines 762-763 > > <https://reviews.apache.org/r/38342/diff/4/?file=1085227#file1085227line762> > > > > 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.
I would like us to document the motivation for `ABORT()` here. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/#review100953 ----------------------------------------------------------- On Oct. 4, 2015, 11:29 a.m., Klaus Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38342/ > ----------------------------------------------------------- > > (Updated Oct. 4, 2015, 11:29 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 > >
