You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Anand Mazumdar <ma...@gmail.com> on 2015/12/12 00:52:12 UTC
Review Request 41288: Introduced an callback interface for testing
HTTP based executors.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41288/
-----------------------------------------------------------
Review request for mesos, Ben Mahler and Vinod Kone.
Bugs: MESOS-3550
https://issues.apache.org/jira/browse/MESOS-3550
Repository: mesos
Description
-------
This change introduces a versioned callback interface for testing HTTP based executors. The reasoning is similar to `MESOS-3339` , the corresponding issue for Schedulers.
Diffs
-----
src/tests/mesos.hpp 2429ac5cbcd9c1a3949c11de94b542108a3c13d8
Diff: https://reviews.apache.org/r/41288/diff/
Testing
-------
make check
Thanks,
Anand Mazumdar
Re: Review Request 41288: Introduced an callback interface for
testing HTTP based executors.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41288/#review117527
-----------------------------------------------------------
src/tests/mesos.hpp (line 839)
<https://reviews.apache.org/r/41288/#comment178956>
The naming here is a bit confusing.
How about putting this class and the above in an executor namespace and do something like this:
```
namespace mesos {
namespace internal {
namespace tests {
...
...
...
namespace executor {
template<typename Event>
class Callbacks
{
};
template <typename Mesos, typename Event>
class TestMesos:: public Mesos
{
};
}
...
...
...
}
}
}
```
One issue might be that someone might be confused between the top level MesosTest class and TestMesos class, but I think the namespace should alleviate the confusion.
src/tests/mesos.hpp (line 850)
<https://reviews.apache.org/r/41288/#comment178724>
each argument should be on its own line when wrapping. put `lambda::_1` on the next line.
src/tests/mesos.hpp (line 853)
<https://reviews.apache.org/r/41288/#comment178733>
why is this public?
src/tests/mesos.hpp (line 863)
<https://reviews.apache.org/r/41288/#comment178730>
should this be an owned pointer?
src/tests/mesos.hpp (line 869)
<https://reviews.apache.org/r/41288/#comment178723>
new line.
- Vinod Kone
On Jan. 19, 2016, 10:28 p.m., Anand Mazumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41288/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2016, 10:28 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-4433
> https://issues.apache.org/jira/browse/MESOS-4433
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This change introduces a versioned callback interface for testing HTTP based executors. The reasoning is similar to `MESOS-3339` , the corresponding issue for Schedulers.
>
>
> Diffs
> -----
>
> src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac
>
> Diff: https://reviews.apache.org/r/41288/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Anand Mazumdar
>
>
Re: Review Request 41288: Introduced an callback interface for
testing HTTP based executors.
Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41288/
-----------------------------------------------------------
(Updated Feb. 9, 2016, 1:49 a.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Review comment
Bugs: MESOS-4433
https://issues.apache.org/jira/browse/MESOS-4433
Repository: mesos
Description
-------
This change introduces a versioned callback interface for testing HTTP based executors. The reasoning is similar to `MESOS-3339` , the corresponding issue for Schedulers.
Diffs (updated)
-----
src/tests/mesos.hpp e07d8aa6f1e507a91ce70763aafa134bdd9a7ec8
Diff: https://reviews.apache.org/r/41288/diff/
Testing
-------
make check
Thanks,
Anand Mazumdar
Re: Review Request 41288: Introduced an callback interface for
testing HTTP based executors.
Posted by Anand Mazumdar <ma...@gmail.com>.
> On Feb. 9, 2016, 1:10 a.m., Vinod Kone wrote:
> > src/tests/mesos.hpp, line 948
> > <https://reviews.apache.org/r/41288/diff/5/?file=1237957#file1237957line948>
> >
> > looks like you forgot to pass "this" to this callback?
Good catch. Fixed
- Anand
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41288/#review118340
-----------------------------------------------------------
On Feb. 9, 2016, 1:49 a.m., Anand Mazumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41288/
> -----------------------------------------------------------
>
> (Updated Feb. 9, 2016, 1:49 a.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-4433
> https://issues.apache.org/jira/browse/MESOS-4433
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This change introduces a versioned callback interface for testing HTTP based executors. The reasoning is similar to `MESOS-3339` , the corresponding issue for Schedulers.
>
>
> Diffs
> -----
>
> src/tests/mesos.hpp e07d8aa6f1e507a91ce70763aafa134bdd9a7ec8
>
> Diff: https://reviews.apache.org/r/41288/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Anand Mazumdar
>
>
Re: Review Request 41288: Introduced an callback interface for
testing HTTP based executors.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41288/#review118340
-----------------------------------------------------------
Fix it, then Ship it!
src/tests/mesos.hpp (line 948)
<https://reviews.apache.org/r/41288/#comment179564>
looks like you forgot to pass "this" to this callback?
- Vinod Kone
On Feb. 9, 2016, 12:12 a.m., Anand Mazumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41288/
> -----------------------------------------------------------
>
> (Updated Feb. 9, 2016, 12:12 a.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-4433
> https://issues.apache.org/jira/browse/MESOS-4433
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This change introduces a versioned callback interface for testing HTTP based executors. The reasoning is similar to `MESOS-3339` , the corresponding issue for Schedulers.
>
>
> Diffs
> -----
>
> src/tests/mesos.hpp e07d8aa6f1e507a91ce70763aafa134bdd9a7ec8
>
> Diff: https://reviews.apache.org/r/41288/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Anand Mazumdar
>
>
Re: Review Request 41288: Introduced an callback interface for
testing HTTP based executors.
Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41288/
-----------------------------------------------------------
(Updated Feb. 9, 2016, 12:12 a.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Review comments
Bugs: MESOS-4433
https://issues.apache.org/jira/browse/MESOS-4433
Repository: mesos
Description
-------
This change introduces a versioned callback interface for testing HTTP based executors. The reasoning is similar to `MESOS-3339` , the corresponding issue for Schedulers.
Diffs (updated)
-----
src/tests/mesos.hpp e07d8aa6f1e507a91ce70763aafa134bdd9a7ec8
Diff: https://reviews.apache.org/r/41288/diff/
Testing
-------
make check
Thanks,
Anand Mazumdar
Re: Review Request 41288: Introduced an callback interface for
testing HTTP based executors.
Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41288/
-----------------------------------------------------------
(Updated Feb. 8, 2016, 10:15 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Review comments from Vinod
Bugs: MESOS-4433
https://issues.apache.org/jira/browse/MESOS-4433
Repository: mesos
Description
-------
This change introduces a versioned callback interface for testing HTTP based executors. The reasoning is similar to `MESOS-3339` , the corresponding issue for Schedulers.
Diffs (updated)
-----
src/tests/mesos.hpp e07d8aa6f1e507a91ce70763aafa134bdd9a7ec8
Diff: https://reviews.apache.org/r/41288/diff/
Testing
-------
make check
Thanks,
Anand Mazumdar
Re: Review Request 41288: Introduced an callback interface for
testing HTTP based executors.
Posted by Anand Mazumdar <ma...@gmail.com>.
> On Feb. 8, 2016, 10 p.m., Vinod Kone wrote:
> > src/tests/mesos.hpp, line 960
> > <https://reviews.apache.org/r/41288/diff/3/?file=1233179#file1233179line960>
> >
> > s/executor_/executor/
> >
> > we typically don't use underscores for member variables.
Our style guide does recommend to prefer trailing underscores for member variables. From the style guide:
```
Prefer trailing underscores for use as member fields (but not required).
```
I removed the trailing underscores for now since all the other classes/structs in this file do not use it.
> On Feb. 8, 2016, 10 p.m., Vinod Kone wrote:
> > src/tests/mesos.hpp, line 965
> > <https://reviews.apache.org/r/41288/diff/3/?file=1233179#file1233179line965>
> >
> > do you need executor::TestMesos even though you are in executor namespace?
Yep, right. My bad, fixed. :-)
- Anand
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41288/#review118299
-----------------------------------------------------------
On Feb. 8, 2016, 10:15 p.m., Anand Mazumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41288/
> -----------------------------------------------------------
>
> (Updated Feb. 8, 2016, 10:15 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-4433
> https://issues.apache.org/jira/browse/MESOS-4433
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This change introduces a versioned callback interface for testing HTTP based executors. The reasoning is similar to `MESOS-3339` , the corresponding issue for Schedulers.
>
>
> Diffs
> -----
>
> src/tests/mesos.hpp e07d8aa6f1e507a91ce70763aafa134bdd9a7ec8
>
> Diff: https://reviews.apache.org/r/41288/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Anand Mazumdar
>
>
Re: Review Request 41288: Introduced an callback interface for
testing HTTP based executors.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41288/#review118299
-----------------------------------------------------------
src/tests/mesos.hpp (line 937)
<https://reviews.apache.org/r/41288/#comment179521>
s/executor/_executor/
src/tests/mesos.hpp (line 943)
<https://reviews.apache.org/r/41288/#comment179522>
put the second argument on next line for consistency.
also pass "this" as the 3rd argument for consistency with connected callback.
src/tests/mesos.hpp (line 960)
<https://reviews.apache.org/r/41288/#comment179520>
s/executor_/executor/
we typically don't use underscores for member variables.
src/tests/mesos.hpp (line 965)
<https://reviews.apache.org/r/41288/#comment179524>
do you need executor::TestMesos even though you are in executor namespace?
- Vinod Kone
On Feb. 4, 2016, 11:11 p.m., Anand Mazumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41288/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2016, 11:11 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-4433
> https://issues.apache.org/jira/browse/MESOS-4433
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This change introduces a versioned callback interface for testing HTTP based executors. The reasoning is similar to `MESOS-3339` , the corresponding issue for Schedulers.
>
>
> Diffs
> -----
>
> src/tests/mesos.hpp c2bae4767ee7372c796bfad44ed1e86db7dd3488
>
> Diff: https://reviews.apache.org/r/41288/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Anand Mazumdar
>
>
Re: Review Request 41288: Introduced an callback interface for
testing HTTP based executors.
Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41288/
-----------------------------------------------------------
(Updated Feb. 4, 2016, 11:11 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Review comments
Bugs: MESOS-4433
https://issues.apache.org/jira/browse/MESOS-4433
Repository: mesos
Description
-------
This change introduces a versioned callback interface for testing HTTP based executors. The reasoning is similar to `MESOS-3339` , the corresponding issue for Schedulers.
Diffs (updated)
-----
src/tests/mesos.hpp c2bae4767ee7372c796bfad44ed1e86db7dd3488
Diff: https://reviews.apache.org/r/41288/diff/
Testing
-------
make check
Thanks,
Anand Mazumdar
Re: Review Request 41288: Introduced an callback interface for
testing HTTP based executors.
Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41288/
-----------------------------------------------------------
(Updated Feb. 3, 2016, 10:14 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Updated deps since the previous change has now been committed. Would address review comments in next iteration.
Bugs: MESOS-4433
https://issues.apache.org/jira/browse/MESOS-4433
Repository: mesos
Description
-------
This change introduces a versioned callback interface for testing HTTP based executors. The reasoning is similar to `MESOS-3339` , the corresponding issue for Schedulers.
Diffs
-----
src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac
Diff: https://reviews.apache.org/r/41288/diff/
Testing
-------
make check
Thanks,
Anand Mazumdar
Re: Review Request 41288: Introduced an callback interface for
testing HTTP based executors.
Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41288/
-----------------------------------------------------------
(Updated Jan. 19, 2016, 10:28 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Update deps
Bugs: MESOS-4433
https://issues.apache.org/jira/browse/MESOS-4433
Repository: mesos
Description
-------
This change introduces a versioned callback interface for testing HTTP based executors. The reasoning is similar to `MESOS-3339` , the corresponding issue for Schedulers.
Diffs
-----
src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac
Diff: https://reviews.apache.org/r/41288/diff/
Testing
-------
make check
Thanks,
Anand Mazumdar
Re: Review Request 41288: Introduced an callback interface for
testing HTTP based executors.
Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41288/
-----------------------------------------------------------
(Updated Jan. 19, 2016, 8:05 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Updated Bugs link
Bugs: MESOS-4433
https://issues.apache.org/jira/browse/MESOS-4433
Repository: mesos
Description
-------
This change introduces a versioned callback interface for testing HTTP based executors. The reasoning is similar to `MESOS-3339` , the corresponding issue for Schedulers.
Diffs
-----
src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac
Diff: https://reviews.apache.org/r/41288/diff/
Testing
-------
make check
Thanks,
Anand Mazumdar
Re: Review Request 41288: Introduced an callback interface for
testing HTTP based executors.
Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41288/
-----------------------------------------------------------
(Updated Jan. 12, 2016, 9:21 a.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Rebased
Bugs: MESOS-3550
https://issues.apache.org/jira/browse/MESOS-3550
Repository: mesos
Description
-------
This change introduces a versioned callback interface for testing HTTP based executors. The reasoning is similar to `MESOS-3339` , the corresponding issue for Schedulers.
Diffs (updated)
-----
src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac
Diff: https://reviews.apache.org/r/41288/diff/
Testing
-------
make check
Thanks,
Anand Mazumdar