Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2016-01-03 Thread Adam B

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

Ship it!


Looks good. I'm cleaning up the comments before I commit.


include/mesos/mesos.proto (lines 1557 - 1560)


Re-wrap.
I also removed some fluff so this fits on 3 lines: "This field represents", 
"potentially", and s/for instance,/e.g./
```
// The backend port on which the task is running. This could be different 
than
// the `number` field (1), e.g. when an agent achieves network isolation
// between containers using port (layer 4) segregation.
```



include/mesos/mesos.proto (lines 1577 - 1579)


TODOs don't use a space before the `(userid)`.
Let's add the missing field as a comment, so we can easily see what we're 
omitting.
Also reworded to emphasize the skipped tag `1` rather than ip_address using 
tag `2`, and binary compatibility over backwards compatibility:
```
// TODO(asridharan): We skip tag number 1 in case we later want to be binary
// compatible with `NetworkInfo.IPAddress`. See MESOS-4243 for more details.
// optional Protocol protocol = 1;
```


- Adam B


On Dec. 28, 2015, 5:10 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 28, 2015, 5:10 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
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-28 Thread Avinash sridharan


> On Dec. 24, 2015, 11:10 a.m., Adam B wrote:
> > 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.
> 
> Anand Mazumdar wrote:
> 1. Adam, can you elaborate a bit more on why do you think the `Protocol` 
> field is a misnomer here ? Are you alluding for that field to be called 
> `version` or something else to suit it better i.e. something like:
> 
> ```
> enum Version {
>   IPV4 = 1;
>   IPV6 = 2;
> }
> ```
> 
> 2. Also, we shouldn't worry too much about how the network isolator deals 
> with the `Protocol` field being set/unset, since that is relevant business 
> logic limited to its functionality and should not influence how the 
> representation of `IPAddress` should look like. We should only be concerned 
> about wire compatibility if we intend to migrate `NetworkInfo` to use this 
> message later. Were you referring to the fact that just renaming `Protocol` 
> to `Version` or something else would make it wire incompatible with the old 
> message inside `NetworkInfo` and hence we should not do it now ?
> 
> PS: I am fine with not introducing the `Protocol` field for now like you 
> suggested. I just wanted to be sure about the reasoning/problems with not 
> doing it now and leaving it off for later.

Will remove the Protocol field for the time being and set the position 
identifier for the ip_address field to 2.


- Avinash


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


On Dec. 23, 2015, 3 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 23, 2015, 3 a.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
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-28 Thread Avinash sridharan

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

(Updated Dec. 29, 2015, 1:10 a.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 (updated)
-

  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



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-24 Thread Anand Mazumdar


> On Dec. 24, 2015, 11:10 a.m., Adam B wrote:
> > 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.

1. Adam, can you elaborate a bit more on why do you think the `Protocol` field 
is a misnomer here ? Are you alluding for that field to be called `version` or 
something else to suit it better i.e. something like:

```
enum Version {
  IPV4 = 1;
  IPV6 = 2;
}
```

2. Also, we shouldn't worry too much about how the network isolator deals with 
the `Protocol` field being set/unset, since that is relevant business logic 
limited to its functionality and should not influence how the representation of 
`IPAddress` should look like. We should only be concerned about wire 
compatibility if we intend to migrate `NetworkInfo` to use this message later. 
Were you referring to the fact that just renaming `Protocol` to `Version` or 
something else would make it wire incompatible with the old message inside 
`NetworkInfo` and hence we should not do it now ?

PS: I am fine with not introducing the `Protocol` field for now like you 
suggested. I just wanted to be sure about the reasoning/problems with not doing 
it now and leaving it off for later.


- Anand


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


On Dec. 23, 2015, 3 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 23, 2015, 3 a.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
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-24 Thread Adam B

---
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)


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)


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
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-22 Thread Avinash sridharan

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

