Re: Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

2015-12-17 Thread Adam B 

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


Thanks! A few wording suggestions, and a clarification of 'visibility' scoping, 
but otherwise shippable.


include/mesos/mesos.proto (line 1564)


"Port number on which the framework exposes a service."



include/mesos/mesos.proto (line 1566)


"Name of the service hosted on this port."



include/mesos/mesos.proto (line 1568)


s/frameworks/framework/

And technically, you've got a dangling preposition. Grammar nazis would 
demand "protocol on which the framework exposes its services".



include/mesos/mesos.proto (lines 1570 - 1571)


The visibility setting for a Port overrides the general visibility setting 
in the DiscoveryInfo.



include/mesos/mesos.proto (line 1573)


s/tags that could be/metadata to be/ since you shouldn't decorate the 
message unless you have a consumer in mind, plus 'metadata' is more "meta".
Also, you probably don't need to quote "decorate".


- Adam B


On Dec. 16, 2015, 7:07 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41187/
> ---
> 
> (Updated Dec. 16, 2015, 7:07 p.m.)
> 
> 
> Review request for mesos, Adam B and Anand Mazumdar.
> 
> 
> Bugs: MESOS-3962
> https://issues.apache.org/jira/browse/MESOS-3962
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding labels field to Port information, for service discovery to associate 
> arbitrary tags by applications to ports for a given task.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
>   include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 
> 
> Diff: https://reviews.apache.org/r/41187/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

2015-12-17 Thread Avinash sridharan 

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

(Updated Dec. 17, 2015, 10:35 p.m.)


Review request for mesos, Adam B and Anand Mazumdar.


Bugs: MESOS-3962
https://issues.apache.org/jira/browse/MESOS-3962


Repository: mesos


Description
---

Adding labels field to Port information, for service discovery to associate 
arbitrary tags by applications to ports for a given task.


Diffs (updated)
-

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

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


Testing
---

make check


Thanks,

Avinash sridharan

Re: Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

2015-12-17 Thread Adam B 

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

Ship it!


Ship It!

- Adam B


On Dec. 17, 2015, 2:35 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41187/
> ---
> 
> (Updated Dec. 17, 2015, 2:35 p.m.)
> 
> 
> Review request for mesos, Adam B and Anand Mazumdar.
> 
> 
> Bugs: MESOS-3962
> https://issues.apache.org/jira/browse/MESOS-3962
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding labels field to Port information, for service discovery to associate 
> arbitrary tags by applications to ports for a given task.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
>   include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 
> 
> Diff: https://reviews.apache.org/r/41187/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

2015-12-16 Thread Avinash sridharan 

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

(Updated Dec. 17, 2015, 1:44 a.m.)


Review request for mesos, Adam B and Anand Mazumdar.


Bugs: MESOS-3962
https://issues.apache.org/jira/browse/MESOS-3962


Repository: mesos


Description
---

Adding labels field to Port information, for service discovery to associate 
arbitrary tags by applications to ports for a given task.


Diffs (updated)
-

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

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


Testing
---

make check


Thanks,

Avinash sridharan

Re: Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

2015-12-16 Thread Avinash sridharan 

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

(Updated Dec. 17, 2015, 3:07 a.m.)


Review request for mesos, Adam B and Anand Mazumdar.


Bugs: MESOS-3962
https://issues.apache.org/jira/browse/MESOS-3962


Repository: mesos


Description
---

Adding labels field to Port information, for service discovery to associate 
arbitrary tags by applications to ports for a given task.


Diffs (updated)
-

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

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


Testing
---

make check


Thanks,

Avinash sridharan

Re: Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

2015-12-15 Thread Avinash sridharan 

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

(Updated Dec. 15, 2015, 4:32 p.m.)


Review request for mesos, Adam B and Anand Mazumdar.


Bugs: MESOS-3962
https://issues.apache.org/jira/browse/MESOS-3962


Repository: mesos


Description
---

Adding labels field to Port information, for service discovery to associate 
arbitrary tags by applications to ports for a given task.


