You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Michael Park <mp...@apache.org> on 2018/01/08 18:14:44 UTC
Review Request 65021: Returned `Try` from `protobuf::read(path)`
rather than `Result`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65021/
-----------------------------------------------------------
Review request for mesos and Benjamin Mahler.
Bugs: MESOS-8375
https://issues.apache.org/jira/browse/MESOS-8375
Repository: mesos
Description
-------
The path version of `protobuf::read` used to return `Result<T>` and
returned `None` only when the file is empty (`ignorePartial` is always
`false`). The `None` return represents EOF for the "streaming" version
of `protobuf::read` that takes an FD, but for the path version an empty
file when we expected to read `T` is simply an error. Thus, we map the
`None` return to an `Error` for the path version and return a `Try<T>`.
Diffs
-----
3rdparty/stout/include/stout/protobuf.hpp f00a048fb0363f86cada97352a00a65a27f15bac
Diff: https://reviews.apache.org/r/65021/diff/1/
Testing
-------
Thanks,
Michael Park
Re: Review Request 65021: Returned `Try` from
`protobuf::read(path)` rather than `Result`.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65021/#review195098
-----------------------------------------------------------
Fix it, then Ship it!
3rdparty/stout/include/stout/protobuf.hpp
Lines 367 (patched)
<https://reviews.apache.org/r/65021/#comment274243>
Maybe say something more informative, like the file was empty? If we know that's what None means?
- Benjamin Mahler
On Jan. 8, 2018, 6:14 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65021/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2018, 6:14 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-8375
> https://issues.apache.org/jira/browse/MESOS-8375
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The path version of `protobuf::read` used to return `Result<T>` and
> returned `None` only when the file is empty (`ignorePartial` is always
> `false`). The `None` return represents EOF for the "streaming" version
> of `protobuf::read` that takes an FD, but for the path version an empty
> file when we expected to read `T` is simply an error. Thus, we map the
> `None` return to an `Error` for the path version and return a `Try<T>`.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/protobuf.hpp f00a048fb0363f86cada97352a00a65a27f15bac
>
>
> Diff: https://reviews.apache.org/r/65021/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael Park
>
>