Re: Review Request 38278: Updating unversioned Executor protobuf

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 九月 15, 2015, 10:24 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38278/
> ---
> 
> (Updated 九月 15, 2015, 10:24 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding last changes of the executor HTTP API design to the unversioned 
> protobuf
> 
> 
> Diffs
> -
> 
>   include/mesos/executor/executor.proto 52c84b3 
> 
> Diff: https://reviews.apache.org/r/38278/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 38278: Updating unversioned Executor protobuf

[Isabel Jimenez]

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

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


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


Repository: mesos


Description
---

Adding last changes of the executor HTTP API design to the unversioned protobuf


Diffs (updated)
-

  include/mesos/executor/executor.proto 52c84b3 

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


Testing
---

make


Thanks,

Isabel Jimenez

Re: Review Request 38278: Updating unversioned Executor protobuf

[Anand Mazumdar]

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

Ship it!


Ship It!

- Anand Mazumdar


On Sept. 15, 2015, 10:24 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38278/
> ---
> 
> (Updated Sept. 15, 2015, 10:24 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding last changes of the executor HTTP API design to the unversioned 
> protobuf
> 
> 
> Diffs
> -
> 
>   include/mesos/executor/executor.proto 52c84b3 
> 
> Diff: https://reviews.apache.org/r/38278/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 38278: Updating unversioned Executor protobuf

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Sept. 15, 2015, 9:32 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38278/
> ---
> 
> (Updated Sept. 15, 2015, 9:32 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding last changes of the executor HTTP API design to the unversioned 
> protobuf
> 
> 
> Diffs
> -
> 
>   include/mesos/executor/executor.proto 52c84b3 
> 
> Diff: https://reviews.apache.org/r/38278/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 38278: Updating unversioned Executor protobuf

[Anand Mazumdar]

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


Looks good, just a few more minor things. Also to save effort don't bother 
updating the corresponding V1 review for now. You can just copy paste once the 
changes in this review is finalized.


include/mesos/executor/executor.proto (line 129)


`message Subscribe` is not the first event received when the executor 
subscribes.

Can we change the comment to:
// Request to subscribe with the slave. If subscribing after a 
disconnection, it must include a list of all the tasks and updates which 
haven't been acknowledged by the scheduler.



include/mesos/executor/executor.proto (line 160)


Minor: Can we also have a comment before the `framework_id` field ?

// Identifies the framework which generated this call.


- Anand Mazumdar


On Sept. 15, 2015, 9:32 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38278/
> ---
> 
> (Updated Sept. 15, 2015, 9:32 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding last changes of the executor HTTP API design to the unversioned 
> protobuf
> 
> 
> Diffs
> -
> 
>   include/mesos/executor/executor.proto 52c84b3 
> 
> Diff: https://reviews.apache.org/r/38278/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 38278: Updating unversioned Executor protobuf

[Anand Mazumdar]

> On Sept. 15, 2015, 10:10 p.m., Anand Mazumdar wrote:
> > include/mesos/executor/executor.proto, line 163
> > 
> >
> > Minor: Can we also have a comment before the `framework_id` field ?
> > 
> > // Identifies the framework which generated this call.

My bad, ignore. I did not see already that the `tuple` (`ExecutorID`, 
`FrameworkID`) in itself idenfies the executor.


- Anand


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


On Sept. 15, 2015, 9:32 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38278/
> ---
> 
> (Updated Sept. 15, 2015, 9:32 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding last changes of the executor HTTP API design to the unversioned 
> protobuf
> 
> 
> Diffs
> -
> 
>   include/mesos/executor/executor.proto 52c84b3 
> 
> Diff: https://reviews.apache.org/r/38278/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 38278: Updating unversioned Executor protobuf

[Isabel Jimenez]

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

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


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


Repository: mesos


Description
---

Adding last changes of the executor HTTP API design to the unversioned protobuf


Diffs (updated)
-

  include/mesos/executor/executor.proto 52c84b3 

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


Testing
---

make


Thanks,

Isabel Jimenez

Re: Review Request 38278: Updating unversioned Executor protobuf

[Vinod Kone]

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



include/mesos/executor/executor.proto (line 131)


kill this.



include/mesos/executor/executor.proto (line 160)


make this required.

also add a "required FrameworkID framework_id" because executor id is only 
unique within a framework.


- Vinod Kone


On Sept. 15, 2015, 8:13 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38278/
> ---
> 
> (Updated Sept. 15, 2015, 8:13 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding last changes of the executor HTTP API design to the unversioned 
> protobuf
> 
> 
> Diffs
> -
> 
>   include/mesos/executor/executor.proto 52c84b3 
> 
> Diff: https://reviews.apache.org/r/38278/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Review Request 38278: Updating unversioned Executor protobuf

[Isabel Jimenez]

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

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


Repository: mesos


Description
---

Adding last changes of the executor HTTP API design to the unversioned protobuf


Diffs
-

  include/mesos/executor/executor.proto 52c84b3 

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


Testing
---

make


Thanks,

Isabel Jimenez