mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vinod Kone" <vinodk...@gmail.com>
Subject Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.
Date Thu, 27 Aug 2015 20:58:13 GMT


> On Aug. 27, 2015, 6:04 a.m., Alexander Rukletsov wrote:
> > include/mesos/maintenance/maintenance.proto, line 56
> > <https://reviews.apache.org/r/36571/diff/15/?file=1053335#file1053335line56>
> >
> >     I'm not sure `0` is a valid protobuf tag.
> 
> Joseph Wu wrote:
>     After double checking the docs... It is valid for enums: https://developers.google.com/protocol-buffers/docs/proto#enum
(But not valid for non-enums.)
>     
>     For example: https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L434

I would strongly recommend to use a start value of 1 instead of 0 because it's hard to tell
the difference between an enum being unset to enum being set to the first value. This is especially
true if the enum is going to be used as optional. Looks like it is used as a required field
for now, but who knows.


- Vinod


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


On Aug. 27, 2015, 7:33 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 7:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van
Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> * MachineInfo - Describes a single box that holds one or more agents.
> * MachineInfos - A list of boxes.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining,
Deactivated.
> * Registry::MaintenanceStatus - Holds the maintenance mode of a machine.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 715b8cf38e1e56c18a3f2ddbb82c920bd9414f05 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


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