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 Hindman <be...@berkeley.edu> on 2017/01/08 07:51:11 UTC
Review Request 55324: Included decoder error strings.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55324/
-----------------------------------------------------------
Review request for mesos and Benjamin Mahler.
Repository: mesos
Description
-------
Included decoder error strings.
Diffs
-----
3rdparty/libprocess/src/decoder.hpp 4c779d42548958e610142438a57529ccb4478053
3rdparty/libprocess/src/tests/decoder_tests.cpp 87563f4fcdfaa2a33c4482533ddff24e062e603a
Diff: https://reviews.apache.org/r/55324/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request 55324: Included decoder error strings.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55324/#review160964
-----------------------------------------------------------
Any reason not to expose the error via the API so that the caller can log it? Right now it's only being exposed in one case (failure during body decoding) to the consumer of the body.
3rdparty/libprocess/src/decoder.hpp (lines 78 - 81)
<https://reviews.apache.org/r/55324/#comment232203>
Any reason for not exposing the error here as well?
3rdparty/libprocess/src/decoder.hpp (lines 92 - 95)
<https://reviews.apache.org/r/55324/#comment232205>
I was hoping that we could expose the error / failure via the API, for example we have a function like failed that returns an Option<Error>.
3rdparty/libprocess/src/decoder.hpp (lines 320 - 323)
<https://reviews.apache.org/r/55324/#comment232208>
Ditto here.
3rdparty/libprocess/src/decoder.hpp (lines 549 - 550)
<https://reviews.apache.org/r/55324/#comment232207>
It would be nice if the caller could get the error via the API, currently it's only exposed to the consumer of the body (if the failure occurs while decoding the body).
- Benjamin Mahler
On Jan. 8, 2017, 7:51 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55324/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2017, 7:51 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Included decoder error strings.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/decoder.hpp 4c779d42548958e610142438a57529ccb4478053
> 3rdparty/libprocess/src/tests/decoder_tests.cpp 87563f4fcdfaa2a33c4482533ddff24e062e603a
>
> Diff: https://reviews.apache.org/r/55324/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 55324: Included decoder error strings.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55324/#review160838
-----------------------------------------------------------
Bad patch!
Reviews applied: [55324, 55323, 55322, 55321, 55320]
Failed command: ./support/apply-review.sh -n -r 55320
Error:
https://reviews.apache.org/r/55320/diff/raw/:
2017-01-08 11:54:05 ERROR 404: NOT FOUND.
Full log: https://builds.apache.org/job/mesos-reviewbot/16638/console
- Mesos ReviewBot
On Jan. 8, 2017, 7:51 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55324/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2017, 7:51 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Included decoder error strings.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/decoder.hpp 4c779d42548958e610142438a57529ccb4478053
> 3rdparty/libprocess/src/tests/decoder_tests.cpp 87563f4fcdfaa2a33c4482533ddff24e062e603a
>
> Diff: https://reviews.apache.org/r/55324/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>