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/07/15 03:47:30 UTC

Review Request 36499: Implemented the UPDATE Event handler in the scheduler driver.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36499/
-----------------------------------------------------------

Review request for mesos and Vinod Kone.


Bugs: MESOS-2910
    https://issues.apache.org/jira/browse/MESOS-2910


Repository: mesos


Description
-------

See above.


Diffs
-----

  include/mesos/type_utils.hpp e7bfe8ca60af945e76b5d85ab37cc97b17ff1b4a 
  src/common/type_utils.cpp 19f79b47539ab51a5dff97f381a44c679cf5ecaf 
  src/sched/sched.cpp e372a15db035f74d525561839b873ed659e2c33f 
  src/tests/scheduler_event_call_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/36499/diff/


Testing
-------

Added a test.


Thanks,

Ben Mahler


Re: Review Request 36499: Implemented the UPDATE Event handler in the scheduler driver.

Posted by Ben Mahler <be...@gmail.com>.

> On July 16, 2015, 6:57 p.m., Vinod Kone wrote:
> > src/tests/scheduler_event_call_tests.cpp, lines 331-332
> > <https://reviews.apache.org/r/36499/diff/1/?file=1011999#file1011999line331>
> >
> >     why this check?

I need the frameworkId from the message, so wanted to make sure the message parsed correctly before grabbing the framework id.


> On July 16, 2015, 6:57 p.m., Vinod Kone wrote:
> > src/tests/scheduler_event_call_tests.cpp, line 380
> > <https://reviews.apache.org/r/36499/diff/1/?file=1011999#file1011999line380>
> >
> >     No need for driver.stop() and driver.join()? Shutdown()?

The driver destructor and the mesos test cleanup look sufficient. Was there something about this test that made you ask? Note that I don't do explicit stopping, joining, shutdown in the other tests.


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36499/#review91942
-----------------------------------------------------------


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/36499/
> -----------------------------------------------------------
> 
> (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
> -----
> 
>   include/mesos/type_utils.hpp e7bfe8ca60af945e76b5d85ab37cc97b17ff1b4a 
>   src/common/type_utils.cpp 19f79b47539ab51a5dff97f381a44c679cf5ecaf 
>   src/sched/sched.cpp e372a15db035f74d525561839b873ed659e2c33f 
>   src/tests/scheduler_event_call_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36499/diff/
> 
> 
> Testing
> -------
> 
> Added a test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 36499: Implemented the UPDATE 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/36499/#review91942
-----------------------------------------------------------

Ship it!



src/sched/sched.cpp (line 547)
<https://reviews.apache.org/r/36499/#comment145703>

    TODO refactor.



src/tests/scheduler_event_call_tests.cpp (lines 331 - 332)
<https://reviews.apache.org/r/36499/#comment145704>

    why this check?



src/tests/scheduler_event_call_tests.cpp (line 380)
<https://reviews.apache.org/r/36499/#comment145705>

    No need for driver.stop() and driver.join()? Shutdown()?


- 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/36499/
> -----------------------------------------------------------
> 
> (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
> -----
> 
>   include/mesos/type_utils.hpp e7bfe8ca60af945e76b5d85ab37cc97b17ff1b4a 
>   src/common/type_utils.cpp 19f79b47539ab51a5dff97f381a44c679cf5ecaf 
>   src/sched/sched.cpp e372a15db035f74d525561839b873ed659e2c33f 
>   src/tests/scheduler_event_call_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36499/diff/
> 
> 
> Testing
> -------
> 
> Added a test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 36499: Implemented the UPDATE Event handler in the 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/36499/#review91704
-----------------------------------------------------------


Patch looks great!

Reviews applied: [36450, 36493, 35752, 36494, 36495, 36496, 36497, 36498, 36499]

All tests passed.

- Mesos ReviewBot


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/36499/
> -----------------------------------------------------------
> 
> (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
> -----
> 
>   include/mesos/type_utils.hpp e7bfe8ca60af945e76b5d85ab37cc97b17ff1b4a 
>   src/common/type_utils.cpp 19f79b47539ab51a5dff97f381a44c679cf5ecaf 
>   src/sched/sched.cpp e372a15db035f74d525561839b873ed659e2c33f 
>   src/tests/scheduler_event_call_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36499/diff/
> 
> 
> Testing
> -------
> 
> Added a test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>