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



src/common/http.hpp
<https://reviews.apache.org/r/18144/#comment65517>

    When are you planning to do this TODO Dominic?
    
    We should avoid making API changes that are short-term, in this case 
exposing "role_resources" temporarily seems like a bad idea. Why even expose 
"role_resources" at all in this patch?
    
    What about the following for this patch:
    1. Kill perRoleModel and "role_resources", since it's not being used and we 
plan to kill it anyway.
    2. Your fix for model(Resource&) can remain since it fixes MESOS-692 by 
summing resources.
    
    And a follow-up patch:
    1. Exposes roles in model(Resources&) by swapping it with your perRoleModel 
implementation.
    2. Updates the webui to handle the new format.


- Ben Mahler


On Feb. 20, 2014, 11:04 p.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18144/
> -----------------------------------------------------------
> 
> (Updated Feb. 20, 2014, 11:04 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-692 and MESOS-990
>     https://issues.apache.org/jira/browse/MESOS-692
>     https://issues.apache.org/jira/browse/MESOS-990
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am e02d416e5cb8edf6c44ed05c5785a5084bf02abb 
>   src/common/http.hpp PRE-CREATION 
>   src/common/http.cpp PRE-CREATION 
>   src/master/http.cpp c9412749baeb88e8fc270eeca8710f04da2ac076 
>   src/slave/http.cpp 7c4cfba6676124926cfaa665df9fccf68c7dc187 
> 
> Diff: https://reviews.apache.org/r/18144/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> ran master/slave and checked http://localhost:5050/master/state.json by eye.
> 
> ran local/python test framework and watched webui main, framework, and slave 
> pages.
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>

Reply via email to