mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Benjamin Hindman" <b...@berkeley.edu>
Subject Re: Review Request 37358: Maintenance Primitives: Adds an endpoint for transitioning agents into the Deactivated maintenance mode.
Date Fri, 28 Aug 2015 05:02:41 GMT

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

Ship it!



src/master/http.cpp (line 1477)
<https://reviews.apache.org/r/37358/#comment152511>

    Need to s/Deactivated/DOWN/ mode here and elsewhere please.



src/master/http.cpp (line 1503)
<https://reviews.apache.org/r/37358/#comment152512>

    See previous review comments.



src/master/http.cpp (line 1514)
<https://reviews.apache.org/r/37358/#comment152513>

    Why 'DebugString' instead of JSON representation? I might have missed this on previous
reviews as well ... the advantage of the JSON representation is that folks can map back what
they submitted to what's wrong. If we print out in this format that won't be as obvious.
    
    Here and below in this entire review as well as elsewhere we should clean this up please,
thanks!



src/master/http.cpp (line 1527)
<https://reviews.apache.org/r/37358/#comment152514>

    s/operationResult/result/ here and other reviews please.



src/master/maintenance.cpp (lines 237 - 239)
<https://reviews.apache.org/r/37358/#comment152515>

    It seems weird to make this an error ... why is it a prerequisite that validation of a
MachineInfos requires more than one machine in the list? Is this somehow tied to your expectation
in the code?



src/tests/registrar_tests.cpp (line 532)
<https://reviews.apache.org/r/37358/#comment152517>

    s/Schdule/Schedule/


- Benjamin Hindman


On Aug. 26, 2015, 9:59 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37358/
> -----------------------------------------------------------
> 
> (Updated Aug. 26, 2015, 9:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van
Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2067 and MESOS-3069
>     https://issues.apache.org/jira/browse/MESOS-2067
>     https://issues.apache.org/jira/browse/MESOS-3069
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Endpoint: /maintenance.start
> 
> Registry operation = maintenance::StartMaintenance
>   Sets the list of machines as Deactivated.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 
>   src/master/maintenance.hpp PRE-CREATION 
>   src/master/maintenance.cpp PRE-CREATION 
>   src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/master_maintenance_tests.cpp PRE-CREATION 
>   src/tests/registrar_tests.cpp 032e644ee19751b4ce5767d46f474d34ec4b9166 
> 
> Diff: https://reviews.apache.org/r/37358/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> New Tests:
>   RegistrarTest.StartMaintenance
>     Schedules some machines.  Deactivates some.
>   MasterMaintenanceTest.StartMaintenance
>     Tests some invalid lists.
>     Schedules some machines.  Tests some valid and invalid lists.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


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