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


Looks good, but I wonder if we need to go so far as to introduce the `enum 
Protocol` misnomer in the global IPAddress message now. We could always add it 
in later, when we actually get NetworkInfo off of it.


include/mesos/mesos.proto (line 1577)
<https://reviews.apache.org/r/41380/#comment172149>

    Since an enum is essentially a uint32, I would think you could actually 
rename the enum or its value-names, as long as the numeric values stay the 
same. Unfortunately, I couldn't find verification in 
https://developers.google.com/protocol-buffers/docs/proto#updating



include/mesos/mesos.proto (lines 1582 - 1583)
<https://reviews.apache.org/r/41380/#comment172143>

    Interestingly, in NetworkInfo.IPAddress, only one of protocol or ip_address 
is expected to be set, since setting the protocol field actually requests the 
network isolator to assign an IP to the container being launched.
    Your use in DiscoveryInfo is rather different.
    I'd almost lean toward not putting Protocol in this IPAddress. You could 
leave field 1 out and set `ip_address = 2` still, if you want to migrate 
NetworkInfo over to this message later.


- Adam B


On Dec. 22, 2015, 7 p.m., Avinash sridharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> -----------------------------------------------------------
> 
> (Updated Dec. 22, 2015, 7 p.m.)
> 
> 
> Review request for mesos, Adam B and Anand Mazumdar.
> 
> 
> Bugs: MESOS-4114
>     https://issues.apache.org/jira/browse/MESOS-4114
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added repeated vip field to DiscoveryInfo and an instance_port field to Port
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 2431fdd6b84625c6140a2b3913736bffada4e7f6 
>   include/mesos/v1/mesos.proto 4aed0980b28dc1000aa2821f35303b736bc5bff8 
> 
> Diff: https://reviews.apache.org/r/41380/diff/
> 
> 
> Testing
> -------
> 
> make check, and make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Reply via email to