mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 69818: Added minimal allocatable resources to framework information.
Date Tue, 29 Jan 2019 03:09:48 GMT

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



Can you add context to the commit description to clarify that this is making the existing
global flag overridable by each framework, for each of its roles?


include/mesos/mesos.proto
Lines 416-418 (patched)
<https://reviews.apache.org/r/69818/#comment298164>

    This needs to be per-role rather than globally for the framework
    
    How about `OfferFilters` to clarify that this is a mechanism for filtering offers?



include/mesos/mesos.proto
Lines 1519 (patched)
<https://reviews.apache.org/r/69818/#comment298165>

    I'm not sure if this gives much information to the reader, thoughts on the following?
    
    ```
    /**
     * Represents filters that allow a framework to control the
     * shape of offers that will be sent to its role(s). These
     * filters apply globally to any agent (unlike the existing
     * `DECLINE` filter, which is a time based resource subset
     * filter that only applies to the agent that was declined).
     */
    ```



include/mesos/mesos.proto
Lines 1522 (patched)
<https://reviews.apache.org/r/69818/#comment298166>

    This comment doesn't seem to describe a quantity of resources? Can we just remove it?



include/mesos/mesos.proto
Lines 1524 (patched)
<https://reviews.apache.org/r/69818/#comment298167>

    Can you reconcile this with the code? i.e. this should use Value::Scalar instead of double
    
    s/quantity/quantities/



include/mesos/mesos.proto
Lines 1527-1528 (patched)
<https://reviews.apache.org/r/69818/#comment298168>

    Can you put this on the variable instead of the message? Seems worth reconciling this
comment with the flag help string?


- Benjamin Mahler


On Jan. 23, 2019, 12:17 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69818/
> -----------------------------------------------------------
> 
> (Updated Jan. 23, 2019, 12:17 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
>     https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds minimal allocatable resources to a framework's
> `FrameworkInfo`. We introduce a dedicated type to model allocatable
> resources which is composed of types loosly based on the internal
> `ResourceQuantity` class.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d 
>   include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb 
>   src/common/resource_quantities.hpp 11eb426104577bbbbb7977c2307df3e4917085cd 
>   src/common/resource_quantities.cpp 320983929cd7d14973c4b98d6ed5338de690ff5f 
> 
> 
> Diff: https://reviews.apache.org/r/69818/diff/1/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


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