Diffs (updated)
-

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

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


Testing
---

make check


Thanks,

Avinash sridharan

Re: Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

2015-12-15 Thread Avinash sridharan 


> On Dec. 15, 2015, 1:01 a.m., Guangya Liu wrote:
> > include/mesos/v1/mesos.proto, line 1567
> > 
> >
> > It is always recommeded to add some comments here.

The whole Port message was missing comments so I augmented it by adding 
comments similar to DiscoveryInfo.


- Avinash


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


On Dec. 14, 2015, 11:34 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41187/
> ---
> 
> (Updated Dec. 14, 2015, 11:34 p.m.)
> 
> 
> Review request for mesos, Adam B and Anand Mazumdar.
> 
> 
> Bugs: MESOS-3962
> https://issues.apache.org/jira/browse/MESOS-3962
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding labels field to Port information, for service discovery to associate 
> arbitrary tags by applications to ports for a given task.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
>   include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 
> 
> Diff: https://reviews.apache.org/r/41187/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

2015-12-15 Thread Adam B 

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

Ship it!


Looks good, but needs a little documentation.


include/mesos/v1/mesos.proto (lines 1560 - 1567)


I agree with Guangya. Could you improve the comment for this protobuf 
message, explaining what protocol and visibility should mean here too?


- Adam B


On Dec. 14, 2015, 3:34 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41187/
> ---
> 
> (Updated Dec. 14, 2015, 3:34 p.m.)
> 
> 
> Review request for mesos, Adam B and Anand Mazumdar.
> 
> 
> Bugs: MESOS-3962
> https://issues.apache.org/jira/browse/MESOS-3962
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding labels field to Port information, for service discovery to associate 
> arbitrary tags by applications to ports for a given task.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
>   include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 
> 
> Diff: https://reviews.apache.org/r/41187/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

2015-12-15 Thread Avinash sridharan 

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

(Updated Dec. 15, 2015, 10:11 p.m.)


Review request for mesos, Adam B and Anand Mazumdar.


Bugs: MESOS-3962
https://issues.apache.org/jira/browse/MESOS-3962


Repository: mesos


Description
---

Adding labels field to Port information, for service discovery to associate 
arbitrary tags by applications to ports for a given task.


Diffs (updated)
-

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

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


Testing
---

make check


Thanks,

Avinash sridharan

Re: Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

2015-12-15 Thread Avinash sridharan 


> On Dec. 15, 2015, 11:08 a.m., Adam B wrote:
> > include/mesos/v1/mesos.proto, lines 1560-1567
> > 
> >
> > I agree with Guangya. Could you improve the comment for this protobuf 
> > message, explaining what protocol and visibility should mean here too?

Agreed. See the latest diff for comments on Port message.


- Avinash


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


On Dec. 15, 2015, 10:11 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41187/
> ---
> 
> (Updated Dec. 15, 2015, 10:11 p.m.)
> 
> 
> Review request for mesos, Adam B and Anand Mazumdar.
> 
> 
> Bugs: MESOS-3962
> https://issues.apache.org/jira/browse/MESOS-3962
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding labels field to Port information, for service discovery to associate 
> arbitrary tags by applications to ports for a given task.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
>   include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 
> 
> Diff: https://reviews.apache.org/r/41187/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

2015-12-15 Thread Avinash sridharan 


> On Dec. 15, 2015, 11:32 p.m., Guangya Liu wrote:
> > include/mesos/mesos.proto, lines 1561-1567
> > 
> >
> > Can you please split the comments to different fields just like others.
> > 
> > // Represents the port on which the framework exposes its service on.
> > required uint32 number = 1;
> > 
> > 

I was trying to follow the convention set by DiscoveryInfo. Speaking to Adam, 
looks like that's not a good example to follow. Going to move the comments per 
field.


- Avinash


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


