mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexander Rukletsov" <ruklet...@gmail.com>
Subject Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)
Date Mon, 12 Oct 2015 22:18:57 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message