> On March 13, 2018, 9:22 a.m., Chun-Hung Hsiao wrote: > > 3rdparty/stout/include/stout/protobuf.hpp > > Lines 454-538 (patched) > > <https://reviews.apache.org/r/66025/diff/1/?file=1973649#file1973649line455> > > > > 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); > > } > > ```
Agree, thanks! - Qian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66025/#review199049 ----------------------------------------------------------- On March 12, 2018, 9: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, 9: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 > >
