You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by James Peach <jp...@apache.org> on 2018/07/01 10:08:06 UTC
Review Request 67794: Moved `validation::healthCheck` to common code.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67794/
-----------------------------------------------------------
Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
Bugs: MESOS-9043
https://issues.apache.org/jira/browse/MESOS-9043
Repository: mesos
Description
-------
The master validation uses `validation::healthCheck`, so move
it to common validation library so that the master doesn't have
a dependency on the checker library.
Diffs
-----
src/checks/health_checker.hpp b3f508edf3ac8f6125a7bfe58ec9657c290ab258
src/checks/health_checker.cpp 497549540ea90826e6d266024af2b76e7bade8e4
src/common/validation.hpp 3f60d7ac084b5341e4f33737bff931e3c3daf404
src/common/validation.cpp 74450df1ff0eb2a892f082c60416307ecfcd9e34
src/master/validation.cpp 798fc7975cf0f0b8b029a33137faa1737d47df66
src/tests/health_check_tests.cpp 8c7100ff53e44be511af9618899bd662bd547376
Diff: https://reviews.apache.org/r/67794/diff/1/
Testing
-------
make check (Fedora 28)
Thanks,
James Peach
Re: Review Request 67794: Moved `validation::healthCheck` to common
code.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67794/#review205658
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Rukletsov
On July 3, 2018, 6:32 a.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67794/
> -----------------------------------------------------------
>
> (Updated July 3, 2018, 6:32 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
>
>
> Bugs: MESOS-9043
> https://issues.apache.org/jira/browse/MESOS-9043
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The master validation uses `validation::healthCheck`, so move
> it to common validation library so that the master doesn't have
> a dependency on the checker library.
>
>
> Diffs
> -----
>
> src/checks/health_checker.hpp b3f508edf3ac8f6125a7bfe58ec9657c290ab258
> src/checks/health_checker.cpp 497549540ea90826e6d266024af2b76e7bade8e4
> src/common/validation.hpp 3f60d7ac084b5341e4f33737bff931e3c3daf404
> src/common/validation.cpp 74450df1ff0eb2a892f082c60416307ecfcd9e34
> src/master/validation.cpp 798fc7975cf0f0b8b029a33137faa1737d47df66
> src/tests/health_check_tests.cpp 8c7100ff53e44be511af9618899bd662bd547376
>
>
> Diff: https://reviews.apache.org/r/67794/diff/2/
>
>
> Testing
> -------
>
> make check (Fedora 28)
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 67794: Moved `validation::healthCheck` to common
code.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67794/
-----------------------------------------------------------
(Updated July 3, 2018, 6:32 a.m.)
Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
Bugs: MESOS-9043
https://issues.apache.org/jira/browse/MESOS-9043
Repository: mesos
Description
-------
The master validation uses `validation::healthCheck`, so move
it to common validation library so that the master doesn't have
a dependency on the checker library.
Diffs (updated)
-----
src/checks/health_checker.hpp b3f508edf3ac8f6125a7bfe58ec9657c290ab258
src/checks/health_checker.cpp 497549540ea90826e6d266024af2b76e7bade8e4
src/common/validation.hpp 3f60d7ac084b5341e4f33737bff931e3c3daf404
src/common/validation.cpp 74450df1ff0eb2a892f082c60416307ecfcd9e34
src/master/validation.cpp 798fc7975cf0f0b8b029a33137faa1737d47df66
src/tests/health_check_tests.cpp 8c7100ff53e44be511af9618899bd662bd547376
Diff: https://reviews.apache.org/r/67794/diff/2/
Changes: https://reviews.apache.org/r/67794/diff/1-2/
Testing
-------
make check (Fedora 28)
Thanks,
James Peach
Re: Review Request 67794: Moved `validation::healthCheck` to common
code.
Posted by James Peach <jp...@apache.org>.
> On July 2, 2018, 8:17 a.m., Alexander Rukletsov wrote:
> > Can you please also move validations for checks?
> > ```
> > Option<Error> checkInfo(const CheckInfo& checkInfo);
> > Option<Error> checkStatusInfo(const CheckStatusInfo& checkStatusInfo);
> > ```
> > from "checker.hpp".
Yep done in [67795](https://reviews.apache.org/r/67795/)
- James
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67794/#review205626
-----------------------------------------------------------
On July 1, 2018, 10:08 a.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67794/
> -----------------------------------------------------------
>
> (Updated July 1, 2018, 10:08 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
>
>
> Bugs: MESOS-9043
> https://issues.apache.org/jira/browse/MESOS-9043
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The master validation uses `validation::healthCheck`, so move
> it to common validation library so that the master doesn't have
> a dependency on the checker library.
>
>
> Diffs
> -----
>
> src/checks/health_checker.hpp b3f508edf3ac8f6125a7bfe58ec9657c290ab258
> src/checks/health_checker.cpp 497549540ea90826e6d266024af2b76e7bade8e4
> src/common/validation.hpp 3f60d7ac084b5341e4f33737bff931e3c3daf404
> src/common/validation.cpp 74450df1ff0eb2a892f082c60416307ecfcd9e34
> src/master/validation.cpp 798fc7975cf0f0b8b029a33137faa1737d47df66
> src/tests/health_check_tests.cpp 8c7100ff53e44be511af9618899bd662bd547376
>
>
> Diff: https://reviews.apache.org/r/67794/diff/1/
>
>
> Testing
> -------
>
> make check (Fedora 28)
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 67794: Moved `validation::healthCheck` to common
code.
Posted by James Peach <jp...@apache.org>.
> On July 2, 2018, 8:17 a.m., Alexander Rukletsov wrote:
> > src/common/validation.hpp
> > Lines 57-58 (patched)
> > <https://reviews.apache.org/r/67794/diff/1/?file=2047211#file2047211line57>
> >
> > `validation::healthCheck` makes more sense to me than `validation::validateHealthCheck`, but the latter seems locally consistent, hence consider using it instead.
I agree, updated.
- James
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67794/#review205626
-----------------------------------------------------------
On July 1, 2018, 10:08 a.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67794/
> -----------------------------------------------------------
>
> (Updated July 1, 2018, 10:08 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
>
>
> Bugs: MESOS-9043
> https://issues.apache.org/jira/browse/MESOS-9043
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The master validation uses `validation::healthCheck`, so move
> it to common validation library so that the master doesn't have
> a dependency on the checker library.
>
>
> Diffs
> -----
>
> src/checks/health_checker.hpp b3f508edf3ac8f6125a7bfe58ec9657c290ab258
> src/checks/health_checker.cpp 497549540ea90826e6d266024af2b76e7bade8e4
> src/common/validation.hpp 3f60d7ac084b5341e4f33737bff931e3c3daf404
> src/common/validation.cpp 74450df1ff0eb2a892f082c60416307ecfcd9e34
> src/master/validation.cpp 798fc7975cf0f0b8b029a33137faa1737d47df66
> src/tests/health_check_tests.cpp 8c7100ff53e44be511af9618899bd662bd547376
>
>
> Diff: https://reviews.apache.org/r/67794/diff/1/
>
>
> Testing
> -------
>
> make check (Fedora 28)
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 67794: Moved `validation::healthCheck` to common
code.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67794/#review205626
-----------------------------------------------------------
Can you please also move validations for checks?
```
Option<Error> checkInfo(const CheckInfo& checkInfo);
Option<Error> checkStatusInfo(const CheckStatusInfo& checkStatusInfo);
```
from "checker.hpp".
src/common/validation.hpp
Lines 57-58 (patched)
<https://reviews.apache.org/r/67794/#comment288492>
`validation::healthCheck` makes more sense to me than `validation::validateHealthCheck`, but the latter seems locally consistent, hence consider using it instead.
- Alexander Rukletsov
On July 1, 2018, 10:08 a.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67794/
> -----------------------------------------------------------
>
> (Updated July 1, 2018, 10:08 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
>
>
> Bugs: MESOS-9043
> https://issues.apache.org/jira/browse/MESOS-9043
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The master validation uses `validation::healthCheck`, so move
> it to common validation library so that the master doesn't have
> a dependency on the checker library.
>
>
> Diffs
> -----
>
> src/checks/health_checker.hpp b3f508edf3ac8f6125a7bfe58ec9657c290ab258
> src/checks/health_checker.cpp 497549540ea90826e6d266024af2b76e7bade8e4
> src/common/validation.hpp 3f60d7ac084b5341e4f33737bff931e3c3daf404
> src/common/validation.cpp 74450df1ff0eb2a892f082c60416307ecfcd9e34
> src/master/validation.cpp 798fc7975cf0f0b8b029a33137faa1737d47df66
> src/tests/health_check_tests.cpp 8c7100ff53e44be511af9618899bd662bd547376
>
>
> Diff: https://reviews.apache.org/r/67794/diff/1/
>
>
> Testing
> -------
>
> make check (Fedora 28)
>
>
> Thanks,
>
> James Peach
>
>