mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 63991: Added helpers to create and forward offer operation updates.
Date Wed, 14 Mar 2018 15:56:23 GMT

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




src/common/protobuf_utils.hpp
Lines 165-170 (patched)
<https://reviews.apache.org/r/63991/#comment279464>

    This looks unusual when compared to other helpers in this file which usually have arguments
modelled pretty closely after the fields of message they produce. The function names usually
also pretty directly reflect what kind of message they produce. This helper OTOH only can
create a single flavor of `Scheduler::Event`.
    
    Given that this function does not abstract away any logic and will be used just a single
helper added in the next patch, I'd suggest to just inline the creation of `Scheduler::Event`
there for now and not add this helper.



src/master/master.hpp
Lines 913-920 (patched)
<https://reviews.apache.org/r/63991/#comment279466>

    Do we plan to use this same helper to forward operation updates from agents and rps as
well in the future? Wouldn't we want to pass in the updates instead of exploding fields into
separate args?



src/master/master.cpp
Lines 7979-7985 (patched)
<https://reviews.apache.org/r/63991/#comment279467>

    Shouldn't we add this operation into some operation status update manager?
    
    One possible way for that would be to just accept an operation update as arg here and
deal with the distinction between master-generated and other updates elsewhere, e.g., in https://reviews.apache.org/r/63992/.


- Benjamin Bannier


On March 13, 2018, 5:33 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63991/
> -----------------------------------------------------------
> 
> (Updated March 13, 2018, 5:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
> 
> 
> Bugs: MESOS-8190
>     https://issues.apache.org/jira/browse/MESOS-8190
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added helpers to create and forward offer operation updates.
> 
> 
> Diffs
> -----
> 
>   src/common/protobuf_utils.hpp 2ef1c9a4c2986e6f254e28387635bc01bb0b3259 
>   src/common/protobuf_utils.cpp 9c5fb97afb58f98013b79f3cbaea7dacc3603271 
>   src/master/master.hpp 8bf2c763dafdb7df55c46a56f2ff66f2a951d947 
>   src/master/master.cpp 223ebf29ac4dd1dea9080e4bef4b2d4d064d847f 
> 
> 
> Diff: https://reviews.apache.org/r/63991/diff/4/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message