Re: Review Request 67990: Fixed libprocess tests against rapidjson.
> On July 24, 2018, 6:43 p.m., Alexander Rukletsov wrote: > > Can you please call out this change in the changelog? It is unfortunate we > > do such a change without notifying people on the user and dev lists with > > enough time buffer, maybe you can do it right after the chain lands so that > > folks can prepare to the 1.7 release? Sounds good, thanks for the suggestion! - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67990/#review206408 --- On July 20, 2018, 3:38 a.m., Benjamin Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67990/ > --- > > (Updated July 20, 2018, 3:38 a.m.) > > > Review request for mesos, Alexander Rukletsov and Benno Evers. > > > Bugs: MESOS-9092 > https://issues.apache.org/jira/browse/MESOS-9092 > > > Repository: mesos > > > Description > --- > > The libprocess tests were checking the serialized format of metrics, > which previously escaped forward slashes. However, this is not what > rapidjson does and it's also valid json according to ECMA-404. > > > Diffs > - > > 3rdparty/libprocess/src/tests/metrics_tests.cpp > 8590bdb4da1bead462a5b91c23bbc31905e57a47 > > > Diff: https://reviews.apache.org/r/67990/diff/1/ > > > Testing > --- > > Tested at the end of this chain, since this is split across > stout/libprocess/mesos. > > > Thanks, > > Benjamin Mahler > >
Re: Review Request 67990: Fixed libprocess tests against rapidjson.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67990/#review206408 --- Ship it! Can you please call out this change in the changelog? It is unfortunate we do such a change without notifying people on the user and dev lists with enough time buffer, maybe you can do it right after the chain lands so that folks can prepare to the 1.7 release? - Alexander Rukletsov On July 20, 2018, 3:38 a.m., Benjamin Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67990/ > --- > > (Updated July 20, 2018, 3:38 a.m.) > > > Review request for mesos, Alexander Rukletsov and Benno Evers. > > > Bugs: MESOS-9092 > https://issues.apache.org/jira/browse/MESOS-9092 > > > Repository: mesos > > > Description > --- > > The libprocess tests were checking the serialized format of metrics, > which previously escaped forward slashes. However, this is not what > rapidjson does and it's also valid json according to ECMA-404. > > > Diffs > - > > 3rdparty/libprocess/src/tests/metrics_tests.cpp > 8590bdb4da1bead462a5b91c23bbc31905e57a47 > > > Diff: https://reviews.apache.org/r/67990/diff/1/ > > > Testing > --- > > Tested at the end of this chain, since this is split across > stout/libprocess/mesos. > > > Thanks, > > Benjamin Mahler > >
Re: Review Request 67990: Fixed libprocess tests against rapidjson.
> On July 20, 2018, 11:19 a.m., Benno Evers wrote: > > Just to be clear about the consequences, this could imply that users having > > written custom tooling to parse the response of `/metrics` might see their > > scripts break, right? Yes, in general, if anyone is using a json de-serializer that doesn't conform to the spec they will potentially break since the spec allows different choices and rapidjson made different ones than we did. - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67990/#review206279 --- On July 20, 2018, 3:38 a.m., Benjamin Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67990/ > --- > > (Updated July 20, 2018, 3:38 a.m.) > > > Review request for mesos, Alexander Rukletsov and Benno Evers. > > > Bugs: MESOS-9092 > https://issues.apache.org/jira/browse/MESOS-9092 > > > Repository: mesos > > > Description > --- > > The libprocess tests were checking the serialized format of metrics, > which previously escaped forward slashes. However, this is not what > rapidjson does and it's also valid json according to ECMA-404. > > > Diffs > - > > 3rdparty/libprocess/src/tests/metrics_tests.cpp > 8590bdb4da1bead462a5b91c23bbc31905e57a47 > > > Diff: https://reviews.apache.org/r/67990/diff/1/ > > > Testing > --- > > Tested at the end of this chain, since this is split across > stout/libprocess/mesos. > > > Thanks, > > Benjamin Mahler > >
Re: Review Request 67990: Fixed libprocess tests against rapidjson.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67990/#review206279 --- Ship it! Just to be clear about the consequences, this could imply that users having written custom tooling to parse the response of `/metrics` might see their scripts break, right? - Benno Evers On July 20, 2018, 3:38 a.m., Benjamin Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67990/ > --- > > (Updated July 20, 2018, 3:38 a.m.) > > > Review request for mesos, Alexander Rukletsov and Benno Evers. > > > Bugs: MESOS-9092 > https://issues.apache.org/jira/browse/MESOS-9092 > > > Repository: mesos > > > Description > --- > > The libprocess tests were checking the serialized format of metrics, > which previously escaped forward slashes. However, this is not what > rapidjson does and it's also valid json according to ECMA-404. > > > Diffs > - > > 3rdparty/libprocess/src/tests/metrics_tests.cpp > 8590bdb4da1bead462a5b91c23bbc31905e57a47 > > > Diff: https://reviews.apache.org/r/67990/diff/1/ > > > Testing > --- > > Tested at the end of this chain, since this is split across > stout/libprocess/mesos. > > > Thanks, > > Benjamin Mahler > >
Review Request 67990: Fixed libprocess tests against rapidjson.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67990/ --- Review request for mesos, Alexander Rukletsov and Benno Evers. Bugs: MESOS-9092 https://issues.apache.org/jira/browse/MESOS-9092 Repository: mesos Description --- The libprocess tests were checking the serialized format of metrics, which previously escaped forward slashes. However, this is not what rapidjson does and it's also valid json according to ECMA-404. Diffs - 3rdparty/libprocess/src/tests/metrics_tests.cpp 8590bdb4da1bead462a5b91c23bbc31905e57a47 Diff: https://reviews.apache.org/r/67990/diff/1/ Testing --- Tested at the end of this chain, since this is split across stout/libprocess/mesos. Thanks, Benjamin Mahler
