You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Chun-Hung Hsiao <ch...@apache.org> on 2018/06/20 05:33:32 UTC
Review Request 67666: Added a `TestDiskProfileServer` helper.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67666/
-----------------------------------------------------------
Review request for mesos, Benjamin Bannier, Jie Yu, Joseph Wu, and Jan Schlicht.
Bugs: MESOS-8995
https://issues.apache.org/jira/browse/MESOS-8995
Repository: mesos
Description
-------
This helper class will be used to test functionalities related to
profile changes.
Diffs
-----
src/Makefile.am 2bcee1e0e198e6be009174570cdaa1c8b8372a71
src/tests/disk_profile_adaptor_tests.cpp 4485f1635f484ce6e1c7c532eedb277f5eee118b
src/tests/disk_profile_server.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/67666/diff/1/
Testing
-------
sudo make check
Thanks,
Chun-Hung Hsiao
Re: Review Request 67666: Added a `TestDiskProfileServer` helper.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67666/
-----------------------------------------------------------
(Updated July 6, 2018, 11:25 p.m.)
Review request for mesos, Benjamin Bannier, Jie Yu, Joseph Wu, and Jan Schlicht.
Changes
-------
Rebased.
Bugs: MESOS-8995
https://issues.apache.org/jira/browse/MESOS-8995
Repository: mesos
Description
-------
This helper class will be used to test functionalities related to
profile changes.
Diffs (updated)
-----
src/Makefile.am 3ac1e1ce650d7238c108d0ac4123228500053a6f
src/tests/disk_profile_adaptor_tests.cpp 4485f1635f484ce6e1c7c532eedb277f5eee118b
src/tests/disk_profile_server.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/67666/diff/3/
Changes: https://reviews.apache.org/r/67666/diff/2-3/
Testing
-------
sudo make check
Thanks,
Chun-Hung Hsiao
Re: Review Request 67666: Added a `TestDiskProfileServer` helper.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67666/
-----------------------------------------------------------
(Updated June 21, 2018, 4:33 a.m.)
Review request for mesos, Benjamin Bannier, Jie Yu, Joseph Wu, and Jan Schlicht.
Changes
-------
Addressed Benjamin's comments and added a `url()` method.
Bugs: MESOS-8995
https://issues.apache.org/jira/browse/MESOS-8995
Repository: mesos
Description
-------
This helper class will be used to test functionalities related to
profile changes.
Diffs (updated)
-----
src/Makefile.am 2bcee1e0e198e6be009174570cdaa1c8b8372a71
src/tests/disk_profile_adaptor_tests.cpp 4485f1635f484ce6e1c7c532eedb277f5eee118b
src/tests/disk_profile_server.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/67666/diff/2/
Changes: https://reviews.apache.org/r/67666/diff/1-2/
Testing
-------
sudo make check
Thanks,
Chun-Hung Hsiao
Re: Review Request 67666: Added a `TestDiskProfileServer` helper.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> On June 20, 2018, 11:01 a.m., Benjamin Bannier wrote:
> > src/tests/disk_profile_server.hpp
> > Lines 34-35 (patched)
> > <https://reviews.apache.org/r/67666/diff/1/?file=2042685#file2042685line34>
> >
> > Nit: This class doesn't really fetch, but instead provides a mock remote.
It's a helper for "module configured to fetch from HTTP URIs", not "helper to fetch from HTTP URIs". I copied this from Joseph's comments, but TBH its not nontrivial for me to parse this sentence lol. I'll simplify the comment.
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67666/#review205056
-----------------------------------------------------------
On June 20, 2018, 5:33 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67666/
> -----------------------------------------------------------
>
> (Updated June 20, 2018, 5:33 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Jie Yu, Joseph Wu, and Jan Schlicht.
>
>
> Bugs: MESOS-8995
> https://issues.apache.org/jira/browse/MESOS-8995
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This helper class will be used to test functionalities related to
> profile changes.
>
>
> Diffs
> -----
>
> src/Makefile.am 2bcee1e0e198e6be009174570cdaa1c8b8372a71
> src/tests/disk_profile_adaptor_tests.cpp 4485f1635f484ce6e1c7c532eedb277f5eee118b
> src/tests/disk_profile_server.hpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67666/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 67666: Added a `TestDiskProfileServer` helper.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67666/#review205056
-----------------------------------------------------------
Fix it, then Ship it!
src/tests/disk_profile_server.hpp
Lines 34-35 (patched)
<https://reviews.apache.org/r/67666/#comment287996>
Nit: This class doesn't really fetch, but instead provides a mock remote.
src/tests/disk_profile_server.hpp
Lines 43 (patched)
<https://reviews.apache.org/r/67666/#comment287964>
Why is this needed?
src/tests/disk_profile_server.hpp
Lines 62 (patched)
<https://reviews.apache.org/r/67666/#comment287994>
`Owned` cannot be copied (e.g., into the lambda below), we should probably use a `shared_ptr` here.
Unfortunately this implementation detail would leak into the return value, but it is the correct thing to do I belive.
src/tests/disk_profile_server.hpp
Lines 74 (patched)
<https://reviews.apache.org/r/67666/#comment287963>
Either make this a `std::unique_ptr` or make the class non-copyable (we should reference MESOS-5122 in that case).
- Benjamin Bannier
On June 20, 2018, 7:33 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67666/
> -----------------------------------------------------------
>
> (Updated June 20, 2018, 7:33 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Jie Yu, Joseph Wu, and Jan Schlicht.
>
>
> Bugs: MESOS-8995
> https://issues.apache.org/jira/browse/MESOS-8995
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This helper class will be used to test functionalities related to
> profile changes.
>
>
> Diffs
> -----
>
> src/Makefile.am 2bcee1e0e198e6be009174570cdaa1c8b8372a71
> src/tests/disk_profile_adaptor_tests.cpp 4485f1635f484ce6e1c7c532eedb277f5eee118b
> src/tests/disk_profile_server.hpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67666/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>