Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-04-01 Thread Adam B
On Feb. 18, 2015, 11:38 a.m., Niklas Nielsen wrote: Let's get tests wired up before committing this :) Adam B wrote: Sure thing. Adding tests in my subsequent patch where we will pass the master's timeout values on to the slave. Will post that very soon. Ben Mahler wrote:

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-03-31 Thread Ben Mahler
On Feb. 18, 2015, 7:38 p.m., Niklas Nielsen wrote: Let's get tests wired up before committing this :) Adam B wrote: Sure thing. Adding tests in my subsequent patch where we will pass the master's timeout values on to the slave. Will post that very soon. Ben Mahler wrote: Can

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-03-27 Thread Adam B
On Feb. 18, 2015, 11:38 a.m., Niklas Nielsen wrote: Let's get tests wired up before committing this :) Adam B wrote: Sure thing. Adding tests in my subsequent patch where we will pass the master's timeout values on to the slave. Will post that very soon. Ben Mahler wrote:

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-03-27 Thread Adam B
On March 6, 2015, 5:19 a.m., Joerg Schad wrote: src/master/flags.hpp, line 383 https://reviews.apache.org/r/29507/diff/4/?file=868836#file868836line383 Shouldn't this also be added to the documentation (i.e. http://mesos.apache.org/documentation/latest/configuration/)? Excellent

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-03-06 Thread Joerg Schad
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/#review75484 --- src/master/flags.hpp

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-02-19 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/ --- (Updated Feb. 19, 2015, 12:10 a.m.) Review request for mesos, Ben Mahler and

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-02-19 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/#review73111 --- Patch looks great! Reviews applied: [29507] All tests passed. -

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-02-19 Thread Adam B
On Feb. 18, 2015, 11:38 a.m., Niklas Nielsen wrote: Let's get tests wired up before committing this :) Adam B wrote: Sure thing. Adding tests in my subsequent patch where we will pass the master's timeout values on to the slave. Will post that very soon. Ben Mahler wrote:

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-02-18 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/#review72992 --- Let's get tests wired up before committing this :) - Niklas

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-02-18 Thread Adam B
On Feb. 18, 2015, 11:38 a.m., Niklas Nielsen wrote: Let's get tests wired up before committing this :) Sure thing. Adding tests in my subsequent patch where we will pass the master's timeout values on to the slave. Will post that very soon. - Adam

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-02-18 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/#review73085 --- Bad patch! Reviews applied: [29507] Failed command:

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-02-18 Thread Ben Mahler
On Feb. 18, 2015, 7:38 p.m., Niklas Nielsen wrote: Let's get tests wired up before committing this :) Adam B wrote: Sure thing. Adding tests in my subsequent patch where we will pass the master's timeout values on to the slave. Will post that very soon. Can you do it in one patch?

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-02-18 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/ --- (Updated Feb. 18, 2015, 4:33 p.m.) Review request for mesos and Niklas

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-02-17 Thread Niklas Nielsen
On Jan. 7, 2015, 2:50 p.m., Ben Mahler wrote: src/master/flags.hpp, lines 361-373 https://reviews.apache.org/r/29507/diff/1/?file=804645#file804645line361 Do you mind changing the names now that they're merely defaults? s/SLAVE_PING_TIMEOUT/DEFAULT_SLAVE_PING_TIMEOUT/

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-01-25 Thread Adam B
On Jan. 7, 2015, 2:50 p.m., Ben Mahler wrote: src/master/flags.hpp, lines 361-373 https://reviews.apache.org/r/29507/diff/1/?file=804645#file804645line361 Do you mind changing the names now that they're merely defaults? s/SLAVE_PING_TIMEOUT/DEFAULT_SLAVE_PING_TIMEOUT/

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-01-09 Thread Ben Mahler
On Jan. 7, 2015, 10:50 p.m., Ben Mahler wrote: src/master/flags.hpp, lines 361-373 https://reviews.apache.org/r/29507/diff/1/?file=804645#file804645line361 Do you mind changing the names now that they're merely defaults? s/SLAVE_PING_TIMEOUT/DEFAULT_SLAVE_PING_TIMEOUT/

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-01-08 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/#review67183 --- Patch looks great! Reviews applied: [29507] All tests passed. -

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-01-08 Thread Adam B
On Jan. 7, 2015, 2:50 p.m., Ben Mahler wrote: src/master/flags.hpp, lines 361-373 https://reviews.apache.org/r/29507/diff/1/?file=804645#file804645line361 Do you mind changing the names now that they're merely defaults? s/SLAVE_PING_TIMEOUT/DEFAULT_SLAVE_PING_TIMEOUT/

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-01-08 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/ --- (Updated Jan. 8, 2015, 12:44 a.m.) Review request for mesos and Niklas

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-01-08 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/#review67182 --- src/tests/partition_tests.cpp

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-01-07 Thread Adam B
On Jan. 6, 2015, 9:17 a.m., Niklas Nielsen wrote: src/tests/mesos.cpp, line 129 https://reviews.apache.org/r/29507/diff/1/?file=804648#file804648line129 Can we use a constant here instead (to make sure it follows future changes to the default value?) The point of setting these

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-01-07 Thread Adam B
On Jan. 6, 2015, 9:17 a.m., Niklas Nielsen wrote: src/tests/mesos.cpp, line 129 https://reviews.apache.org/r/29507/diff/1/?file=804648#file804648line129 Can we use a constant here instead (to make sure it follows future changes to the default value?) Adam B wrote: The

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-01-07 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/#review67113 --- src/master/flags.hpp

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-01-07 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/#review67077 --- src/master/master.cpp

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-01-07 Thread Michael Park
On Jan. 7, 2015, 8:22 p.m., Michael Park wrote: Looks good to me. Just a minor comment on consistent variable naming. - Michael --- This is an automatically generated e-mail. To reply, visit:

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-01-07 Thread Michael Park
On Jan. 6, 2015, 5:17 p.m., Niklas Nielsen wrote: src/tests/mesos.cpp, line 129 https://reviews.apache.org/r/29507/diff/1/?file=804648#file804648line129 Can we use a constant here instead (to make sure it follows future changes to the default value?) Adam B wrote: The

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-01-06 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/#review66859 --- Looks good! Have a couple of small questions src/master/flags.hpp

Review Request 29507: Added Configurable Slave Ping Timeouts

2014-12-31 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/ --- Review request for mesos and Niklas Nielsen. Bugs: MESOS-2150

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

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