Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-15 Thread Isabel Jimenez


> On Sept. 15, 2015, 10:40 p.m., Anand Mazumdar wrote:
> > LGTM minus Two Nits:
> > 
> > 1. Can we update the description of the review to just say that this change 
> > just copies the existing unversioned protobuf to the V1 namespace.
> > 2. I am assuming that you would take care of Vinod's earlier comment around 
> > `invalid calls result in BadRequest` for `message Error` description in a 
> > separate patch as you had commented earlier.

1. Done
2. Yes, we have to capture this in both executor and scheduler protos. I'm 
sending a separate patch.


- Isabel


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


On Sept. 15, 2015, 10:46 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38143/
> ---
> 
> (Updated Sept. 15, 2015, 10:46 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3375
> https://issues.apache.org/jira/browse/MESOS-3375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
> added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
> ought to be changed as the executor HTTP API design evolves.
> Note: This proto is a copy from the existing unversioned executor proto with 
> only the necessary changes.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor/executor.hpp PRE-CREATION 
>   include/mesos/v1/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 509256f 
> 
> Diff: https://reviews.apache.org/r/38143/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-15 Thread Isabel Jimenez


- Isabel


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


On Sept. 15, 2015, 10:46 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38143/
> ---
> 
> (Updated Sept. 15, 2015, 10:46 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3375
> https://issues.apache.org/jira/browse/MESOS-3375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
> added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
> ought to be changed as the executor HTTP API design evolves.
> Note: This proto is a copy from the existing unversioned executor proto with 
> only the necessary changes.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor/executor.hpp PRE-CREATION 
>   include/mesos/v1/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 509256f 
> 
> Diff: https://reviews.apache.org/r/38143/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-15 Thread Isabel Jimenez

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

(Updated Sept. 15, 2015, 10:46 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, and Vinod Kone.


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


Repository: mesos


Description (updated)
---

Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
ought to be changed as the executor HTTP API design evolves.
Note: This proto is a copy from the existing unversioned executor proto with 
only the necessary changes.


Diffs
-

  include/mesos/v1/executor/executor.hpp PRE-CREATION 
  include/mesos/v1/executor/executor.proto PRE-CREATION 
  src/Makefile.am 509256f 

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


Testing
---

make && make check


Thanks,

Isabel Jimenez



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-15 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [38278, 38143]

All tests passed.

- Mesos ReviewBot


On Sept. 15, 2015, 10:46 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38143/
> ---
> 
> (Updated Sept. 15, 2015, 10:46 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3375
> https://issues.apache.org/jira/browse/MESOS-3375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
> added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
> ought to be changed as the executor HTTP API design evolves.
> Note: This proto is a copy from the existing unversioned executor proto with 
> only the necessary changes.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor/executor.hpp PRE-CREATION 
>   include/mesos/v1/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 509256f 
> 
> Diff: https://reviews.apache.org/r/38143/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-15 Thread Anand Mazumdar

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

Ship it!


LGTM minus Two Nits:

1. Can we update the description of the review to just say that this change 
just copies the existing unversioned protobuf to the V1 namespace.
2. I am assuming that you would take care of Vinod's earlier comment around 
`invalid calls result in BadRequest` for `message Error` description in a 
separate patch as you had commented earlier.

- Anand Mazumdar


On Sept. 15, 2015, 10:35 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38143/
> ---
> 
> (Updated Sept. 15, 2015, 10:35 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3375
> https://issues.apache.org/jira/browse/MESOS-3375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
> added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
> ought to be changed as the executor HTTP API design evolves.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor/executor.hpp PRE-CREATION 
>   include/mesos/v1/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 509256f 
> 
> Diff: https://reviews.apache.org/r/38143/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-15 Thread Isabel Jimenez

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

(Updated Sept. 15, 2015, 8:16 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
ought to be changed as the executor HTTP API design evolves.


Diffs (updated)
-

  include/mesos/v1/executor/executor.hpp PRE-CREATION 
  include/mesos/v1/executor/executor.proto PRE-CREATION 
  src/Makefile.am 509256f 

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


Testing
---

make && make check


Thanks,

Isabel Jimenez



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-10 Thread Yong Qiao Wang

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

Ship it!


Ship It!

- Yong Qiao Wang


On 九月 10, 2015, 10:32 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38143/
> ---
> 
> (Updated 九月 10, 2015, 10:32 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3375
> https://issues.apache.org/jira/browse/MESOS-3375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
> added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
> ought to be changed as the executor HTTP API design evolves.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor/executor.hpp PRE-CREATION 
>   include/mesos/v1/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 8963cea 
> 
> Diff: https://reviews.apache.org/r/38143/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-10 Thread Isabel Jimenez

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

(Updated Sept. 10, 2015, 11:38 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
ought to be changed as the executor HTTP API design evolves.


Diffs (updated)
-

  include/mesos/v1/executor/executor.hpp PRE-CREATION 
  include/mesos/v1/executor/executor.proto PRE-CREATION 
  src/Makefile.am 8963cea 

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


Testing
---

make && make check


Thanks,

Isabel Jimenez



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-10 Thread Isabel Jimenez

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

(Updated Sept. 10, 2015, 10:32 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
ought to be changed as the executor HTTP API design evolves.


Diffs (updated)
-

  include/mesos/v1/executor/executor.hpp PRE-CREATION 
  include/mesos/v1/executor/executor.proto PRE-CREATION 
  src/Makefile.am 8963cea 

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


Testing
---

make && make check


Thanks,

Isabel Jimenez



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-10 Thread Isabel Jimenez

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

(Updated Sept. 10, 2015, 11:33 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
ought to be changed as the executor HTTP API design evolves.


Diffs
-

  include/mesos/v1/executor/executor.hpp PRE-CREATION 
  include/mesos/v1/executor/executor.proto PRE-CREATION 
  src/Makefile.am 8963cea 

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


Testing
---

make && make check


Thanks,

Isabel Jimenez



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-10 Thread Isabel Jimenez


> On Sept. 10, 2015, 1:17 a.m., Vinod Kone wrote:
> > include/mesos/v1/executor/executor.proto, lines 89-90
> > 
> >
> > Much like with the scheduler, invalid calls result in BadRequest. We 
> > should really update this wording in executor and scheduler protos.

Will open a new patch with both updates.


- Isabel


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


On Sept. 10, 2015, 10:32 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38143/
> ---
> 
> (Updated Sept. 10, 2015, 10:32 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3375
> https://issues.apache.org/jira/browse/MESOS-3375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
> added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
> ought to be changed as the executor HTTP API design evolves.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor/executor.hpp PRE-CREATION 
>   include/mesos/v1/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 8963cea 
> 
> Diff: https://reviews.apache.org/r/38143/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-09 Thread Vinod Kone

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


most (if not all) comments here pertain to the unversioned executor.proto as 
well. i would recommend sending a review for unversioned proto addressing these 
comments and make this review depend on it. that way, this review would be 
purely copy paste.


include/mesos/v1/executor/executor.proto (line 42)


There is nothing in comments on 'Type' about shutdown. Just explain it here.



include/mesos/v1/executor/executor.proto (lines 89 - 90)


Much like with the scheduler, invalid calls result in BadRequest. We should 
really update this wording in executor and scheduler protos.



include/mesos/v1/executor/executor.proto (line 128)


s/acknoledged/acknowledged/



include/mesos/v1/executor/executor.proto (line 165)


This should be required ExecutorID similar to what we did with scheduler.

Also means that we need a 'Subscribe' message. I would recommend renaming 
Resubscribe to Subscribe and adding ExecutorInfo to it.


- Vinod Kone


On Sept. 9, 2015, 11:09 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38143/
> ---
> 
> (Updated Sept. 9, 2015, 11:09 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3375
> https://issues.apache.org/jira/browse/MESOS-3375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
> added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
> ought to be changed as the executor HTTP API design evolves.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor/executor.hpp PRE-CREATION 
>   include/mesos/v1/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 5fdca0f 
> 
> Diff: https://reviews.apache.org/r/38143/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-08 Thread Anand Mazumdar

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

Ship it!


LGTM !

@gyliu : This change just moves the existing executor protobuf's to the V1 
namespace, we can revisit the semantics later when we finalize the design doc. 
Also, thanks for pointing out the typos we had missed during the earlier review 
of these protobuf's.
@ijimenez : Can we delete the old executor protobuf's/headers in a separate 
patch ? I am assuming that no one is using them.

- Anand Mazumdar


On Sept. 5, 2015, 10:05 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38143/
> ---
> 
> (Updated Sept. 5, 2015, 10:05 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3375
> https://issues.apache.org/jira/browse/MESOS-3375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
> added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
> ought to be changed as the executor HTTP API design evolves.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor/executor.hpp PRE-CREATION 
>   include/mesos/v1/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 5fdca0f 
> 
> Diff: https://reviews.apache.org/r/38143/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-05 Thread Isabel Jimenez

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

(Updated Sept. 5, 2015, 10:05 p.m.)


Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.


Changes
---

Fixing typo


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


Repository: mesos


Description
---

Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
ought to be changed as the executor HTTP API design evolves.


Diffs (updated)
-

  include/mesos/v1/executor/executor.hpp PRE-CREATION 
  include/mesos/v1/executor/executor.proto PRE-CREATION 
  src/Makefile.am 5fdca0f 

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


Testing
---

make && make check


Thanks,

Isabel Jimenez



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-05 Thread Isabel Jimenez

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



include/mesos/v1/executor/executor.proto (line 47)


Here we're refering to 'Events', for the Subscribed event message 
FrameworkInfo will have the framework_id. Please note that this protobuf ought 
to have minor changes as the design evolves:

https://docs.google.com/document/d/1dFmTrSZXCo5zj8H8SkJ4HT-V0z2YYnEZVV8Fd_-AupM/edit?usp=sharing



include/mesos/v1/executor/executor.proto (line 91)


arojas is the orginal author of this TODO, if the assignee for this TODO 
needs to be rewritten, we should do it in a separate patch.



include/mesos/v1/executor/executor.proto (line 169)


The design here follows the design for the scheduler HTTP API. All event 
messages require their type. Depending on the 'type' of call message, you 
should expect an optional corresponding type of message. 
For example if you have a 'Call' message of 'type' 'UPDATE' your call 
message will have the optional 'update' message inside with their required 
'status' , 'timestamp' and 'uuid'.


- Isabel Jimenez


On Sept. 5, 2015, 2:35 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38143/
> ---
> 
> (Updated Sept. 5, 2015, 2:35 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3375
> https://issues.apache.org/jira/browse/MESOS-3375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
> added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
> ought to be changed as the executor HTTP API design evolves.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor/executor.hpp PRE-CREATION 
>   include/mesos/v1/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 5fdca0f 
> 
> Diff: https://reviews.apache.org/r/38143/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-05 Thread Isabel Jimenez


> On Sept. 5, 2015, 3:28 a.m., Guangya Liu wrote:
> > include/mesos/v1/executor/executor.proto, line 47
> > 
> >
> > I think that the framework id need to be set when call "SUBSCRIBE" but 
> > not after "SUBSCRIBED"?

Here we're refering to 'Events', for the Subscribed event message FrameworkInfo 
will have the framework_id. Please note that this protobuf ought to have minor 
changes as the design evolves:
https://docs.google.com/document/d/1dFmTrSZXCo5zj8H8SkJ4HT-V0z2YYnEZVV8Fd_-AupM/edit?usp=sharing


> On Sept. 5, 2015, 3:28 a.m., Guangya Liu wrote:
> > include/mesos/v1/executor/executor.proto, line 91
> > 
> >
> > s/arojas/yourid?

arojas is the orginal author of this TODO, if the assignee for this TODO needs 
to be rewritten, we should do it in a separate patch.


> On Sept. 5, 2015, 3:28 a.m., Guangya Liu wrote:
> > include/mesos/v1/executor/executor.proto, line 169
> > 
> >
> > Do you mean that when type is Subscribe, there is no need to set any 
> > message? Why? I think that the framework id, executor id, command should be 
> > set?

The design here follows the design for the scheduler HTTP API. All call and 
event messages require their type. Depending on the 'type' of call message, you 
should expect an optional corresponding type of message. 
For example if you have a 'Call' message of 'type' 'UPDATE' your call message 
will have the optional 'update' message inside with their required 'status' , 
'timestamp' and 'uuid'.


- Isabel


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


On Sept. 5, 2015, 2:35 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38143/
> ---
> 
> (Updated Sept. 5, 2015, 2:35 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3375
> https://issues.apache.org/jira/browse/MESOS-3375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
> added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
> ought to be changed as the executor HTTP API design evolves.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor/executor.hpp PRE-CREATION 
>   include/mesos/v1/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 5fdca0f 
> 
> Diff: https://reviews.apache.org/r/38143/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-05 Thread Guangya Liu

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

Ship it!


Thanks Isabel for the explanation!

- Guangya Liu


On 九月 5, 2015, 10:05 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38143/
> ---
> 
> (Updated 九月 5, 2015, 10:05 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3375
> https://issues.apache.org/jira/browse/MESOS-3375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
> added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
> ought to be changed as the executor HTTP API design evolves.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor/executor.hpp PRE-CREATION 
>   include/mesos/v1/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 5fdca0f 
> 
> Diff: https://reviews.apache.org/r/38143/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-05 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [38143]

All tests passed.

- Mesos ReviewBot


On Sept. 5, 2015, 10:05 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38143/
> ---
> 
> (Updated Sept. 5, 2015, 10:05 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3375
> https://issues.apache.org/jira/browse/MESOS-3375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
> added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
> ought to be changed as the executor HTTP API design evolves.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor/executor.hpp PRE-CREATION 
>   include/mesos/v1/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 5fdca0f 
> 
> Diff: https://reviews.apache.org/r/38143/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-05 Thread Isabel Jimenez


> On Sept. 5, 2015, 3:28 a.m., Guangya Liu wrote:
> > include/mesos/v1/executor/executor.proto, line 169
> > 
> >
> > Do you mean that when type is Subscribe, there is no need to set any 
> > message? Why? I think that the framework id, executor id, command should be 
> > set?
> 
> Isabel Jimenez wrote:
> The design here follows the design for the scheduler HTTP API. All call 
> and event messages require their type. Depending on the 'type' of call 
> message, you should expect an optional corresponding type of message. 
> For example if you have a 'Call' message of 'type' 'UPDATE' your call 
> message will have the optional 'update' message inside with their required 
> 'status' , 'timestamp' and 'uuid'.

Here you can find the design doc for the scheduler HTTP API: 
https://docs.google.com/document/d/1pnIY_HckimKNvpqhKRhbc9eSItWNFT-priXh_urR-T0/edit?usp=sharing


- Isabel


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


On Sept. 5, 2015, 10:05 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38143/
> ---
> 
> (Updated Sept. 5, 2015, 10:05 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3375
> https://issues.apache.org/jira/browse/MESOS-3375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
> added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
> ought to be changed as the executor HTTP API design evolves.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor/executor.hpp PRE-CREATION 
>   include/mesos/v1/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 5fdca0f 
> 
> Diff: https://reviews.apache.org/r/38143/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-04 Thread Guangya Liu

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



include/mesos/v1/executor/executor.proto (line 47)


I think that the framework id need to be set when call "SUBSCRIBE" but not 
after "SUBSCRIBED"?



include/mesos/v1/executor/executor.proto (line 55)


s/successfuly/successfully



include/mesos/v1/executor/executor.proto (line 91)


s/arojas/yourid?



include/mesos/v1/executor/executor.proto (line 169)


Do you mean that when type is Subscribe, there is no need to set any 
message? Why? I think that the framework id, executor id, command should be set?


- Guangya Liu


On 九月 5, 2015, 2:35 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38143/
> ---
> 
> (Updated 九月 5, 2015, 2:35 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3375
> https://issues.apache.org/jira/browse/MESOS-3375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
> added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
> ought to be changed as the executor HTTP API design evolves.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor/executor.hpp PRE-CREATION 
>   include/mesos/v1/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 5fdca0f 
> 
> Diff: https://reviews.apache.org/r/38143/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-04 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [38143]

All tests passed.

- Mesos ReviewBot


On Sept. 5, 2015, 2:35 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38143/
> ---
> 
> (Updated Sept. 5, 2015, 2:35 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3375
> https://issues.apache.org/jira/browse/MESOS-3375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
> added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
> ought to be changed as the executor HTTP API design evolves.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor/executor.hpp PRE-CREATION 
>   include/mesos/v1/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 5fdca0f 
> 
> Diff: https://reviews.apache.org/r/38143/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>