Re: Review Request 38366: Added helper to model Labels message for state.json.

2015-09-16 Thread Kapil Arya 

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

(Updated Sept. 16, 2015, 6 p.m.)


Review request for mesos, Connor Doyle, Jie Yu, and Niklas Nielsen.


Changes
---

rebased


Repository: mesos


Description
---

Also updated Task modelling to show labels only if Task.has_labels() is true. 
This way, the "labels" field won't shown if there are no labels. This makes it 
consistent with the modelling of rest of the "optional" fields.


Diffs (updated)
-

  src/common/http.hpp 61ad5314fb14cab0d1cec4fb855fb89b7ac8cf60 
  src/common/http.cpp 9c0d31e261788c492224345b9e0c32643ddb4156 
  src/common/protobuf_utils.cpp 08612700c456017638a9978e5fe9cfa466294c46 
  src/master/master.cpp f26271c5b21685916c0654488ac1464f21d72e9a 
  src/tests/common/http_tests.cpp bf8712b11339b409514ab86c1f32eaf7e9c9a2f1 

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


Testing
---

make check


Thanks,

Kapil Arya

Re: Review Request 38366: Added helper to model Labels message for state.json.

2015-09-16 Thread Niklas Nielsen 

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

Ship it!


Ship It!

- Niklas Nielsen


