Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On April 13, 2016, 4:24 a.m., Vinod Kone wrote: > > src/launcher/http_command_executor.cpp, lines 804-809 > > > > > > Lets fix this hack now that the executor receives acknowledgements for > > status updates. > >

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On April 12, 2016, 8:24 p.m., Vinod Kone wrote: > > src/launcher/http_command_executor.cpp, lines 296-297 > > > > > > No need to capture taskId because you can get it from `task`? > > Qian Zhang wrote: > I thi

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/#review128773 --- Ship it! Ship It! - Vinod Kone On April 13, 2016, 9:13 a.m.,

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

The ack to the executor is done by the slave as soon as it checkpoints it. @vinodkone > On Apr 13, 2016, at 7:17 AM, Qian Zhang wrote: > > > >>> On April 13, 2016, 4:24 a.m., Vinod Kone wrote: >>> src/launcher/http_command_executor.cpp, lines 804-809 >>>

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On April 13, 2016, 4:24 a.m., Vinod Kone wrote: > > src/launcher/http_command_executor.cpp, lines 804-809 > > > > > > Lets fix this hack now that the executor receives acknowledgements for > > status updates. > >

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/ --- (Updated April 13, 2016, 5:13 p.m.) Review request for mesos, Anand Mazumdar an

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On April 13, 2016, 4:24 a.m., Vinod Kone wrote: > > src/launcher/http_command_executor.cpp, lines 296-297 > > > > > > No need to capture taskId because you can get it from `task`? I think just capuring `task` is n

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/#review128522 --- src/launcher/http_command_executor.cpp (lines 286 - 287)

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/#review127975 --- Ship it! LGTM - Anand Mazumdar On April 9, 2016, 8:40 a.m.,

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On April 8, 2016, 6:52 p.m., Anand Mazumdar wrote: > > src/launcher/http_command_executor.cpp, lines 82-94 > > > > > > Let's move this all out of the `mesos::internal` namespace. We > > typically have `using` decla

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/ --- (Updated April 9, 2016, 4:40 p.m.) Review request for mesos, Anand Mazumdar and

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On April 9, 2016, 2:52 a.m., Anand Mazumdar wrote: > > src/launcher/http_command_executor.cpp, lines 82-94 > > > > > > Let's move this all out of the `mesos::internal` namespace. We > > typically have `using` decla

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/#review127791 --- Looks good! Just one new major comment around moving the code to t

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/ --- (Updated April 7, 2016, 10:13 p.m.) Review request for mesos, Anand Mazumdar an

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On April 6, 2016, 11:14 p.m., Anand Mazumdar wrote: > > Thanks for bearing with me. Most of the new comments are from the previous > > version itself. My bad, should have been caught by me earlier. Thanks Anand for all the comments, you are really rigorous and all your review comments are ve

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On April 6, 2016, 5 p.m., haosdent huang wrote: > > src/launcher/http_command_executor.cpp, line 264 > > > > > > May you add a comment why need add `Second(1)` here? > > Anand Mazumdar wrote: > It's pretty sel

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On April 6, 2016, 5 p.m., haosdent huang wrote: > > src/launcher/http_command_executor.cpp, line 264 > > > > > > May you add a comment why need add `Second(1)` here? It's pretty self-explanatory that it's the back

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/#review127368 --- src/launcher/http_command_executor.cpp (line 256)

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On March 8, 2016, 12:08 a.m., Anand Mazumdar wrote: > > include/mesos/v1/mesos.proto, lines 1796-1813 > > > > > > hmmm .. Did you test if the health check workflow works? > > > > IIUC, the `mesos-health-c

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/#review127329 --- Thanks for bearing with me. Most of the new comments are from the

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/ --- (Updated April 5, 2016, 8:36 p.m.) Review request for mesos, Anand Mazumdar and

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On April 5, 2016, 7:37 a.m., Anand Mazumdar wrote: > > src/launcher/http_command_executor.cpp, lines 475-477 > > > > > > Why change this? What's wrong with the previous version spanning 2 > > lines? The reason sh

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/#review126927 --- This looks pretty good. One more round of minor cleanups and we wo

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/ --- (Updated April 4, 2016, 5:11 p.m.) Review request for mesos, Anand Mazumdar and

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On March 8, 2016, 8:08 a.m., Anand Mazumdar wrote: > > include/mesos/v1/mesos.proto, lines 1796-1813 > > > > > > hmmm .. Did you test if the health check workflow works? > > > > IIUC, the `mesos-health-ch

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On March 8, 2016, 12:08 a.m., Anand Mazumdar wrote: > > include/mesos/v1/mesos.proto, lines 1796-1813 > > > > > > hmmm .. Did you test if the health check workflow works? > > > > IIUC, the `mesos-health-c

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On March 8, 2016, 8:08 a.m., Anand Mazumdar wrote: > > include/mesos/v1/mesos.proto, lines 1796-1813 > > > > > > hmmm .. Did you test if the health check workflow works? > > > > IIUC, the `mesos-health-ch

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On March 8, 2016, 12:08 a.m., Anand Mazumdar wrote: > > include/mesos/v1/mesos.proto, lines 1796-1813 > > > > > > hmmm .. Did you test if the health check workflow works? > > > > IIUC, the `mesos-health-c

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On March 8, 2016, 8:08 a.m., Anand Mazumdar wrote: > > include/mesos/v1/mesos.proto, lines 1796-1813 > > > > > > hmmm .. Did you test if the health check workflow works? > > > > IIUC, the `mesos-health-ch

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On March 8, 2016, 12:08 a.m., Anand Mazumdar wrote: > > include/mesos/v1/mesos.proto, lines 1796-1813 > > > > > > hmmm .. Did you test if the health check workflow works? > > > > IIUC, the `mesos-health-c

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/ --- (Updated April 4, 2016, 9:58 a.m.) Review request for mesos, Anand Mazumdar and

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On March 8, 2016, 8:08 a.m., Anand Mazumdar wrote: > > src/launcher/http_command_executor.cpp, line 120 > > > > > > Let's scope all the functions after this to the `protected` namespace. > > > > I know th

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On March 8, 2016, 12:08 a.m., Anand Mazumdar wrote: > > include/mesos/v1/mesos.proto, lines 1796-1813 > > > > > > hmmm .. Did you test if the health check workflow works? > > > > IIUC, the `mesos-health-c

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/ --- (Updated April 3, 2016, 8:52 p.m.) Review request for mesos, Anand Mazumdar and

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On March 8, 2016, 8:08 a.m., Anand Mazumdar wrote: > > src/launcher/http_command_executor.cpp, line 105 > > > > > > Why did you move this? Let's have the ordering of member variables the > > same as command execut

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On March 14, 2016, 10:18 p.m., Anand Mazumdar wrote: > > Qian, any updates on this? > > Qian Zhang wrote: > Sorry Anand, I am a little busy on the implementation of CNI support in > Mesos, will get back to this patch soon. Do you still have cycles to work on this? If not, I can ask someo

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

> On March 15, 2016, 6:18 a.m., Anand Mazumdar wrote: > > Qian, any updates on this? Sorry Anand, I am a little busy on the implementation of CNI support in Mesos, will get back to this patch soon. - Qian --- This is an automatically g

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/#review123517 --- Qian, any updates on this? - Anand Mazumdar On March 6, 2016, 9

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/#review122377 --- Thanks for working on this. Looks good, just one major issue aroun

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/ --- (Updated March 6, 2016, 5:08 p.m.) Review request for mesos, Anand Mazumdar and

Review Request 44424: Updated http_command_executor.cpp to use v1 API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/ --- Review request for mesos, Anand Mazumdar and Vinod Kone. Bugs: MESOS-3558 h