-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53567/#review155337
-----------------------------------------------------------
I think we can definitely improve how we report error messages to the user, but
I wonder about consistency. Couldn't you argue that probably ~90% of the places
we `return Error(...);` in `validation.cpp` would equivalently benefit from
presenting the stringified version of the operator that failed validation?
e.g., to pick a few errors at random:
* `Error("'persistence' is not set in DiskInfo");`
* `Error("Expecting 'volume' to be set for persistent volume");`
* `Error("Read-only persistent volume not supported");`
* `Error("'ExecutorInfo.type' must be 'CUSTOM'");`
* `Error("'ExecutorInfo.command' must be set");`
In any case, if we do change the suggested errors, we should probably update
the similar error messages in `Option<Error> validate(const
Offer::Operation::Reserve& reserve, const Option<string>& principal)` for
consistency.
- Neil Conway
On Nov. 8, 2016, 4:56 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53567/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2016, 4:56 p.m.)
>
>
> Review request for mesos, (Disabled_DoNotUse) Anindya Sinha, Greg Mann, and
> Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added more context about the volume in the error message and removed
> unnecessary words.
>
> Note that the callers to this function prepends framework ID in logging this
> error.
>
> More context for this change at /r/52587/
>
>
> Diffs
> -----
>
> src/master/validation.cpp 658f0dd1085c83cd82f8d2b0e2c4ec0ba785bb92
>
> Diff: https://reviews.apache.org/r/53567/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
