Re: Review Request 34943: Added validation to flags.

2015-06-18 Thread Benjamin Hindman
On June 15, 2015, 10:17 a.m., Bernd Mathiske wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp, line 146 https://reviews.apache.org/r/34943/diff/4/?file=984516#file984516line146 Could we use some more concrete typing like this here?

Re: Review Request 35405: Enable deleting MasterDetector in MesosSchedulerDriver::join.

2015-06-18 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35405/ --- (Updated June 18, 2015, 12:39 p.m.) Review request for mesos and Niklas

Re: Review Request 34943: Added validation to flags.

2015-06-18 Thread Benjamin Hindman
On June 15, 2015, 11:48 p.m., Niklas Nielsen wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp, lines 484-490 https://reviews.apache.org/r/34943/diff/4/?file=984516#file984516line484 I am a bit torn whether we should copy this block so many places (taken

Re: Review Request 35571: Adding ability to decode JSON from ZK

2015-06-18 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35571/#review88359 --- Ship it! This looks great Marco. Let's get this committed folks!

Re: Review Request 35566: Refactor executorEnvironment to take slave::Flags.

2015-06-18 Thread Benjamin Hindman
On June 18, 2015, 1:01 a.m., Till Toenshoff wrote: src/tests/containerizer.cpp, line 124 https://reviews.apache.org/r/35566/diff/1/?file=986478#file986478line124 This indention will very soon not comply with our styleguide anymore, no? No, it will, we still wrap the same way for

Re: Review Request 35438: Fixed fetcher cache test race for resource offers when starting tasks and changed corresponding CHECK to EXPECT.

2015-06-18 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35438/#review88361 --- Ship it! This looks good Bernd. And you should be able to commit

Re: Review Request 34943: Added validation to flags.

2015-06-18 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34943/ --- (Updated June 18, 2015, 12:29 p.m.) Review request for mesos and Niklas

Re: Review Request 35571: Adding ability to decode JSON from ZK

2015-06-18 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35571/ --- (Updated June 18, 2015, 2:46 p.m.) Review request for mesos, Niklas Nielsen

Re: Review Request 35571: Adding ability to decode JSON from ZK

2015-06-18 Thread Benjamin Hindman
On June 18, 2015, 11:09 a.m., Benjamin Hindman wrote: src/master/detector.cpp, lines 468-469 https://reviews.apache.org/r/35571/diff/5/?file=986674#file986674line468 You should always know this is an error because you're in the 'else' branch, so you can just do:

Re: Review Request 35611: Added initial doxygen documentation for stout flags.

2015-06-18 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35611/ --- (Updated June 18, 2015, 3:33 p.m.) Review request for mesos and Bernd

Re: Review Request 35571: Adding ability to decode JSON from ZK

2015-06-18 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35571/#review88378 --- Ship it! Still a Ship It, just a minor nit from the revision.

Re: Review Request 35571: Adding ability to decode JSON from ZK

2015-06-18 Thread Niklas Nielsen
On June 18, 2015, 7:53 a.m., Benjamin Hindman wrote: Still a Ship It, just a minor nit from the revision. @BenH and @BenM; thanks for stepping in! I can get this committed today. - Niklas --- This is an automatically generated

Review Request 35611: Added initial doxygen documentation for stout flags.

2015-06-18 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35611/ --- Review request for mesos and Bernd Mathiske. Repository: mesos Description

