mesos-reviews mailing list archives

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



src/master/master.hpp (line 534)
<https://reviews.apache.org/r/35797/#comment145723>

    s/capabilities_size()/capabilities.size()/



src/master/master.hpp (line 536)
<https://reviews.apache.org/r/35797/#comment145722>

    Do you also want to deal with the case where capabilities are previously set but now being
unset? In other words, the "else" case.
    
    if (source.capabilities.size() > 0 ) {
      info.mutable_capabilities()->CopyFrom(source.capabilities);
    } else {
      info.clear_capabilities();
    }
    
    If you want do this in a follow up patch, add a TODO here. You will also need a new test
for this.



src/tests/fault_tolerance_tests.cpp (line 1730)
<https://reviews.apache.org/r/35797/#comment145724>

    Add a comment on the top of the test that you are verifying that when a framework re-registers
with updated framework info, it gets updated in the master.



src/tests/fault_tolerance_tests.cpp (lines 1742 - 1743)
<https://reviews.apache.org/r/35797/#comment145720>

    We now require a much newer gcc version, so this shouldn't be an issue. You can just do.
    
    FrameworkInfo framework1 = DEFAULT_FRAMEWORK_INFO;



src/tests/fault_tolerance_tests.cpp (lines 1769 - 1770)
<https://reviews.apache.org/r/35797/#comment145725>

    ditto. just assign on the same line.



src/tests/fault_tolerance_tests.cpp (line 1772)
<https://reviews.apache.org/r/35797/#comment145726>

    s/Name/Type/



src/tests/fault_tolerance_tests.cpp (lines 1773 - 1774)
<https://reviews.apache.org/r/35797/#comment145730>

    Can you also update webui_url, hostname and failover_timeout while you are at it?



src/tests/fault_tolerance_tests.cpp (line 1803)
<https://reviews.apache.org/r/35797/#comment145732>

    s/masterState/response/



src/tests/fault_tolerance_tests.cpp (line 1806)
<https://reviews.apache.org/r/35797/#comment145733>

    s/masterStateObject/parse/
    
    you also need a
    
    ASSERT_SOME(parse);



src/tests/fault_tolerance_tests.cpp (line 1809)
<https://reviews.apache.org/r/35797/#comment145734>

    new line after ASSERT.
    
    s/masterStateObject_/state/



src/tests/fault_tolerance_tests.cpp (line 1812)
<https://reviews.apache.org/r/35797/#comment145736>

    s/famework_/framework/
    
    new line after this.



src/tests/fault_tolerance_tests.cpp (line 1831)
<https://reviews.apache.org/r/35797/#comment145737>

    Nice test!


- Vinod Kone


On July 16, 2015, 6:46 p.m., Aditi Dixit wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35797/
> -----------------------------------------------------------
> 
> (Updated July 16, 2015, 6:46 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2880
>     https://issues.apache.org/jira/browse/MESOS-2880
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This only updates the master, not the allocator. Added test too.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec 
>   src/tests/fault_tolerance_tests.cpp 1070ccf17f98f6b3800684a5edd6517d90631c3e 
> 
> Diff: https://reviews.apache.org/r/35797/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Aditi Dixit
> 
>