(Updated Dec. 22, 2015, 10:03 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 (updated)
-

  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



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-22 Thread Avinash sridharan

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

(Updated Dec. 23, 2015, 2:09 a.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 (updated)
-

  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



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-22 Thread Avinash sridharan

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

(Updated Dec. 23, 2015, 2:05 a.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



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-22 Thread Avinash sridharan

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

(Updated Dec. 23, 2015, 3 a.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



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-22 Thread Avinash sridharan


> On Dec. 19, 2015, 10:01 a.m., Adam B wrote:
> > include/mesos/mesos.proto, lines 1576-1577
> > 
> >
> > Do we expect to add more fields to the Vip message later?
> > If not, why don't we just add a `repeated string vips` to DiscoveryInfo?
> > If so, what do you think we'd add? Maybe a service name, in case the 
> > same task has multiple services exposed on different vips?
> 
> Anand Mazumdar wrote:
> hmm ... Since a VIP is just a virtual IP address. Why not use the 
> existing `Address` message and just add the following to `DiscoveryInfo`:
> 
> `repeated Address vips;`
> 
> Avinash sridharan wrote:
> This is the Address message :message Address {
>   // May contain a hostname, IP address, or both.
>   optional string hostname = 1;
>   optional string ip = 2;
> 
>   required int32 port = 3;
> }
> 
> The problem with using this is that port is a required field in the 
> Address type (a bit odd). In case of virtual IP's they need to be explicitly 
> IP and port is mapping that is provided separately (message Port). So don't 
> think will make sense to use it in this context ?
> 
> Avinash sridharan wrote:
> I am just thinking that given this is a virtual endpoint there might be 
> other metadata (labels is what comes to mind right now) that might be 
> associated with this in the future.  Similar to the message Port. As you 
> pointed out named Vips is another option.

Have added a new message type IPAddress which is in the top-level namespace. 
Also, after talking to Anand have kept the enums in the IPAddress field as 
protocol, since renaming the field will cause another set of churn that is 
better to avoid at this stage.


- Avinash


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


On Dec. 22, 2015, 10:03 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, 10:03 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
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-22 Thread Avinash sridharan

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

(Updated Dec. 22, 2015, 11:47 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 (updated)
-

  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



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-19 Thread Anand Mazumdar


> On Dec. 19, 2015, 10:01 a.m., Adam B wrote:
> > include/mesos/mesos.proto, lines 1576-1577
> > 
> >
> > Do we expect to add more fields to the Vip message later?
> > If not, why don't we just add a `repeated string vips` to DiscoveryInfo?
> > If so, what do you think we'd add? Maybe a service name, in case the 
> > same task has multiple services exposed on different vips?

hmm ... Since a VIP is just a virtual IP address. Why not use the existing 
`Address` message and just add the following to `DiscoveryInfo`:

`repeated Address vips;`


- Anand


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


On Dec. 19, 2015, 7:16 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 19, 2015, 7:16 a.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
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-19 Thread Avinash sridharan


> On Dec. 19, 2015, 10:01 a.m., Adam B wrote:
> > include/mesos/mesos.proto, lines 1576-1577
> > 
> >
> > Do we expect to add more fields to the Vip message later?
> > If not, why don't we just add a `repeated string vips` to DiscoveryInfo?
> > If so, what do you think we'd add? Maybe a service name, in case the 
> > same task has multiple services exposed on different vips?
> 
> Anand Mazumdar wrote:
> hmm ... Since a VIP is just a virtual IP address. Why not use the 
> existing `Address` message and just add the following to `DiscoveryInfo`:
> 
> `repeated Address vips;`

This is the Address message :message Address {
  // May contain a hostname, IP address, or both.
  optional string hostname = 1;
  optional string ip = 2;

  required int32 port = 3;
}

The problem with using this is that port is a required field in the Address 
type (a bit odd). In case of virtual IP's they need to be explicitly IP and 
port is mapping that is provided separately (message Port). So don't think will 
make sense to use it in this context ?


- Avinash


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


On Dec. 19, 2015, 7:16 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 19, 2015, 7:16 a.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
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-19 Thread Avinash sridharan


> On Dec. 19, 2015, 10:01 a.m., Adam B wrote:
> > include/mesos/mesos.proto, lines 1576-1577
> > 
> >
> > Do we expect to add more fields to the Vip message later?
> > If not, why don't we just add a `repeated string vips` to DiscoveryInfo?
> > If so, what do you think we'd add? Maybe a service name, in case the 
> > same task has multiple services exposed on different vips?
> 
> Anand Mazumdar wrote:
> hmm ... Since a VIP is just a virtual IP address. Why not use the 
> existing `Address` message and just add the following to `DiscoveryInfo`:
> 
> `repeated Address vips;`
> 
> Avinash sridharan wrote:
> This is the Address message :message Address {
>   // May contain a hostname, IP address, or both.
>   optional string hostname = 1;
>   optional string ip = 2;
> 
>   required int32 port = 3;
> }
> 
> The problem with using this is that port is a required field in the 
> Address type (a bit odd). In case of virtual IP's they need to be explicitly 
> IP and port is mapping that is provided separately (message Port). So don't 
> think will make sense to use it in this context ?

I am just thinking that given this is a virtual endpoint there might be other 
metadata (labels is what comes to mind right now) that might be associated with 
this in the future.  Similar to the message Port. As you pointed out named Vips 
is another option.


- Avinash


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


On Dec. 19, 2015, 7:16 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 19, 2015, 7:16 a.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
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-19 Thread Avinash sridharan

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

(Updated Dec. 20, 2015, 1:51 a.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 (updated)
-

  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



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-19 Thread Avinash sridharan


> On Dec. 18, 2015, 10:54 p.m., Artem Harutyunyan wrote:
> >
> 
> Artem Harutyunyan wrote:
> Commit messages should usually end with a period.

Fixed.


> On Dec. 18, 2015, 10:54 p.m., Artem Harutyunyan wrote:
> > include/mesos/mesos.proto, line 1579
> > 
> >
> > s/then/than/?

Thanks. Lot of confusion betweent then and than !! You are right comparison so 
than !!


> On Dec. 18, 2015, 10:54 p.m., Artem Harutyunyan wrote:
> > include/mesos/mesos.proto, line 1603
> > 
> >
> > Please add a full stop after comment.

Removed the message type Vips as per Adam's suggestion.


> On Dec. 18, 2015, 10:54 p.m., Artem Harutyunyan wrote:
> > include/mesos/mesos.proto, line 1597
> > 
> >
> > Why is it a Vip and not VIP, or VIp, or maybe even virtual_ip to be 
> > consistent with the style?
> 
> Adam B wrote:
> Yikes. `VIp` is the worst of those options.
> I would go with `Vip` according to the Google style guide:
> "Prefer to capitalize acronyms as single words (i.e. StartRpc(), not 
> StartRPC())." (although that applies to C++ function names)
> Or `VIP`, since we already have an `ACL` (and `ACLs`).

Oh oh should have been more stubborn sticking with Vip !! I thought VIp is also 
conforming to the coding style. Will go with Vip


- Avinash


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


On Dec. 19, 2015, 7:16 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 19, 2015, 7:16 a.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
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-19 Thread Adam B


> On Dec. 18, 2015, 2:54 p.m., Artem Harutyunyan wrote:
> > include/mesos/mesos.proto, line 1597
> > 
> >
> > Why is it a Vip and not VIP, or VIp, or maybe even virtual_ip to be 
> > consistent with the style?

Yikes. `VIp` is the worst of those options.
I would go with `Vip` according to the Google style guide:
"Prefer to capitalize acronyms as single words (i.e. StartRpc(), not 
StartRPC())." (although that applies to C++ function names)
Or `VIP`, since we already have an `ACL` (and `ACLs`).


- Adam


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


On Dec. 18, 2015, 11:16 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 18, 2015, 11:16 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
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-19 Thread Adam B

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


Please rename `Vip` back, or go with `VIP`. Or get rid of that message 
altogether if it makes sense.


include/mesos/mesos.proto (lines 1576 - 1577)


Do we expect to add more fields to the Vip message later?
If not, why don't we just add a `repeated string vips` to DiscoveryInfo?
If so, what do you think we'd add? Maybe a service name, in case the same 
task has multiple services exposed on different vips?


- Adam B


On Dec. 18, 2015, 11:16 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 18, 2015, 11:16 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
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-18 Thread Artem Harutyunyan

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



include/mesos/mesos.proto (line 1579)


s/then/than/?



include/mesos/mesos.proto (line 1597)


Why is it a Vip and not VIP, or VIp, or maybe even virtual_ip to be 
consistent with the style?



include/mesos/mesos.proto (line 1603)


Please add a full stop after comment.



include/mesos/mesos.proto (line 1638)


s/used/is used/?



include/mesos/v1/mesos.proto (line 1578)


s/then/than/?



include/mesos/v1/mesos.proto (line 1602)


s/Vip/virtual IPs./

to be consistent with the comment above. And please don't forget the point 
at the end.



include/mesos/v1/mesos.proto (line 1637)


s/IP used/IPs are used/?


- Artem Harutyunyan


On Dec. 17, 2015, 2:41 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 17, 2015, 2:41 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 8ca213062c480f0266ffc51a621eb4a118140c77 
>   include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 
> 
> Diff: https://reviews.apache.org/r/41380/diff/
> 
> 
> Testing
> ---
> 
> make check, and make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-18 Thread Artem Harutyunyan


> On Dec. 18, 2015, 2:54 p.m., Artem Harutyunyan wrote:
> >

Commit messages should usually end with a period.


- Artem


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


On Dec. 17, 2015, 2:41 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 17, 2015, 2:41 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 8ca213062c480f0266ffc51a621eb4a118140c77 
>   include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 
> 
> Diff: https://reviews.apache.org/r/41380/diff/
> 
> 
> Testing
> ---
> 
> make check, and make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-18 Thread Artem Harutyunyan


> On Dec. 17, 2015, 5:40 p.m., Adam B wrote:
> > Let's get rid of the Vips message per MPark's recommendation, since we're 
> > unlikely to ever add any more fields to that message.

Could you please also clean the `Depends On` field for this review?


- Artem


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


On Dec. 17, 2015, 2:41 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 17, 2015, 2:41 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 8ca213062c480f0266ffc51a621eb4a118140c77 
>   include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 
> 
> Diff: https://reviews.apache.org/r/41380/diff/
> 
> 
> Testing
> ---
> 
> make check, and make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-18 Thread Avinash sridharan

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

(Updated Dec. 19, 2015, 7:05 a.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 (updated)
-

  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



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-18 Thread Avinash sridharan

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

(Updated Dec. 19, 2015, 7:16 a.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 (updated)
-

  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



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-17 Thread Kevin Klues

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


This review depends on 41189, but 41189 is somehow marked as "Invite-only", so 
it is not viewable by the public.  This is causing the jenkins review-bot to 
fail with:

Verifying review 41381
Dependent review: https://reviews.apache.org/api/review-requests/41380/
Dependent review: https://reviews.apache.org/api/review-requests/41189/
Error handling URL https://reviews.apache.org/api/review-requests/41189/: 
FORBIDDEN

>From my reading of 
>https://www.reviewboard.org/docs/manual/2.5/admin/configuration/access-control/
My guess is all you need to do is make sure that 41189 has the 'mesos' group 
added to it. If no group is there, it will likely be treated as "Invite-only" 
since no public groups are accessible.

- Kevin Klues


On Dec. 17, 2015, 3:31 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 17, 2015, 3:31 a.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 8ca213062c480f0266ffc51a621eb4a118140c77 
>   include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 
> 
> Diff: https://reviews.apache.org/r/41380/diff/
> 
> 
> Testing
> ---
> 
> make check, and make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-17 Thread Avinash sridharan

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

(Updated Dec. 17, 2015, 9:33 a.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 8ca213062c480f0266ffc51a621eb4a118140c77 
  include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 

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


Testing
---

make check, and make


Thanks,

Avinash sridharan



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-17 Thread Avinash sridharan


> On Dec. 17, 2015, 8:37 a.m., Kevin Klues wrote:
> > This review depends on 41189, but 41189 is somehow marked as "Invite-only", 
> > so it is not viewable by the public.  This is causing the jenkins 
> > review-bot to fail with:
> > 
> > Verifying review 41381
> > Dependent review: https://reviews.apache.org/api/review-requests/41380/
> > Dependent review: https://reviews.apache.org/api/review-requests/41189/
> > Error handling URL https://reviews.apache.org/api/review-requests/41189/: 
> > FORBIDDEN
> > 
> > From my reading of 
> > https://www.reviewboard.org/docs/manual/2.5/admin/configuration/access-control/
> > My guess is all you need to do is make sure that 41189 has the 'mesos' 
> > group added to it. If no group is there, it will likely be treated as 
> > "Invite-only" since no public groups are accessible.

Thanks Kevin,
 Had discarded 41189. Since the changes were on the same branch the dependency 
got created but persisted after discarding 41189. Have updated the dependency 
to 41187


- Avinash


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


On Dec. 17, 2015, 9:33 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 17, 2015, 9:33 a.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 8ca213062c480f0266ffc51a621eb4a118140c77 
>   include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 
> 
> Diff: https://reviews.apache.org/r/41380/diff/
> 
> 
> Testing
> ---
> 
> make check, and make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-17 Thread Avinash sridharan

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

(Updated Dec. 17, 2015, 10:41 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 (updated)
-

  include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
  include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 

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


Testing
---

make check, and make


Thanks,

Avinash sridharan



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-17 Thread Adam B

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


Please explain (the intended use of) instance_port better. Otherwise shippable.


include/mesos/mesos.proto (line 1577)


When/why/how would it be different from the port number? Please give a 
clearer example.

If it would help the explanation to have `number` and `instance_port` next 
to each other, you can put `instance_port` just below `number`. The ordering in 
the protobuf definition doesn't matter, since it's the `= 6` fieldId that 
determines order over the wire.



include/mesos/mesos.proto (lines 1615 - 1616)


Please add this comment above the vips field, rather than bloating the 
message-level comment further. Comments that only talk about a single field 
don't have a reason to be at the top-level.



include/mesos/mesos.proto (lines 1617 - 1618)


Fix wrapping


- Adam B


On Dec. 17, 2015, 1:33 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 17, 2015, 1:33 a.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 8ca213062c480f0266ffc51a621eb4a118140c77 
>   include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 
> 
> Diff: https://reviews.apache.org/r/41380/diff/
> 
> 
> Testing
> ---
> 
> make check, and make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-17 Thread Adam B

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


Let's get rid of the Vips message per MPark's recommendation, since we're 
unlikely to ever add any more fields to that message.


include/mesos/mesos.proto (line 1581)


s/seggregation/segregation/



include/mesos/mesos.proto (lines 1602 - 1607)


Latest recommendation from MPark is to get rid of the Vips message and just 
use `repeated Vip vips` everywhere. Then we won't have nested `vips.vips` to 
jsonify.



include/mesos/mesos.proto (line 1638)


s/end-point/endpoint/?



include/mesos/mesos.proto (line 1640)


`repeated Vip vips = 8;`


- Adam B


On Dec. 17, 2015, 2:41 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 17, 2015, 2:41 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 8ca213062c480f0266ffc51a621eb4a118140c77 
>   include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 
> 
> Diff: https://reviews.apache.org/r/41380/diff/
> 
> 
> Testing
> ---
> 
> make check, and make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-16 Thread Avinash sridharan

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

(Updated Dec. 17, 2015, 3:31 a.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 (updated)
-

  include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
  include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 

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


Testing
---

make check, and make


Thanks,

Avinash sridharan



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-16 Thread Anand Mazumdar

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


Looks like this review has overlapping changes with 
https://reviews.apache.org/r/41187 around comments for fields in `Ports` 
message. Can you fix it ?

- Anand Mazumdar


On Dec. 16, 2015, 12:28 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 16, 2015, 12:28 a.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 8ca213062c480f0266ffc51a621eb4a118140c77 
>   include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 
> 
> Diff: https://reviews.apache.org/r/41380/diff/
> 
> 
> Testing
> ---
> 
> make check, and make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-15 Thread Avinash sridharan

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

(Updated Dec. 15, 2015, 4:35 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 (updated)
-

  include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
  include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 

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


Testing
---

make check, and make


Thanks,

Avinash sridharan



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-15 Thread Adam B

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


Lookin good


include/mesos/mesos.proto (line 1569)


Comment on its intended use, please.



include/mesos/mesos.proto (line 1609)


Any chance that we'll want a non-string vip representation in the future? 
Maybe a more structured format, something for which we would prefer to make 
`message Vip` an alias of 'string'?
Also, an intended usage comment would be appropriate here as well.


- Adam B


On Dec. 14, 2015, 7:35 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 14, 2015, 7:35 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 8ca213062c480f0266ffc51a621eb4a118140c77 
>   include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 
> 
> Diff: https://reviews.apache.org/r/41380/diff/
> 
> 
> Testing
> ---
> 
> make check, and make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-15 Thread Avinash sridharan

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

(Updated Dec. 15, 2015, 9:13 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 (updated)
-

  include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
  include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 

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


Testing
---

make check, and make


Thanks,

Avinash sridharan



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-15 Thread Avinash sridharan

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

(Updated Dec. 16, 2015, 12:28 a.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 (updated)
-

  include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
  include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 

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


Testing
---

make check, and make


Thanks,

Avinash sridharan



Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-14 Thread Avinash sridharan

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

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 8ca213062c480f0266ffc51a621eb4a118140c77 
  include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 

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


Testing
---

make check, and make


Thanks,

Avinash sridharan