You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Guangya Liu <gy...@apache.org> on 2017/02/07 10:10:15 UTC

Review Request 56371: Enabled `ReviveOffersMessage` support revive per role.

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

Review request for mesos, Benjamin Mahler and Jay Guo.


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


Repository: mesos


Description
-------

Enabled `ReviveOffersMessage` support revive per role.


Diffs
-----

  src/master/master.hpp ceb9bf7e4be7e0e43a166d7706bd0ab4fd9ed578 
  src/master/master.cpp 98c39b279e7b9830d02efc8ec6a4469afc15d62a 
  src/messages/messages.proto 7a2f37b78a8edcd372558f77f15e6b249742e321 

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


Testing
-------

make
make check


Thanks,

Guangya Liu


Re: Review Request 56371: Enabled `ReviveOffersMessage` support revive per role.

Posted by Guangya Liu <gy...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56371/
-----------------------------------------------------------

(Updated \u4e8c\u6708 9, 2017, 6:21 a.m.)


Review request for mesos, Benjamin Mahler and Jay Guo.


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


Repository: mesos


Description
-------

Enabled `ReviveOffersMessage` support revive per role.


Diffs (updated)
-----

  src/master/master.hpp 32915d049a2d91d648b7d33c15ef50c8bc0c72cd 
  src/master/master.cpp 0b65345d48192a1536d43973cf782ade3c1c8163 
  src/messages/messages.proto 7a2f37b78a8edcd372558f77f15e6b249742e321 

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


Testing
-------

make
make check


Thanks,

Guangya Liu


Re: Review Request 56371: Enabled `ReviveOffersMessage` support revive per role.

Posted by Guangya Liu <gy...@apache.org>.

> On \u4e8c\u6708 8, 2017, 9:20 p.m., Benjamin Mahler wrote:
> > I'm fine adding this here, but it seems weird that we wold update the protobuf but not update the scheduler driver. Are we assuming that other bindings might benefit from this? Are we still updating the old API?

Had some offline discussion with Anand, till now, the Go bindings are the only one that use the old style protobuf handlers on the master currently, but they would eventually move to use the v1 API too once James merges the changes to use the v1 API from the next branch (https://github.com/mesos/mesos-go/tree/next) to the master, so I will not update the old API, comments?


- Guangya


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


On \u4e8c\u6708 7, 2017, 10:10 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56371/
> -----------------------------------------------------------
> 
> (Updated \u4e8c\u6708 7, 2017, 10:10 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jay Guo.
> 
> 
> Bugs: MESOS-6638
>     https://issues.apache.org/jira/browse/MESOS-6638
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Enabled `ReviveOffersMessage` support revive per role.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp ceb9bf7e4be7e0e43a166d7706bd0ab4fd9ed578 
>   src/master/master.cpp 98c39b279e7b9830d02efc8ec6a4469afc15d62a 
>   src/messages/messages.proto 7a2f37b78a8edcd372558f77f15e6b249742e321 
> 
> Diff: https://reviews.apache.org/r/56371/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 56371: Enabled `ReviveOffersMessage` support revive per role.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56371/#review164771
-----------------------------------------------------------


Ship it!




I'm fine adding this here, but it seems weird that we wold update the protobuf but not update the scheduler driver. Are we assuming that other bindings might benefit from this? Are we still updating the old API?


src/messages/messages.proto (lines 269 - 271)
<https://reviews.apache.org/r/56371/#comment236530>

    How about:
    
    Removes filters for the specified role. If a role is not provided, then this this is equivalent to passing all of the framework's subscribed roles.
    
    Do you want to note that this hasn't been added to the v0 API?


- Benjamin Mahler


On Feb. 7, 2017, 10:10 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56371/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2017, 10:10 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jay Guo.
> 
> 
> Bugs: MESOS-6638
>     https://issues.apache.org/jira/browse/MESOS-6638
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Enabled `ReviveOffersMessage` support revive per role.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp ceb9bf7e4be7e0e43a166d7706bd0ab4fd9ed578 
>   src/master/master.cpp 98c39b279e7b9830d02efc8ec6a4469afc15d62a 
>   src/messages/messages.proto 7a2f37b78a8edcd372558f77f15e6b249742e321 
> 
> Diff: https://reviews.apache.org/r/56371/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>