You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2015/08/28 20:49:28 UTC

Review Request 37900: Maintenance Primitives: Add helper functions for the maintenance protobufs.

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

Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.


Repository: mesos


Description
-------

This includes helpers for constructing (used in tests) and for validation (used in HTTP endpoints).


Diffs
-----

  src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
  src/common/protobuf_utils.hpp 63eeb77da5e6020085b9927ce15338aa6ed00488 
  src/common/protobuf_utils.cpp 6b283558b5ef5c16f1d9f3dc2639df4f75c92f7d 
  src/master/maintenance.hpp PRE-CREATION 
  src/master/maintenance.cpp PRE-CREATION 
  src/master/registrar.hpp c6a0655c212646618d93c9c85918af482a9ffd50 

Diff: https://reviews.apache.org/r/37900/diff/


Testing
-------

`make check`


Thanks,

Joseph Wu


Re: Review Request 37900: Maintenance Primitives: Add helper functions for the maintenance protobufs.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37900/#review96912
-----------------------------------------------------------

Ship it!



src/master/maintenance.hpp (line 48)
<https://reviews.apache.org/r/37900/#comment152599>

    s/agenda/schedule/
    
    Here AND below.



src/master/maintenance.cpp (line 78)
<https://reviews.apache.org/r/37900/#comment152601>

    s/DEACTIVATED/DOWN/



src/master/maintenance.cpp (line 90)
<https://reviews.apache.org/r/37900/#comment152600>

    s/interval/unavailability/



src/master/maintenance.cpp (line 94)
<https://reviews.apache.org/r/37900/#comment152602>

    Duration::zero() exists, so you can kill this.



src/master/maintenance.cpp (line 95)
<https://reviews.apache.org/r/37900/#comment152605>

    It seems weird to not let start be something before the epoch ... but we should still validate that the duration is greater than zero (because IIUC there isn't code that assumes backwards in time maintenance).



src/master/maintenance.cpp (line 99)
<https://reviews.apache.org/r/37900/#comment152603>

    if (start < Duration::zero() || duration < Duration::zero()) {



src/master/maintenance.cpp (line 100)
<https://reviews.apache.org/r/37900/#comment152604>

    'start' and 'duration'



src/master/maintenance.cpp (line 117)
<https://reviews.apache.org/r/37900/#comment152606>

    Let's do a sweep and clean up all of these comments that no longer apply please!



src/master/maintenance.cpp (line 140)
<https://reviews.apache.org/r/37900/#comment152607>

    Single quotes not double here.


- Benjamin Hindman


On Aug. 28, 2015, 7:15 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37900/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2015, 7:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This includes helpers for constructing (used in tests) and for validation (used in HTTP endpoints).
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/common/protobuf_utils.hpp 63eeb77da5e6020085b9927ce15338aa6ed00488 
>   src/common/protobuf_utils.cpp 6b283558b5ef5c16f1d9f3dc2639df4f75c92f7d 
>   src/master/maintenance.hpp PRE-CREATION 
>   src/master/maintenance.cpp PRE-CREATION 
>   src/master/registrar.hpp c6a0655c212646618d93c9c85918af482a9ffd50 
> 
> Diff: https://reviews.apache.org/r/37900/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 37900: Maintenance Primitives: Add helper functions for the maintenance protobufs.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37900/#review96913
-----------------------------------------------------------


Patch looks great!

Reviews applied: [37655, 36321, 36571, 37314, 37900]

All tests passed.

- Mesos ReviewBot


On Aug. 28, 2015, 7:15 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37900/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2015, 7:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This includes helpers for constructing (used in tests) and for validation (used in HTTP endpoints).
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/common/protobuf_utils.hpp 63eeb77da5e6020085b9927ce15338aa6ed00488 
>   src/common/protobuf_utils.cpp 6b283558b5ef5c16f1d9f3dc2639df4f75c92f7d 
>   src/master/maintenance.hpp PRE-CREATION 
>   src/master/maintenance.cpp PRE-CREATION 
>   src/master/registrar.hpp c6a0655c212646618d93c9c85918af482a9ffd50 
> 
> Diff: https://reviews.apache.org/r/37900/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 37900: Maintenance Primitives: Add helper functions for the maintenance protobufs.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37900/
-----------------------------------------------------------

(Updated Aug. 28, 2015, 2:07 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Addressed issued raised by comments.


Repository: mesos


Description
-------

This includes helpers for constructing (used in tests) and for validation (used in HTTP endpoints).


Diffs (updated)
-----

  src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
  src/common/protobuf_utils.hpp 63eeb77da5e6020085b9927ce15338aa6ed00488 
  src/common/protobuf_utils.cpp 6b283558b5ef5c16f1d9f3dc2639df4f75c92f7d 
  src/master/maintenance.hpp PRE-CREATION 
  src/master/maintenance.cpp PRE-CREATION 
  src/master/registrar.hpp c6a0655c212646618d93c9c85918af482a9ffd50 

Diff: https://reviews.apache.org/r/37900/diff/


Testing
-------

`make check`


Thanks,

Joseph Wu


Re: Review Request 37900: Maintenance Primitives: Add helper functions for the maintenance protobufs.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37900/
-----------------------------------------------------------

(Updated Aug. 28, 2015, 12:15 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Add missing default argument for creating unavailability.


Repository: mesos


Description
-------

This includes helpers for constructing (used in tests) and for validation (used in HTTP endpoints).


Diffs (updated)
-----

  src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
  src/common/protobuf_utils.hpp 63eeb77da5e6020085b9927ce15338aa6ed00488 
  src/common/protobuf_utils.cpp 6b283558b5ef5c16f1d9f3dc2639df4f75c92f7d 
  src/master/maintenance.hpp PRE-CREATION 
  src/master/maintenance.cpp PRE-CREATION 
  src/master/registrar.hpp c6a0655c212646618d93c9c85918af482a9ffd50 

Diff: https://reviews.apache.org/r/37900/diff/


Testing
-------

`make check`


Thanks,

Joseph Wu