You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2015/06/23 21:26:27 UTC
Review Request 35752: Added stub Event protobuf handler to scheduler
driver.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35752/
-----------------------------------------------------------
Review request for mesos and Vinod Kone.
Bugs: MESOS-2910
https://issues.apache.org/jira/browse/MESOS-2910
Repository: mesos
Description
-------
Adds an initial handler, with an implementation for ERROR events.
Diffs
-----
src/Makefile.am dfebd2b14c9cb45c437509809fdf5ac3b0c8838c
src/sched/sched.cpp bc76c71ae9d44bdddd291049223366e38cb0fd0c
src/tests/scheduler_event_call_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/35752/diff/
Testing
-------
For now, testing needs to be done via manual injection of events. I've added a new test file for this, which will include the driver and library tests for event / call handling. We should be able to remove these tests as all of the existing tests will later test the event/call path.
Thanks,
Ben Mahler
Re: Review Request 35752: Added stub Event protobuf handler to
scheduler driver.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35752/#review89059
-----------------------------------------------------------
Patch looks great!
Reviews applied: [35752]
All tests passed.
- Mesos ReviewBot
On June 23, 2015, 7:26 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35752/
> -----------------------------------------------------------
>
> (Updated June 23, 2015, 7:26 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-2910
> https://issues.apache.org/jira/browse/MESOS-2910
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Adds an initial handler, with an implementation for ERROR events.
>
>
> Diffs
> -----
>
> src/Makefile.am dfebd2b14c9cb45c437509809fdf5ac3b0c8838c
> src/sched/sched.cpp bc76c71ae9d44bdddd291049223366e38cb0fd0c
> src/tests/scheduler_event_call_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/35752/diff/
>
>
> Testing
> -------
>
> For now, testing needs to be done via manual injection of events. I've added a new test file for this, which will include the driver and library tests for event / call handling. We should be able to remove these tests as all of the existing tests will later test the event/call path.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 35752: Added stub Event protobuf handler to
scheduler driver.
Posted by Isabel Jimenez <co...@isabeljimenez.com>.
> On June 23, 2015, 11:31 p.m., Isabel Jimenez wrote:
> > src/sched/sched.cpp, line 438
> > <https://reviews.apache.org/r/35752/diff/1/?file=990748#file990748line438>
> >
> > Aren't we using braces on switch cases syntax? like:
> > ```
> > switch (type) {
> > case ENUM: {
> > break
> > }
> > }
> > ```
>
> Till Toenshoff wrote:
> Only if needed Isabel ... So when case specific variable declarations are needed, scoping is needed.
ok, thanks Till!
- Isabel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35752/#review89085
-----------------------------------------------------------
On June 23, 2015, 7:26 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35752/
> -----------------------------------------------------------
>
> (Updated June 23, 2015, 7:26 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-2910
> https://issues.apache.org/jira/browse/MESOS-2910
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Adds an initial handler, with an implementation for ERROR events.
>
>
> Diffs
> -----
>
> src/Makefile.am dfebd2b14c9cb45c437509809fdf5ac3b0c8838c
> src/sched/sched.cpp bc76c71ae9d44bdddd291049223366e38cb0fd0c
> src/tests/scheduler_event_call_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/35752/diff/
>
>
> Testing
> -------
>
> For now, testing needs to be done via manual injection of events. I've added a new test file for this, which will include the driver and library tests for event / call handling. We should be able to remove these tests as all of the existing tests will later test the event/call path.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 35752: Added stub Event protobuf handler to
scheduler driver.
Posted by Till Toenshoff <to...@me.com>.
> On June 23, 2015, 11:31 p.m., Isabel Jimenez wrote:
> > src/sched/sched.cpp, line 438
> > <https://reviews.apache.org/r/35752/diff/1/?file=990748#file990748line438>
> >
> > Aren't we using braces on switch cases syntax? like:
> > ```
> > switch (type) {
> > case ENUM: {
> > break
> > }
> > }
> > ```
Only if needed Isabel ... So when case specific variable declarations are needed, scoping is needed.
- Till
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35752/#review89085
-----------------------------------------------------------
On June 23, 2015, 7:26 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35752/
> -----------------------------------------------------------
>
> (Updated June 23, 2015, 7:26 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-2910
> https://issues.apache.org/jira/browse/MESOS-2910
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Adds an initial handler, with an implementation for ERROR events.
>
>
> Diffs
> -----
>
> src/Makefile.am dfebd2b14c9cb45c437509809fdf5ac3b0c8838c
> src/sched/sched.cpp bc76c71ae9d44bdddd291049223366e38cb0fd0c
> src/tests/scheduler_event_call_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/35752/diff/
>
>
> Testing
> -------
>
> For now, testing needs to be done via manual injection of events. I've added a new test file for this, which will include the driver and library tests for event / call handling. We should be able to remove these tests as all of the existing tests will later test the event/call path.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 35752: Added stub Event protobuf handler to
scheduler driver.
Posted by Isabel Jimenez <co...@isabeljimenez.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35752/#review89085
-----------------------------------------------------------
src/sched/sched.cpp (line 438)
<https://reviews.apache.org/r/35752/#comment141687>
Aren't we using braces on switch cases syntax? like:
```
switch (type) {
case ENUM: {
break
}
}
```
- Isabel Jimenez
On June 23, 2015, 7:26 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35752/
> -----------------------------------------------------------
>
> (Updated June 23, 2015, 7:26 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-2910
> https://issues.apache.org/jira/browse/MESOS-2910
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Adds an initial handler, with an implementation for ERROR events.
>
>
> Diffs
> -----
>
> src/Makefile.am dfebd2b14c9cb45c437509809fdf5ac3b0c8838c
> src/sched/sched.cpp bc76c71ae9d44bdddd291049223366e38cb0fd0c
> src/tests/scheduler_event_call_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/35752/diff/
>
>
> Testing
> -------
>
> For now, testing needs to be done via manual injection of events. I've added a new test file for this, which will include the driver and library tests for event / call handling. We should be able to remove these tests as all of the existing tests will later test the event/call path.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 35752: Added stub Event protobuf handler to
scheduler driver.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35752/#review89099
-----------------------------------------------------------
src/sched/sched.cpp (lines 462 - 464)
<https://reviews.apache.org/r/35752/#comment141695>
Drop the message when event.has_error() is not true.
- Ben Mahler
On June 23, 2015, 7:26 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35752/
> -----------------------------------------------------------
>
> (Updated June 23, 2015, 7:26 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-2910
> https://issues.apache.org/jira/browse/MESOS-2910
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Adds an initial handler, with an implementation for ERROR events.
>
>
> Diffs
> -----
>
> src/Makefile.am dfebd2b14c9cb45c437509809fdf5ac3b0c8838c
> src/sched/sched.cpp bc76c71ae9d44bdddd291049223366e38cb0fd0c
> src/tests/scheduler_event_call_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/35752/diff/
>
>
> Testing
> -------
>
> For now, testing needs to be done via manual injection of events. I've added a new test file for this, which will include the driver and library tests for event / call handling. We should be able to remove these tests as all of the existing tests will later test the event/call path.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 35752: Added stub Event protobuf handler to
scheduler driver.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35752/#review89093
-----------------------------------------------------------
Ship it!
src/tests/scheduler_event_call_tests.cpp (line 54)
<https://reviews.apache.org/r/35752/#comment141691>
s/EventCall/Event/ ?
src/tests/scheduler_event_call_tests.cpp (line 68)
<https://reviews.apache.org/r/35752/#comment141692>
I think we decided to not do Times(1) explicitly.
- Vinod Kone
On June 23, 2015, 7:26 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35752/
> -----------------------------------------------------------
>
> (Updated June 23, 2015, 7:26 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-2910
> https://issues.apache.org/jira/browse/MESOS-2910
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Adds an initial handler, with an implementation for ERROR events.
>
>
> Diffs
> -----
>
> src/Makefile.am dfebd2b14c9cb45c437509809fdf5ac3b0c8838c
> src/sched/sched.cpp bc76c71ae9d44bdddd291049223366e38cb0fd0c
> src/tests/scheduler_event_call_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/35752/diff/
>
>
> Testing
> -------
>
> For now, testing needs to be done via manual injection of events. I've added a new test file for this, which will include the driver and library tests for event / call handling. We should be able to remove these tests as all of the existing tests will later test the event/call path.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 35752: Implemented the ERROR Event handler in the
scheduler driver.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35752/#review91798
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On July 15, 2015, 1:47 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35752/
> -----------------------------------------------------------
>
> (Updated July 15, 2015, 1:47 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-2910
> https://issues.apache.org/jira/browse/MESOS-2910
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See above.
>
>
> Diffs
> -----
>
> src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c
> src/sched/sched.cpp e372a15db035f74d525561839b873ed659e2c33f
> src/tests/scheduler_event_call_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/35752/diff/
>
>
> Testing
> -------
>
> For now, testing needs to be done via manual injection of events. I've added a new test file for this, which will include the driver and library tests for event / call handling. We should be able to remove these tests as all of the existing tests will later test the event/call path.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 35752: Implemented the ERROR Event handler in the
scheduler driver.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35752/
-----------------------------------------------------------
(Updated July 15, 2015, 1:47 a.m.)
Review request for mesos and Vinod Kone.
Changes
-------
Rebased, pulled out the stub handler change into:
https://reviews.apache.org/r/36493/
Summary (updated)
-----------------
Implemented the ERROR Event handler in the scheduler driver.
Bugs: MESOS-2910
https://issues.apache.org/jira/browse/MESOS-2910
Repository: mesos
Description (updated)
-------
See above.
Diffs (updated)
-----
src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c
src/sched/sched.cpp e372a15db035f74d525561839b873ed659e2c33f
src/tests/scheduler_event_call_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/35752/diff/
Testing
-------
For now, testing needs to be done via manual injection of events. I've added a new test file for this, which will include the driver and library tests for event / call handling. We should be able to remove these tests as all of the existing tests will later test the event/call path.
Thanks,
Ben Mahler