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




3rdparty/stout/include/stout/protobuf.hpp
Lines 454-538 (patched)
<https://reviews.apache.org/r/66025/#comment279344>

    As Ben mentioned, this is to match the "Notes" column of Google's JSON 
apping table ( 
https://developers.google.com/protocol-buffers/docs/proto3#json), except that 
the documentation missed the note that string values are also accepted for 
boolean fields.
    
    Therefore, we probably should also include string -> float/double 
conversion. Given this, I'd suggest that we do the following instead to further 
simplify the logic:
    
    ```
    case ...: // All 32-bit and 64-bit ints, float an double
    case ...: {
      Try<JSON::Number> number = JSON::parse<JSON::Number>(string.value);
    
      if (number.isError()) {
        return Error(...);
      }
    
      return operator()(number);
    }
    case TYPE_BOOL: {
      Try<JSON::Boolean> boolean = JSON::parse<JSON::Boolean>(string.value);
      
      if (boolean.isError()) {
        return Error(...);
      }
      
      return operator()(boolean);
    }
    ```


- Chun-Hung Hsiao


On March 12, 2018, 1:28 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66025/
> -----------------------------------------------------------
> 
> (Updated March 12, 2018, 1:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-8656
>     https://issues.apache.org/jira/browse/MESOS-8656
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previsouly when converting a JSON to a protobuf message in stout, we
> cannot handle the fields like below which are actually valid.
>   "int32": "-2147483647"
>   "int64": "-9223372036854775807"
>   "bool": "true"
> The conversion will fail with an error like "Not expecting a JSON string
> for field 'int32'".
> 
> So in this patch, `Try<Nothing> operator()(const JSON::String& string)`
> was enhanced to be able to convert `JSON::String` to bool and integers.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/protobuf.hpp 
> 4a1605e5130dbf7e8286dbb43d0d04ab4394e79a 
> 
> 
> Diff: https://reviews.apache.org/r/66025/diff/1/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Reply via email to