On Dec. 15, 2015, 10:11 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41187/
> ---
> 
> (Updated Dec. 15, 2015, 10:11 p.m.)
> 
> 
> Review request for mesos, Adam B and Anand Mazumdar.
> 
> 
> Bugs: MESOS-3962
> https://issues.apache.org/jira/browse/MESOS-3962
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding labels field to Port information, for service discovery to associate 
> arbitrary tags by applications to ports for a given task.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
>   include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 
> 
> Diff: https://reviews.apache.org/r/41187/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

2015-12-15 Thread Guangya Liu 

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



include/mesos/mesos.proto (lines 1561 - 1567)


Can you please split the comments to different fields just like others.

// Represents the port on which the framework exposes its service on.
required uint32 number = 1;




- Guangya Liu


On Dec. 15, 2015, 10:11 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41187/
> ---
> 
> (Updated Dec. 15, 2015, 10:11 p.m.)
> 
> 
> Review request for mesos, Adam B and Anand Mazumdar.
> 
> 
> Bugs: MESOS-3962
> https://issues.apache.org/jira/browse/MESOS-3962
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding labels field to Port information, for service discovery to associate 
> arbitrary tags by applications to ports for a given task.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
>   include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 
> 
> Diff: https://reviews.apache.org/r/41187/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

2015-12-14 Thread Guangya Liu 

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



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


It is always recommeded to add some comments here.


- Guangya Liu


On Dec. 14, 2015, 11:34 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41187/
> ---
> 
> (Updated Dec. 14, 2015, 11:34 p.m.)
> 
> 
> Review request for mesos, Adam B and Anand Mazumdar.
> 
> 
> Bugs: MESOS-3962
> https://issues.apache.org/jira/browse/MESOS-3962
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding labels field to Port information, for service discovery to associate 
> arbitrary tags by applications to ports for a given task.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
>   include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 
> 
> Diff: https://reviews.apache.org/r/41187/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

2015-12-14 Thread Avinash sridharan 

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

(Updated Dec. 14, 2015, 11:34 p.m.)


Review request for mesos, Adam B and Anand Mazumdar.


Bugs: MESOS-3962
https://issues.apache.org/jira/browse/MESOS-3962


Repository: mesos


Description
---

Adding labels field to Port information, for service discovery to associate 
arbitrary tags by applications to ports for a given task.


Diffs (updated)
-

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

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


Testing
---

make check


Thanks,

Avinash sridharan

Re: Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

2015-12-14 Thread Avinash sridharan 

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

(Updated Dec. 14, 2015, 10:29 p.m.)


Review request for mesos, Adam B and Anand Mazumdar.


Bugs: MESOS-3962
https://issues.apache.org/jira/browse/MESOS-3962


Repository: mesos


Description
---

Adding labels field to Port information, for service discovery to associate 
arbitrary tags by applications to ports for a given task.


Diffs (updated)
-

  include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 

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


Testing
---

make check


Thanks,

Avinash sridharan

Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

2015-12-09 Thread Avinash sridharan 

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

Review request for mesos and Adam B.


Bugs: MESOS-3962
https://issues.apache.org/jira/browse/MESOS-3962


Repository: mesos


Description
---

Adding labels field to Port information, for service discovery to associate 
arbitrary tags by applications to ports for a given task.


Diffs
-

  include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 

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


Testing
---

make check


Thanks,

Avinash sridharan

Re: Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

2015-12-09 Thread Anand Mazumdar 

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


Can we also update the V1 protobufs since this would be external user-facing ?


include/mesos/mesos.proto (line 1568)


Why `Parameters` and not just `Labels` ?

Also why not name the field as `labels` as is being done in other places in 
the file ?


- Anand Mazumdar


On Dec. 10, 2015, 3:32 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41187/
> ---
> 
> (Updated Dec. 10, 2015, 3:32 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3962
> https://issues.apache.org/jira/browse/MESOS-3962
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding labels field to Port information, for service discovery to associate 
> arbitrary tags by applications to ports for a given task.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
> 
> Diff: https://reviews.apache.org/r/41187/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>