mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrei Sekretenko <asekrete...@mesosphere.io>
Subject Re: Review Request 71748: Support jsonifying v0 protobuf to v1 protobuf.
Date Tue, 19 Nov 2019 17:29:01 GMT

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


Fix it, then Ship it!





src/common/http.cpp
Lines 177 (patched)
<https://reviews.apache.org/r/71748/#comment306541>

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



src/common/http.cpp
Lines 253 (patched)
<https://reviews.apache.org/r/71748/#comment306542>

    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?


- Andrei Sekretenko


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