You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Mahler <bm...@apache.org> on 2019/12/12 23:12:05 UTC

Review Request 71908: Updated protobuf -> JSON mapping to emit empty repeated fields / maps.

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

Review request for mesos, Andrei Sekretenko and Greg Mann.


Bugs: MESOS-6568
    https://issues.apache.org/jira/browse/MESOS-6568


Repository: mesos


Description
-------

Per MESOS-6568, the omission of empty repeated fields / maps is
inconsistent with the standard proto3 -> JSON mapping. An empty
array is also more convenient to program against since you don't
need to special case the empty case with an absence check.

This may break any tests that rely on exact string outputs, but
it generally should not break any logic.


Diffs
-----

  src/common/http.cpp c5b2a91958c870e272895520ba04fc5287891c3c 


Diff: https://reviews.apache.org/r/71908/diff/1/


Testing
-------

make check


Thanks,

Benjamin Mahler


Re: Review Request 71908: Updated protobuf -> JSON mapping to emit empty repeated fields / maps.

Posted by Andrei Sekretenko <as...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71908/#review219121
-----------------------------------------------------------




src/common/http.cpp
Line 210 (original), 210 (patched)
<https://reviews.apache.org/r/71908/#comment307221>

    Do I understand correctly that this behavior detail is not covered by any Mesos tests? 
    
    Maybe it is worth adding something similar to HTTP.AsV1Protobuf but with empty map/repeated field?


- Andrei Sekretenko


On Dec. 12, 2019, 11:12 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71908/
> -----------------------------------------------------------
> 
> (Updated Dec. 12, 2019, 11:12 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Greg Mann.
> 
> 
> Bugs: MESOS-6568
>     https://issues.apache.org/jira/browse/MESOS-6568
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Per MESOS-6568, the omission of empty repeated fields / maps is
> inconsistent with the standard proto3 -> JSON mapping. An empty
> array is also more convenient to program against since you don't
> need to special case the empty case with an absence check.
> 
> This may break any tests that rely on exact string outputs, but
> it generally should not break any logic.
> 
> 
> Diffs
> -----
> 
>   src/common/http.cpp c5b2a91958c870e272895520ba04fc5287891c3c 
> 
> 
> Diff: https://reviews.apache.org/r/71908/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 71908: Updated protobuf -> JSON mapping to emit empty repeated fields / maps.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71908/#review219022
-----------------------------------------------------------



Patch looks great!

Reviews applied: [71906, 71907, 71908]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Dec. 12, 2019, 11:12 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71908/
> -----------------------------------------------------------
> 
> (Updated Dec. 12, 2019, 11:12 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Greg Mann.
> 
> 
> Bugs: MESOS-6568
>     https://issues.apache.org/jira/browse/MESOS-6568
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Per MESOS-6568, the omission of empty repeated fields / maps is
> inconsistent with the standard proto3 -> JSON mapping. An empty
> array is also more convenient to program against since you don't
> need to special case the empty case with an absence check.
> 
> This may break any tests that rely on exact string outputs, but
> it generally should not break any logic.
> 
> 
> Diffs
> -----
> 
>   src/common/http.cpp c5b2a91958c870e272895520ba04fc5287891c3c 
> 
> 
> Diff: https://reviews.apache.org/r/71908/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>