Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-24 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/#review58315 --- As discussed on IRC: -- please split this into multiple reviews

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-21 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/#review57511 --- src/common/slaveinfo_utils.hpp

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-21 Thread Vinod Kone
On Oct. 15, 2014, 1:25 a.m., Vinod Kone wrote: src/master/master.cpp, lines 3146-3149 https://reviews.apache.org/r/25525/diff/8/?file=717605#file717605line3146 Hmm. i don't think we want these semantics. Why not just fail if readmission failed? Cody Maloney wrote: So if

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-21 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/ --- (Updated Oct. 21, 2014, 11:55 p.m.) Review request for mesos, Adam B and Vinod

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-21 Thread Cody Maloney
On Oct. 21, 2014, 5:23 p.m., Vinod Kone wrote: src/common/slaveinfo_utils.hpp, line 32 https://reviews.apache.org/r/25525/diff/8-11/?file=717600#file717600line32 Actually I think this would be better as OptionError compatible() instead of Trybool isCompatible(), similar to how we

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-21 Thread Cody Maloney
On Oct. 15, 2014, 1:25 a.m., Vinod Kone wrote: src/master/master.cpp, lines 3146-3149 https://reviews.apache.org/r/25525/diff/8/?file=717605#file717605line3146 Hmm. i don't think we want these semantics. Why not just fail if readmission failed? Cody Maloney wrote: So if

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/#review57715 --- Bad patch! Reviews applied: [25525] Failed command:

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-21 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/ --- (Updated Oct. 22, 2014, 12:32 a.m.) Review request for mesos, Adam B and Vinod

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/#review57722 --- Bad patch! Reviews applied: [25525] Failed command:

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-15 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/ --- (Updated Oct. 15, 2014, 7:30 p.m.) Review request for mesos, Adam B and Vinod

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-15 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/#review56832 --- Patch looks great! Reviews applied: [25525] All tests passed. -

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-15 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/#review56872 --- Bad patch! Reviews applied: [25525] Failed command:

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-15 Thread Cody Maloney
On Oct. 16, 2014, 2:50 a.m., Cody Maloney wrote: Bad patch! Reviews applied: [25525] Failed command: ['./support/appy-review.sh', '-r', '25525'] Error: -r: ./support/appy-review.sh: No such file or directory Ignore this. I was testing out some changes to the review checking

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-14 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/ --- (Updated Oct. 15, 2014, 3:06 a.m.) Review request for mesos, Adam B and Vinod

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-14 Thread Cody Maloney
On Oct. 15, 2014, 1:25 a.m., Vinod Kone wrote: src/master/master.hpp, line 1308 https://reviews.apache.org/r/25525/diff/8/?file=717604#file717604line1308 This should be CHECK_SOME(compatible). True, a incompatible change shouldn't get that far because there are advance checks, but

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-14 Thread Cody Maloney
- Cody --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/#review56590 --- On Oct. 15, 2014, 3:13 a.m., Cody Maloney wrote:

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-14 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/ --- (Updated Oct. 15, 2014, 3:13 a.m.) Review request for mesos, Adam B and Vinod

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-10 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/#review56158 --- src/common/attributes.cpp

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-10 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/ --- (Updated Oct. 10, 2014, 6:49 p.m.) Review request for mesos, Adam B and Vinod

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-09 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/ --- (Updated Oct. 10, 2014, 2:29 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-09 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/ --- (Updated Oct. 10, 2014, 2:29 a.m.) Review request for mesos, Adam B and Vinod

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-09 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/#review56104 --- Patch looks great! Reviews applied: [25525] All tests passed. -

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-09-30 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/ --- (Updated Sept. 30, 2014, 6:05 p.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-09-30 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/#review55001 --- Patch looks great! Reviews applied: [25525] All tests passed. -

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-09-29 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/ --- (Updated Sept. 29, 2014, 11:26 p.m.) Review request for mesos, Adam B,

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-09-29 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/#review54937 --- Bad patch! Reviews applied: [25261] Failed command: git apply

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-09-19 Thread Vinod Kone
On Sept. 11, 2014, 9:53 p.m., Vinod Kone wrote: OK. I went through parts of this review but I have a bigger suggestion in mind, before I get too much into the weeds. I think it's worthwhile for you to write up a design doc similar to the framework info doc w.r.t. updating

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-09-19 Thread Cody Maloney
On Sept. 11, 2014, 9:53 p.m., Vinod Kone wrote: OK. I went through parts of this review but I have a bigger suggestion in mind, before I get too much into the weeds. I think it's worthwhile for you to write up a design doc similar to the framework info doc w.r.t. updating

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-09-13 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/#review53271 --- Patch looks great! Reviews applied: [25261, 25525] All tests

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-09-12 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/ --- (Updated Sept. 12, 2014, 11:33 p.m.) Review request for mesos, Adam B,

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-09-12 Thread Cody Maloney
On Sept. 11, 2014, 9:53 p.m., Vinod Kone wrote: OK. I went through parts of this review but I have a bigger suggestion in mind, before I get too much into the weeds. I think it's worthwhile for you to write up a design doc similar to the framework info doc w.r.t. updating

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-09-12 Thread Cody Maloney
On Sept. 11, 2014, 9:53 p.m., Vinod Kone wrote: src/slave/slave.cpp, line 3122 https://reviews.apache.org/r/25525/diff/2/?file=685078#file685078line3122 You should checkpoint the updated slave info! Cody Maloney wrote: info contains the new slave info set at the command

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-09-12 Thread Dominic Hamon
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/#review53248 --- src/Makefile.am https://reviews.apache.org/r/25525/#comment92804

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-09-12 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/ --- (Updated Sept. 13, 2014, 12:33 a.m.) Review request for mesos, Adam B,

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-09-12 Thread Cody Maloney
On Sept. 12, 2014, 11:48 p.m., Dominic Hamon wrote: src/common/slaveinfo_utils.cpp, line 44 https://reviews.apache.org/r/25525/diff/3/?file=688208#file688208line44 std::pairconst std::string, const T* might be preferable. T is a pointer to member function in this case. I could try

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-09-11 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/ --- (Updated Sept. 11, 2014, 8:13 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-09-11 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/#review53102 --- Patch looks great! Reviews applied: [25525] All tests passed. -

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-09-11 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/#review53100 --- OK. I went through parts of this review but I have a bigger

Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-09-10 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/ --- Review request for mesos, Adam B, Benjamin Hindman, Patrick Reilly, and Vinod