You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2015/07/17 23:04:20 UTC
Review Request 36586: Updated scheduler driver to send SUBSCRIBE call.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36586/
-----------------------------------------------------------
Review request for mesos and Ben Mahler.
Bugs: MESOS-3055
https://issues.apache.org/jira/browse/MESOS-3055
Repository: mesos
Description
-------
See summary.
Diffs
-----
src/sched/sched.cpp 81637961f3d931a27ac14220409571593d70b112
src/tests/master_tests.cpp 9205ec409d10fc9f7e968eed962fd9ea3c33eed5
src/tests/rate_limiting_tests.cpp 6a93df086bc0f256c7750d06f950d61f2dfb7b5c
src/tests/slave_recovery_tests.cpp de2fc280abae98a5fbbeae6e230a1bfdaf0fc86e
Diff: https://reviews.apache.org/r/36586/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request 36586: Updated scheduler driver to send SUBSCRIBE
call.
Posted by Vinod Kone <vi...@gmail.com>.
> On July 20, 2015, 9:46 p.m., Ben Mahler wrote:
> > src/tests/rate_limiting_tests.cpp, lines 134-136
> > <https://reviews.apache.org/r/36586/diff/2/?file=1015049#file1015049line134>
> >
> > Note that in these tests, we've lost the fact that some of these subscribe calls should not have a FrameworkID (i.e. register) and some should have a FrameworkID (i.e. re-register). I suppose we were technically not testing this before, but at least we had more confidence given the different types of the messages.
> >
> > Should we be adding in some expectations for 'FrameworkID' being set/unset appropriately?
If framework id is not set/unset appropriately master should barf and respond with FrameworkError, so I think we are ok.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36586/#review92326
-----------------------------------------------------------
On July 17, 2015, 11:08 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36586/
> -----------------------------------------------------------
>
> (Updated July 17, 2015, 11:08 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3055
> https://issues.apache.org/jira/browse/MESOS-3055
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 25e2d660f4ee4c0b21c887f78ad04819012966f9
> src/tests/master_tests.cpp 1e934c4b168a0afabd5065e2c8ffa131362ed29b
> src/tests/rate_limiting_tests.cpp 6a93df086bc0f256c7750d06f950d61f2dfb7b5c
> src/tests/slave_recovery_tests.cpp de2fc280abae98a5fbbeae6e230a1bfdaf0fc86e
>
> Diff: https://reviews.apache.org/r/36586/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 36586: Updated scheduler driver to send SUBSCRIBE
call.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36586/#review92326
-----------------------------------------------------------
Ship it!
Looks good, just a question around whether we want to be testing for 'FrameworkID' being set correctly, in order to distinguish subscription from re-subscription (yet another time that re-subscription seems to be a useful concept ;)).
src/tests/rate_limiting_tests.cpp (lines 134 - 136)
<https://reviews.apache.org/r/36586/#comment146421>
Note that in these tests, we've lost the fact that some of these subscribe calls should not have a FrameworkID (i.e. register) and some should have a FrameworkID (i.e. re-register). I suppose we were technically not testing this before, but at least we had more confidence given the different types of the messages.
Should we be adding in some expectations for 'FrameworkID' being set/unset appropriately?
- Ben Mahler
On July 17, 2015, 11:08 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36586/
> -----------------------------------------------------------
>
> (Updated July 17, 2015, 11:08 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3055
> https://issues.apache.org/jira/browse/MESOS-3055
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 25e2d660f4ee4c0b21c887f78ad04819012966f9
> src/tests/master_tests.cpp 1e934c4b168a0afabd5065e2c8ffa131362ed29b
> src/tests/rate_limiting_tests.cpp 6a93df086bc0f256c7750d06f950d61f2dfb7b5c
> src/tests/slave_recovery_tests.cpp de2fc280abae98a5fbbeae6e230a1bfdaf0fc86e
>
> Diff: https://reviews.apache.org/r/36586/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 36586: Updated scheduler driver to send SUBSCRIBE
call.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36586/#review92144
-----------------------------------------------------------
Patch looks great!
Reviews applied: [36586]
All tests passed.
- Mesos ReviewBot
On July 17, 2015, 11:08 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36586/
> -----------------------------------------------------------
>
> (Updated July 17, 2015, 11:08 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3055
> https://issues.apache.org/jira/browse/MESOS-3055
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 25e2d660f4ee4c0b21c887f78ad04819012966f9
> src/tests/master_tests.cpp 1e934c4b168a0afabd5065e2c8ffa131362ed29b
> src/tests/rate_limiting_tests.cpp 6a93df086bc0f256c7750d06f950d61f2dfb7b5c
> src/tests/slave_recovery_tests.cpp de2fc280abae98a5fbbeae6e230a1bfdaf0fc86e
>
> Diff: https://reviews.apache.org/r/36586/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 36586: Updated scheduler driver to send SUBSCRIBE
call.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36586/
-----------------------------------------------------------
(Updated July 22, 2015, 6:11 p.m.)
Review request for mesos and Ben Mahler.
Changes
-------
updated bug id.
Bugs: MESOS-3088
https://issues.apache.org/jira/browse/MESOS-3088
Repository: mesos
Description
-------
See summary.
Diffs
-----
src/sched/sched.cpp 25e2d660f4ee4c0b21c887f78ad04819012966f9
src/tests/master_tests.cpp 1e934c4b168a0afabd5065e2c8ffa131362ed29b
src/tests/rate_limiting_tests.cpp 6a93df086bc0f256c7750d06f950d61f2dfb7b5c
src/tests/slave_recovery_tests.cpp de2fc280abae98a5fbbeae6e230a1bfdaf0fc86e
Diff: https://reviews.apache.org/r/36586/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request 36586: Updated scheduler driver to send SUBSCRIBE
call.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36586/
-----------------------------------------------------------
(Updated July 17, 2015, 11:08 p.m.)
Review request for mesos and Ben Mahler.
Changes
-------
rebased.
Bugs: MESOS-3055
https://issues.apache.org/jira/browse/MESOS-3055
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
src/sched/sched.cpp 25e2d660f4ee4c0b21c887f78ad04819012966f9
src/tests/master_tests.cpp 1e934c4b168a0afabd5065e2c8ffa131362ed29b
src/tests/rate_limiting_tests.cpp 6a93df086bc0f256c7750d06f950d61f2dfb7b5c
src/tests/slave_recovery_tests.cpp de2fc280abae98a5fbbeae6e230a1bfdaf0fc86e
Diff: https://reviews.apache.org/r/36586/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request 36586: Updated scheduler driver to send SUBSCRIBE
call.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36586/#review92134
-----------------------------------------------------------
Bad patch!
Reviews applied: [36586]
Failed command: ./support/apply-review.sh -n -r 36586
Error:
2015-07-17 22:15:49 URL:https://reviews.apache.org/r/36586/diff/raw/ [19897/19897] -> "36586.patch" [1]
error: patch failed: src/sched/sched.cpp:662
error: src/sched/sched.cpp: patch does not apply
Failed to apply patch
- Mesos ReviewBot
On July 17, 2015, 9:04 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36586/
> -----------------------------------------------------------
>
> (Updated July 17, 2015, 9:04 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3055
> https://issues.apache.org/jira/browse/MESOS-3055
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 81637961f3d931a27ac14220409571593d70b112
> src/tests/master_tests.cpp 9205ec409d10fc9f7e968eed962fd9ea3c33eed5
> src/tests/rate_limiting_tests.cpp 6a93df086bc0f256c7750d06f950d61f2dfb7b5c
> src/tests/slave_recovery_tests.cpp de2fc280abae98a5fbbeae6e230a1bfdaf0fc86e
>
> Diff: https://reviews.apache.org/r/36586/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>