You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Meng Zhu <mz...@mesosphere.io> on 2019/03/13 00:10:36 UTC
Review Request 70198: Added validation method for input scalar values.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70198/
-----------------------------------------------------------
Review request for mesos and Benjamin Mahler.
Repository: mesos
Description
-------
The method validates the input scalar values to be
non-negative normal values ("normal" as defined by
`std::fpclassify`).
Also refactored some related validation logic and tests.
Diffs
-----
src/common/validation.hpp d09528f56e8b3675d677159c597399688019e65e
src/common/validation.cpp b622a3284cfc528f8245b0300df667d525b7eea3
src/common/values.cpp c788302b928747a1aed66efd4d654711410928f1
src/tests/common_validation_tests.cpp 214b993fa1942f15cfb4c732a5d280274d0512e9
src/tests/master_validation_tests.cpp 9568d39accb00eec68b622fdc7f02249fefbc063
src/v1/values.cpp 5fd9dc5a03ec7b8f6ed31acb3f868764a45b2bfd
Diff: https://reviews.apache.org/r/70198/diff/1/
Testing
-------
make check
Thanks,
Meng Zhu
Re: Review Request 70198: Added validation method for input scalar
values.
Posted by Meng Zhu <mz...@mesosphere.io>.
> On March 13, 2019, 11:15 a.m., Benjamin Mahler wrote:
> > src/common/validation.hpp
> > Lines 74 (patched)
> > <https://reviews.apache.org/r/70198/diff/1/?file=2131672#file2131672line74>
> >
> > s/const//
> >
> > Was expecting this to take a Value::Scalar, might be worth a comment about why it's taking double?
Yeah, I was also thinking of that. The reason is that if we take `Value::Scalar`, we will also need to add v1 or `devovle`.
- Meng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70198/#review213671
-----------------------------------------------------------
On March 12, 2019, 5:10 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70198/
> -----------------------------------------------------------
>
> (Updated March 12, 2019, 5:10 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The method validates the input scalar values to be
> non-negative normal values ("normal" as defined by
> `std::fpclassify`).
>
> Also refactored some related validation logic and tests.
>
>
> Diffs
> -----
>
> src/common/validation.hpp d09528f56e8b3675d677159c597399688019e65e
> src/common/validation.cpp b622a3284cfc528f8245b0300df667d525b7eea3
> src/common/values.cpp c788302b928747a1aed66efd4d654711410928f1
> src/tests/common_validation_tests.cpp 214b993fa1942f15cfb4c732a5d280274d0512e9
> src/tests/master_validation_tests.cpp 9568d39accb00eec68b622fdc7f02249fefbc063
> src/v1/values.cpp 5fd9dc5a03ec7b8f6ed31acb3f868764a45b2bfd
>
>
> Diff: https://reviews.apache.org/r/70198/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 70198: Added validation method for input scalar
values.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70198/#review213671
-----------------------------------------------------------
Fix it, then Ship it!
Nice to see this consolidated!
src/common/validation.hpp
Lines 74 (patched)
<https://reviews.apache.org/r/70198/#comment299652>
s/const//
Was expecting this to take a Value::Scalar, might be worth a comment about why it's taking double?
src/common/validation.cpp
Line 629 (original), 629 (patched)
<https://reviews.apache.org/r/70198/#comment299654>
s/strinf/string/
src/common/validation.cpp
Line 634 (original), 634 (patched)
<https://reviews.apache.org/r/70198/#comment299653>
Can you include the quantity name? E.g.
```
Invalid resource quantity for 'cpu': NaN not supported
```
src/common/values.cpp
Line 729 (original), 730 (patched)
<https://reviews.apache.org/r/70198/#comment299656>
`*value_`?
src/v1/values.cpp
Line 758 (original), 760 (patched)
<https://reviews.apache.org/r/70198/#comment299655>
`*value_`?
- Benjamin Mahler
On March 13, 2019, 12:10 a.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70198/
> -----------------------------------------------------------
>
> (Updated March 13, 2019, 12:10 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The method validates the input scalar values to be
> non-negative normal values ("normal" as defined by
> `std::fpclassify`).
>
> Also refactored some related validation logic and tests.
>
>
> Diffs
> -----
>
> src/common/validation.hpp d09528f56e8b3675d677159c597399688019e65e
> src/common/validation.cpp b622a3284cfc528f8245b0300df667d525b7eea3
> src/common/values.cpp c788302b928747a1aed66efd4d654711410928f1
> src/tests/common_validation_tests.cpp 214b993fa1942f15cfb4c732a5d280274d0512e9
> src/tests/master_validation_tests.cpp 9568d39accb00eec68b622fdc7f02249fefbc063
> src/v1/values.cpp 5fd9dc5a03ec7b8f6ed31acb3f868764a45b2bfd
>
>
> Diff: https://reviews.apache.org/r/70198/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>