Re: Review Request 35433: Send StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-18 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433/ --- (Updated June 18, 2015, 5:01 p.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 35433: Send StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-18 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433/#review88393 --- Michael, there are some artifacts in the patch, could you please

Re: Review Request 35433: Send StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-18 Thread Michael Park
On June 18, 2015, 5:02 p.m., Alexander Rukletsov wrote: docs/mesos-frameworks.md, lines 29-30 https://reviews.apache.org/r/35433/diff/3/?file=986908#file986908line29 Artifact? This was recently removed. Yeah, sorry about that. Rebased. - Michael

Re: Review Request 35433: Send StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-18 Thread Michael Park
On June 18, 2015, 5:02 p.m., Alexander Rukletsov wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp, lines 104-112 https://reviews.apache.org/r/35433/diff/3/?file=986907#file986907line104 Ditto. Yeah, sorry about that. Rebased. - Michael

Re: Review Request 35433: Send StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-18 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433/ --- (Updated June 18, 2015, 5:03 p.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 35433: Send StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-18 Thread Jie Yu
On June 18, 2015, 5:27 p.m., Jie Yu wrote: src/slave/slave.cpp, lines 1403-1421 https://reviews.apache.org/r/35433/diff/5/?file=986979#file986979line1403 Hum, are you sending multiple status update if there are multiple 'resource's in 'checkpointedTaskResources'? My bad. Didn't

Re: Review Request 34767: Added testing patterns doc.

2015-06-18 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34767/#review88399 --- Ship it! Only comment here is that we need to use ~~~ instead of

Re: Review Request 35433: Send StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-18 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433/#review88405 --- src/slave/slave.cpp (lines 1418 - 1420)

Re: Review Request 34767: Added testing patterns doc.

2015-06-18 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34767/ --- (Updated June 18, 2015, 3:56 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 35433: Send StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-18 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433/#review88392 --- Your latest diff does not look right. - Jie Yu On June 18, 2015,

Re: Review Request 34943: Added validation to flags.

2015-06-18 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34943/#review88386 --- Ship it! Ship It! - Bernd Mathiske On June 18, 2015, 8:34 a.m.,

Re: Review Request 35433: Send StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-18 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433/ --- (Updated June 18, 2015, 4:54 p.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 35433: CHECK that checkpointed resources exist on the slave.

2015-06-18 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433/ --- (Updated June 18, 2015, 4:54 p.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 35571: Adding ability to decode JSON from ZK

2015-06-18 Thread Vinod Kone
On June 17, 2015, 11:52 p.m., Niklas Nielsen wrote: LGTM modulo wrapping at line 450-451. Would love to see a test before we ship this. @vinod - can you take a look at this too? i'll take a look now. - Vinod --- This is an

Re: Review Request 35433: Send StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-18 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433/#review88406 --- src/slave/slave.cpp (lines 1403 - 1421)

Review Request 35638: Removed const-ref to temporaries from the Slave.

2015-06-18 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35638/ --- Review request for mesos, Benjamin Hindman, Jie Yu, and Joris Van Remoortere.

Re: Review Request 35638: Removed const-ref to temporaries from the Slave.

2015-06-18 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35638/ --- (Updated June 19, 2015, 12:43 a.m.) Review request for mesos, Benjamin

Re: Review Request 35635: Formatting cleanup in the Slave.

2015-06-18 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35635/#review88475 --- src/slave/slave.cpp (lines 2904 - 2905)

Re: Review Request 35433: Sent StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-18 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433/ --- (Updated June 19, 2015, 12:42 a.m.) Review request for mesos, Alexander

Re: Review Request 35433: Send StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-18 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433/ --- (Updated June 19, 2015, 12:42 a.m.) Review request for mesos, Alexander

Re: Review Request 35631: Added a performance benchmark for hierarchical allocator addSlave.

2015-06-18 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35631/ --- (Updated June 19, 2015, 1:11 a.m.) Review request for mesos, Ben Mahler and

Re: Review Request 35635: Formatting cleanup in the Slave.

2015-06-18 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35635/ --- (Updated June 19, 2015, 2:12 a.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 35631: Added a performance benchmark for hierarchical allocator addSlave.

2015-06-18 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35631/#review88478 --- Ship it! src/tests/hierarchical_allocator_tests.cpp (line 984)

Re: Review Request 35433: Send StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-18 Thread Jie Yu
On June 18, 2015, 5:21 p.m., Benjamin Hindman wrote: src/slave/slave.cpp, lines 1418-1420 https://reviews.apache.org/r/35433/diff/5/?file=986979#file986979line1418 The only logic question I have here now is how come we don't need the following code even though we use it above?

Re: Review Request 35561: Updated process::subprocess to replace environment.

2015-06-18 Thread Ben Mahler
On June 17, 2015, 8:23 p.m., Till Toenshoff wrote: 3rdparty/libprocess/src/subprocess.cpp, line 332 https://reviews.apache.org/r/35561/diff/1/?file=986458#file986458line332 Aren't we leaking this one in the parent process? Benjamin Hindman wrote: We'll be exec'ing, so

Re: Review Request 35571: Adding ability to decode JSON from ZK

2015-06-18 Thread Ben Mahler
On June 18, 2015, 1:08 a.m., Ben Mahler wrote: src/master/detector.cpp, lines 444-447 https://reviews.apache.org/r/35571/diff/4/?file=986626#file986626line444 Our logging messages do not end with periods, please do a sweep :) Ditto for aligning on the operator. To be

Re: Review Request 35561: Updated process::subprocess to replace environment.

2015-06-18 Thread Benjamin Hindman
On June 17, 2015, 8:23 p.m., Till Toenshoff wrote: 3rdparty/libprocess/src/subprocess.cpp, line 332 https://reviews.apache.org/r/35561/diff/1/?file=986458#file986458line332 Aren't we leaking this one in the parent process? Benjamin Hindman wrote: We'll be exec'ing, so

Re: Review Request 35561: Updated process::subprocess to replace environment.

2015-06-18 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35561/ --- (Updated June 18, 2015, 6:39 p.m.) Review request for mesos and Till

Re: Review Request 35433: Send StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-18 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433/ --- (Updated June 18, 2015, 6:45 p.m.) Review request for mesos, Alexander

Re: Review Request 35571: Adding ability to decode JSON from ZK

2015-06-18 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35571/#review88418 --- Ship it! Please definitely write a test (in a follow up review) to

Re: Review Request 35571: Adding ability to decode JSON from ZK

2015-06-18 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35571/#review88423 --- Thanks Marco, want to see this polished off properly before a ship

Re: Review Request 35433: Send StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-18 Thread Michael Park
On June 18, 2015, 5:21 p.m., Benjamin Hindman wrote: src/slave/slave.cpp, lines 1418-1420 https://reviews.apache.org/r/35433/diff/5/?file=986979#file986979line1418 The only logic question I have here now is how come we don't need the following code even though we use it above?

Re: Review Request 35433: Send StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-18 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433/ --- (Updated June 18, 2015, 7:47 p.m.) Review request for mesos, Alexander

Re: Review Request 35561: Updated process::subprocess to replace environment.

2015-06-18 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35561/#review88419 --- 3rdparty/libprocess/src/subprocess.cpp (lines 329 - 340)

Re: Review Request 35561: Updated process::subprocess to replace environment.

2015-06-18 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35561/ --- (Updated June 18, 2015, 6:48 p.m.) Review request for mesos and Till

Re: Review Request 35622: Adding a link to example framework implementations (RENDLER)

2015-06-18 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35622/ --- (Updated June 18, 2015, 9:18 p.m.) Review request for mesos and Joris Van

Re: Review Request 35622: Adding a link to example framework implementations (RENDLR)

2015-06-18 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35622/ --- (Updated June 18, 2015, 9:18 p.m.) Review request for mesos and Joris Van

Re: Review Request 35405: Enable deleting MasterDetector in MesosSchedulerDriver::join.

2015-06-18 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35405/#review88437 --- src/sched/sched.cpp (line 1659)

Re: Review Request 35622: Adding a link to example framework implementations (RENDLER)

2015-06-18 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35622/ --- (Updated June 18, 2015, 9:24 p.m.) Review request for mesos and Joris Van

Review Request 35622: Adding a link to example framework implementations (RENDLR)

2015-06-18 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35622/ --- Review request for mesos and Joris Van Remoortere. Repository: mesos

Re: Review Request 35622: Adding a link to example framework implementations (RENDLR)

2015-06-18 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35622/#review88436 --- docs/app-framework-development-guide.md (line 9)

Re: Review Request 35611: Added initial doxygen documentation for stout flags.

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

Review Request 35625: Fixing a typo

2015-06-18 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35625/ --- Review request for mesos. Repository: mesos Description --- Fixing a

Re: Review Request 35622: Adding a link to example framework implementations (RENDLER)

2015-06-18 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35622/ --- (Updated June 18, 2015, 9:29 p.m.) Review request for mesos and Joris Van

Review Request 35631: Added a performance benchmark for hierarchical allocator addSlave.

2015-06-18 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35631/ --- Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-2892

Re: Review Request 35287: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout)

2015-06-18 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35287/#review88428 --- Thanks for taking this on Mark! Would you mind rebasing this chain

Re: Review Request 35631: Added a performance benchmark for hierarchical allocator addSlave.

2015-06-18 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35631/ --- (Updated June 18, 2015, 11:02 p.m.) Review request for mesos, Ben Mahler and

Re: Review Request 35585: Updated Isolator to return required namespaces.

2015-06-18 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35585/#review88442 --- A few high level comments: 1) Let's use an enum instead of an

Review Request 35632: Added queue size metric for the allocator.

2015-06-18 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35632/ --- Review request for mesos and Jie Yu. Bugs: MESOS-2893

Re: Review Request 35631: Added a performance benchmark for hierarchical allocator addSlave.

2015-06-18 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35631/ --- (Updated June 18, 2015, 11:14 p.m.) Review request for mesos, Ben Mahler and

Re: Review Request 35632: Added queue size metric for the allocator.

2015-06-18 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35632/#review88463 --- Ship it! Ship It! - Jie Yu On June 18, 2015, 11:10 p.m., Ben

Re: Review Request 35353: Smaller fixes on libprocess firewall

2015-06-18 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35353/ --- (Updated June 18, 2015, 11:20 a.m.) Review request for mesos, Ben Mahler and