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 55381: Added test for framework upgrading to multi-role capability.
Date Thu, 19 Jan 2017 22:45:14 GMT

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


Fix it, then Ship it!




Thanks, looks good as far as validation is concerned. Seems we'll need a more comprehensive
upgrade test to ensure a framework with running tasks can do the upgrade and continue to function
normally, as well as launch new tasks in the new model.

Just some minor comments below, I'll make the updates and get this committed.


src/tests/master_validation_tests.cpp (lines 2589 - 2590)
<https://reviews.apache.org/r/55381/#comment233698>

    Both empty would signify a change of roles since an empty `roles` indicates no roles rather
than the default role of `"*"`.
    
    This test appears to be covering the case where they are both set to the same non-default
value (so I'll update the comment), do you want to add another test that covers the `"*`"
case or is the implementation agnostic to this case?



src/tests/master_validation_tests.cpp (line 2602)
<https://reviews.apache.org/r/55381/#comment233699>

    How about using Duration here? e.g.
    
    ```
      frameworkInfo.set_failover_timeout(Weeks(1).secs());
    ```



src/tests/master_validation_tests.cpp (lines 2618 - 2619)
<https://reviews.apache.org/r/55381/#comment233700>

    How about a `driver.stop(failover=true); driver.join()` here to be more explicit?



src/tests/master_validation_tests.cpp (lines 2642 - 2643)
<https://reviews.apache.org/r/55381/#comment233701>

    How about a stop and join here to be consistent with other tests that register a scheduler
driver above?


- Benjamin Mahler


On Jan. 18, 2017, 3:26 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55381/
> -----------------------------------------------------------
> 
> (Updated Jan. 18, 2017, 3:26 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jay Guo, and Guangya Liu.
> 
> 
> Bugs: MESOS-6900
>     https://issues.apache.org/jira/browse/MESOS-6900
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/tests/master_validation_tests.cpp c092362152e1fe8a6b615c2eda171d852c1bbd86 
> 
> Diff: https://reviews.apache.org/r/55381/diff/
> 
> 
> Testing
> -------
> 
> make check (OS X)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


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