On Sept. 14, 2015, 1:55 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38366/
> ---
> 
> (Updated Sept. 14, 2015, 1:55 p.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, and Niklas Nielsen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also updated Task modelling to show labels only if Task.has_labels() is true. 
> This way, the "labels" field won't shown if there are no labels. This makes 
> it consistent with the modelling of rest of the "optional" fields.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 61ad5314fb14cab0d1cec4fb855fb89b7ac8cf60 
>   src/common/http.cpp 9c0d31e261788c492224345b9e0c32643ddb4156 
>   src/common/protobuf_utils.cpp 08612700c456017638a9978e5fe9cfa466294c46 
>   src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
>   src/tests/common/http_tests.cpp bf8712b11339b409514ab86c1f32eaf7e9c9a2f1 
> 
> Diff: https://reviews.apache.org/r/38366/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 38366: Added helper to model Labels message for state.json.

2015-09-15 Thread Kapil Arya 


> On Sept. 15, 2015, 1:56 p.m., Niklas Nielsen wrote:
> > src/common/protobuf_utils.cpp, line 140
> > 
> >
> > This could break existing 3rd party parsing; why not leave it set?

I am not so sure. This is fairly inconsistent with the rest of the format. Is 
there is a guideline that we can follow for such changes?


- Kapil


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


On Sept. 14, 2015, 4:55 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38366/
> ---
> 
> (Updated Sept. 14, 2015, 4:55 p.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, and Niklas Nielsen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also updated Task modelling to show labels only if Task.has_labels() is true. 
> This way, the "labels" field won't shown if there are no labels. This makes 
> it consistent with the modelling of rest of the "optional" fields.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 61ad5314fb14cab0d1cec4fb855fb89b7ac8cf60 
>   src/common/http.cpp 9c0d31e261788c492224345b9e0c32643ddb4156 
>   src/common/protobuf_utils.cpp 08612700c456017638a9978e5fe9cfa466294c46 
>   src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
>   src/tests/common/http_tests.cpp bf8712b11339b409514ab86c1f32eaf7e9c9a2f1 
> 
> Diff: https://reviews.apache.org/r/38366/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 38366: Added helper to model Labels message for state.json.

2015-09-15 Thread Niklas Nielsen 

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


Alrighty; mind adding a comment above the new model() describing that we do 
this to avoid the nested 'labels: labels: [ ... ]' by just using 
JSON::protobuf()?


src/common/protobuf_utils.cpp (line 140)


This could break existing 3rd party parsing; why not leave it set?


- Niklas Nielsen


On Sept. 14, 2015, 1:55 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38366/
> ---
> 
> (Updated Sept. 14, 2015, 1:55 p.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, and Niklas Nielsen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also updated Task modelling to show labels only if Task.has_labels() is true. 
> This way, the "labels" field won't shown if there are no labels. This makes 
> it consistent with the modelling of rest of the "optional" fields.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 61ad5314fb14cab0d1cec4fb855fb89b7ac8cf60 
>   src/common/http.cpp 9c0d31e261788c492224345b9e0c32643ddb4156 
>   src/common/protobuf_utils.cpp 08612700c456017638a9978e5fe9cfa466294c46 
>   src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
>   src/tests/common/http_tests.cpp bf8712b11339b409514ab86c1f32eaf7e9c9a2f1 
> 
> Diff: https://reviews.apache.org/r/38366/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 38366: Added helper to model Labels message for state.json.

2015-09-14 Thread Kapil Arya 


> On Sept. 14, 2015, 5:40 p.m., Niklas Nielsen wrote:
> > High level comment; would it be worth change JSON::Protobuf() to support 
> > Labels instead?

Probably not. We are trying a shortcut when modelling Labels for state.json. 
Thus, I wouldn't want to change JSON::Protobuf which is supposed to translate 
Protobufs to JSONs.


- Kapil


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


On Sept. 14, 2015, 4:55 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38366/
> ---
> 
> (Updated Sept. 14, 2015, 4:55 p.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, and Niklas Nielsen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also updated Task modelling to show labels only if Task.has_labels() is true. 
> This way, the "labels" field won't shown if there are no labels. This makes 
> it consistent with the modelling of rest of the "optional" fields.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 61ad5314fb14cab0d1cec4fb855fb89b7ac8cf60 
>   src/common/http.cpp 9c0d31e261788c492224345b9e0c32643ddb4156 
>   src/common/protobuf_utils.cpp 08612700c456017638a9978e5fe9cfa466294c46 
>   src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
>   src/tests/common/http_tests.cpp bf8712b11339b409514ab86c1f32eaf7e9c9a2f1 
> 
> Diff: https://reviews.apache.org/r/38366/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 38366: Added helper to model Labels message for state.json.

2015-09-14 Thread Niklas Nielsen 

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


High level comment; would it be worth change JSON::Protobuf() to support Labels 
instead?

- Niklas Nielsen


On Sept. 14, 2015, 1:55 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38366/
> ---
> 
> (Updated Sept. 14, 2015, 1:55 p.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, and Niklas Nielsen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also updated Task modelling to show labels only if Task.has_labels() is true. 
> This way, the "labels" field won't shown if there are no labels. This makes 
> it consistent with the modelling of rest of the "optional" fields.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 61ad5314fb14cab0d1cec4fb855fb89b7ac8cf60 
>   src/common/http.cpp 9c0d31e261788c492224345b9e0c32643ddb4156 
>   src/common/protobuf_utils.cpp 08612700c456017638a9978e5fe9cfa466294c46 
>   src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
>   src/tests/common/http_tests.cpp bf8712b11339b409514ab86c1f32eaf7e9c9a2f1 
> 
> Diff: https://reviews.apache.org/r/38366/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Review Request 38366: Added helper to model Labels message for state.json.

2015-09-14 Thread Kapil Arya 

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

Review request for mesos, Connor Doyle, Jie Yu, and Niklas Nielsen.


Repository: mesos


Description
---

Also updated Task modelling to show labels only if Task.has_labels() is true. 
This way, the "labels" field won't shown if there are no labels. This makes it 
consistent with the modelling of rest of the "optional" fields.


Diffs
-

  src/common/http.hpp 61ad5314fb14cab0d1cec4fb855fb89b7ac8cf60 
  src/common/http.cpp 9c0d31e261788c492224345b9e0c32643ddb4156 
  src/common/protobuf_utils.cpp 08612700c456017638a9978e5fe9cfa466294c46 
  src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
  src/tests/common/http_tests.cpp bf8712b11339b409514ab86c1f32eaf7e9c9a2f1 

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


Testing
---

make check


Thanks,

Kapil Arya