Re: Review Request 37303: Moved scheduler library to http

2015-08-13 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37303/#review95240 --- Ship it! Looks good modulo minor fixes. I'll fix them and commit

Re: Review Request 37303: Moved scheduler library to http

2015-08-12 Thread Anand Mazumdar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37303/ --- (Updated Aug. 12, 2015, 10:26 p.m.) Review request for mesos, Ben Mahler and

Re: Review Request 37303: Moved scheduler library to http

2015-08-12 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37303/#review95226 --- src/common/http.hpp (line 51)

Re: Review Request 37303: Moved scheduler library to http

2015-08-12 Thread Anand Mazumdar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37303/ --- (Updated Aug. 12, 2015, 6:51 a.m.) Review request for mesos, Ben Mahler and

Re: Review Request 37303: Moved scheduler library to http

2015-08-12 Thread Anand Mazumdar
On Aug. 12, 2015, 5:09 a.m., Ben Mahler wrote: Sorry for not elaborating on all of these, I added some more explanations here. Main thing is cleaning up the read loop and figuring out the callback semantics (do we need to revisit 'connected' / 'disconnected'?). Let's keep the callback

Re: Review Request 37303: Moved scheduler library to http

2015-08-11 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37303/#review94959 --- src/scheduler/scheduler.cpp (lines 328 - 332)

Re: Review Request 37303: Moved scheduler library to http

2015-08-11 Thread Anand Mazumdar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37303/ --- (Updated Aug. 11, 2015, 10:18 p.m.) Review request for mesos, Ben Mahler and

Re: Review Request 37303: Moved scheduler library to http

2015-08-11 Thread Anand Mazumdar
On Aug. 11, 2015, 6:55 p.m., Vinod Kone wrote: src/scheduler/scheduler.cpp, line 329 https://reviews.apache.org/r/37303/diff/2/?file=1036937#file1036937line329 indent by 4 spaces. Not used now. Fixed the other one. - Anand

Re: Review Request 37303: Moved scheduler library to http

2015-08-11 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37303/#review95036 --- Thanks Anand, mostly thinking we can clean up the read logic if we

Re: Review Request 37303: Moved scheduler library to http

2015-08-11 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37303/#review95059 --- Sorry for not elaborating on all of these, I added some more

Re: Review Request 37303: Moved scheduler library to http

2015-08-11 Thread Anand Mazumdar
On Aug. 12, 2015, 1:20 a.m., Ben Mahler wrote: Thanks Anand, mostly thinking we can clean up the read logic if we have a struct to capture the reader / decoder. Isn't it much more simpler here? It's just a one liner if check to check if the reader is reader is not None and != for stale

Review Request 37303: Moved scheduler library to http

2015-08-10 Thread Anand Mazumdar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37303/ --- Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-2552

Re: Review Request 37303: Moved scheduler library to http

2015-08-10 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37303/#review94777 --- looks like the SchedulerTest.Subscribe test failed?

Re: Review Request 37303: Moved scheduler library to http

2015-08-10 Thread Anand Mazumdar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37303/ --- (Updated Aug. 11, 2015, 12:17 a.m.) Review request for mesos, Ben Mahler and

Re: Review Request 37303: Moved scheduler library to http

2015-08-10 Thread Anand Mazumdar
On Aug. 10, 2015, 7:52 p.m., Vinod Kone wrote: src/common/http.hpp, line 73 https://reviews.apache.org/r/37303/diff/1/?file=1036466#file1036466line73 this helper seems weird. i would rather have isOK()and isAccepted() helpers. anyway, this seems to be used only once.