mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 71748: Support jsonifying v0 protobuf to v1 protobuf.
Date Fri, 22 Nov 2019 18:03:22 GMT


> On Nov. 19, 2019, 5:29 p.m., Andrei Sekretenko wrote:
> > src/common/http.cpp
> > Lines 177 (patched)
> > <https://reviews.apache.org/r/71748/diff/1/?file=2172059#file2172059line177>
> >
> >     Is there any reason not to file a ticket for that and link it here?
> >     
> >     This patch unblocks a way to usable V1, but is *really*  brittle (for example,
I had to run diff on several pairs of `.proto`s and two C++ excerpts to review this). We must
want to replace it with a permanent solution ;)

We have https://issues.apache.org/jira/browse/MESOS-8163 but sounds good to separate out the
idea of holding an up-to-date v1 state object in memory, since that can happen without necessarily
using a state actor to serve it. Filed https://issues.apache.org/jira/browse/MESOS-10040 and
linked them up!

I will note that ticket in this patch.


> On Nov. 19, 2019, 5:29 p.m., Andrei Sekretenko wrote:
> > src/common/http.cpp
> > Lines 253 (patched)
> > <https://reviews.apache.org/r/71748/diff/1/?file=2172059#file2172059line253>
> >
> >     There are at least three differences from the stout version that seem to be
not covered by any Mesos test, including the new one you are adding in r71749.
> >     
> >     How about covering this one (and also repeated ENUM + map/singular MESSAGE below)
in r71749? 
> >     
> >     
> >     
> >     P.S. I'm quite surprised to see that our V1 API coverage is that poor... probably
this only affects JSON?

I ended up just adding a .proto file (and pre-compiled, so no build process complication)
in the next patch.


- Benjamin


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


On Nov. 11, 2019, 11:59 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71748/
> -----------------------------------------------------------
> 
> (Updated Nov. 11, 2019, 11:59 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko, Greg Mann, and Meng Zhu.
> 
> 
> Bugs: MESOS-10026
>     https://issues.apache.org/jira/browse/MESOS-10026
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This allows us to jsonify a v0 protobuf directly to a v1 protobuf
> efficiently, with no need to `evolve()` the message (which is rather
> expensive).
> 
> The way this works is by converting all "slave" and "SLAVE" strings
> in fields and enum values, respectively, to "agent" and "AGENT".
> 
> Our current v0 to v1 conversion for the v1 operator API simply
> serializes the v0 message and de-serializes into a v1 message, which
> means all field tags and message structures are the same, except
> for field names. The only difference with field names is the use
> of "agent" in place of "slave".
> 
> 
> Diffs
> -----
> 
>   src/common/http.hpp 062586c0a5a339c7e63c89a3a893ae015d3fd26e 
>   src/common/http.cpp b79074f823d3bce2a15736c0ef4739ad13db8d9c 
> 
> 
> Diff: https://reviews.apache.org/r/71748/diff/1/
> 
> 
> Testing
> -------
> 
> Added a test in the subsequent patch.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


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