Re: Review Request 43701: Added a command executor based on the new V1 API.

2016-03-05 Thread Qian Zhang
> On Feb. 29, 2016, 10:24 a.m., Vinod Kone wrote: > > Thanks for working on this Qian! > > > > It's really hard to tell what changes were made to the http command > > executor that are different from the command executor. I would suggest you > > to split this into multiple reviews to make

Re: Review Request 43701: Added a command executor based on the new V1 API.

2016-03-04 Thread Vinod Kone
> On Feb. 29, 2016, 2:24 a.m., Vinod Kone wrote: > > Thanks for working on this Qian! > > > > It's really hard to tell what changes were made to the http command > > executor that are different from the command executor. I would suggest you > > to split this into multiple reviews to make

Re: Review Request 43701: Added a command executor based on the new V1 API.

2016-02-29 Thread Qian Zhang
> On Feb. 29, 2016, 1:42 a.m., Shuai Lin wrote: > > src/launcher/http_executor.cpp, line 82 > > > > > > Maybe we should also add some tests that launches tasks with this new > > http cmd executor? Yes, agree! -

Re: Review Request 43701: Added a command executor based on the new V1 API.

2016-02-29 Thread Qian Zhang
> On Feb. 29, 2016, 10:24 a.m., Vinod Kone wrote: > > Thanks for working on this Qian! > > > > It's really hard to tell what changes were made to the http command > > executor that are different from the command executor. I would suggest you > > to split this into multiple reviews to make

Re: Review Request 43701: Added a command executor based on the new V1 API.

2016-02-28 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43701/#review121177 --- Thanks for working on this Qian! It's really hard to tell what

Re: Review Request 43701: Added a command executor based on the new V1 API.

2016-02-28 Thread Shuai Lin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43701/#review121155 --- src/launcher/http_executor.cpp (line 82)

Re: Review Request 43701: Added a command executor based on the new V1 API.

2016-02-20 Thread Shuai Lin
> On Feb. 19, 2016, 4:24 a.m., Shuai Lin wrote: > > Maybe we should also add a test that launches a task with this new http cmd > > executor? ping - Shuai --- This is an automatically generated e-mail. To reply, visit:

Re: Review Request 43701: Added a command executor based on the new V1 API.

2016-02-20 Thread Shuai Lin
> On Feb. 18, 2016, 4:07 p.m., Shuai Lin wrote: > > src/slave/flags.cpp, line 693 > > > > > > One space before `\n`, otherwise the word would be mixed with the first > > word of the next line. > > Qian Zhang

Re: Review Request 43701: Added a command executor based on the new V1 API.

2016-02-20 Thread Shuai Lin
> On Feb. 18, 2016, 4:07 p.m., Shuai Lin wrote: > > src/slave/slave.cpp, line 3709 > > > > > > Instead of repeating the `if (flags.http_command_executor)...` logic > > multiple times, I would prefer use a temp

Re: Review Request 43701: Added a command executor based on the new V1 API.

2016-02-20 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43701/#review120046 --- Patch looks great! Reviews applied: [43701] Passed command:

Re: Review Request 43701: Added a command executor based on the new V1 API.

2016-02-20 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43701/ --- (Updated Feb. 20, 2016, 9:44 p.m.) Review request for mesos, Anand Mazumdar

Re: Review Request 43701: Added a command executor based on the new V1 API.

2016-02-19 Thread Qian Zhang
> On Feb. 19, 2016, 12:23 p.m., Shuai Lin wrote: > > src/launcher/http_executor.cpp, line 776 > > > > > > Unlike unacked status updates, there could be at most one unacked task > > info, so here we don't need a map

Re: Review Request 43701: Added a command executor based on the new V1 API.

2016-02-19 Thread Qian Zhang
> On Feb. 19, 2016, 12:07 a.m., Shuai Lin wrote: > > src/slave/flags.cpp, line 693 > > > > > > One space before `\n`, otherwise the word would be mixed with the first > > word of the next line. I do not think we

Re: Review Request 43701: Added a command executor based on the new V1 API.

2016-02-19 Thread Qian Zhang
> On Feb. 18, 2016, 1:42 p.m., Jian Qiu wrote: > > src/launcher/http_executor.cpp, line 118 > > > > > > this method and other methods not implementation of the virtual method > > should be private I think other

Re: Review Request 43701: Added a command executor based on the new V1 API.

2016-02-18 Thread Shuai Lin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43701/#review119805 --- Maybe we should also add a test that launches a task with this

Re: Review Request 43701: Added a command executor based on the new V1 API.

2016-02-18 Thread Shuai Lin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43701/#review119623 --- src/slave/flags.cpp (line 693)

Re: Review Request 43701: Added a command executor based on the new V1 API.

2016-02-17 Thread Jian Qiu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43701/#review119584 --- src/launcher/http_executor.cpp (line 118)

Re: Review Request 43701: Added a command executor based on the new V1 API.

2016-02-17 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43701/#review119583 --- Patch looks great! Reviews applied: [43701] Passed command: