Re: Review Request 34703: Added stream manipulators for the Time object.

2015-05-28 Thread Alexander Rojas
> On May 28, 2015, 7:49 a.m., Nikita Vetoshkin wrote: > > 3rdparty/libprocess/include/process/time.hpp, line 109 > > > > > > Can I add one more nitpick? If we make `WEEK_DAYS` and `MONTHS` > > `static` then will put o

Re: Review Request 34646: Return empty task list when current master is not a leader.

2015-05-28 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34646/#review85528 --- The current code already returns an empty task list, with response c

Re: Review Request 34724: Doxygen enhancements

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

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

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

Re: Review Request 34195: Refactoring to use FlagsBase common functionality

2015-05-28 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34195/ --- (Updated May 28, 2015, 10:06 a.m.) Review request for mesos, Benjamin Hindman a

Re: Review Request 34195: Refactoring to use FlagsBase common functionality

2015-05-28 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34195/#review85536 --- src/cli/execute.cpp

Re: Review Request 34193: Refactored common functionality into FlagsBase

2015-05-28 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34193/#review85537 --- Ship it! 3rdparty/libprocess/3rdparty/stout/include/stout/flags/fl

Re: Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage.

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

Re: Review Request 34655: Use relative url in /help generated links point

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

Re: Review Request 33781: Add license blobs to Java JNI cpp files

2015-05-28 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33781/#review85558 --- Ship it! Ship It! - Benjamin Hindman On May 2, 2015, 2:35 p.m.,

Re: Review Request 34134: Add container rootfs to Isolator::prepare().

2015-05-28 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34134/#review85564 --- Ship it! Ship It! - Timothy Chen On May 13, 2015, 12:44 a.m., Ia

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-28 Thread Niklas Nielsen
> On May 27, 2015, 5:24 p.m., Vinod Kone wrote: > > src/examples/test_resource_estimator_module.cpp, lines 40-44 > > > > > > I'm confused. 'parameters' are unused and the passed 'flags' are empty? > > Niklas Nielsen wr

Re: Review Request 34676: Add printing of extended attributes to Resource objects.

2015-05-28 Thread Vinod Kone
> On May 26, 2015, 8:57 p.m., Vinod Kone wrote: > > src/common/resources.cpp, line 1160 > > > > > > Lets use something other than "*", since we already abuse it to > > represent the default role and unreserved resour

Re: Review Request 34655: Use relative url in /help generated links point

2015-05-28 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34655/#review85571 --- Do you clearly understand why this change is needed? I didn't unders

Re: Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage.

2015-05-28 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34748/#review85585 --- This will change the resource estimator patch set too - do you want

Re: Review Request 34655: Use relative url in /help generated links point

2015-05-28 Thread Marco Massenzio
> On May 28, 2015, 5:40 p.m., Adam B wrote: > > Do you clearly understand why this change is needed? I didn't understand > > after just reading the JIRA, and had to ask the reporter(s). Mesosphere is > > hosting the Mesos UI(s) underneath the DCOS UI behind a reverse proxy, so > > that `http:/

Re: Review Request 34655: Use relative url in /help generated links point

2015-05-28 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34655/#review85592 --- Ship it! Ship It! - Marco Massenzio On May 28, 2015, 7:54 a.m.,

Re: Review Request 34654: Send docker inspect output with TaskStatus data.

2015-05-28 Thread Timothy Chen
> On May 27, 2015, 5:50 p.m., Benjamin Hindman wrote: > > src/docker/executor.cpp, line 154 > > > > > > Why back to MergeFrom instead of CopyFrom? Ah sorry merge conflict! > On May 27, 2015, 5:50 p.m., Benjamin Hind

Re: Review Request 34737: Added test to verify fix for MESOS-2771

2015-05-28 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34737/#review85586 --- Ship it! Looks good. Main issue is that this can and should be a ge

Re: Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage. Pass ResourceUsage to ResourceEstimator

2015-05-28 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34748/ --- (Updated May 28, 2015, 6:41 p.m.) Review request for mesos, Jie Yu, Joris Van R

Re: Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage. Pass ResourceUsage to ResourceEstimator

2015-05-28 Thread Bartek Plotka
> On May 28, 2015, 6:12 p.m., Niklas Nielsen wrote: > > This will change the resource estimator patch set too - do you want to make > > them dependent on this one? This patch depents on Resource Estimator modularization. (: - Bartek --

Re: Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage. Pass ResourceUsage to ResourceEstimator

2015-05-28 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34748/#review85601 --- Patch looks great! Reviews applied: [34662, 34748] All tests passe

Re: Review Request 34737: Added test to verify fix for MESOS-2771

2015-05-28 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34737/ --- (Updated May 28, 2015, 12:48 p.m.) Review request for mesos, Ian Downes, Jie Yu

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-28 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/#review85606 --- Ship it! Ship It! - Niklas Nielsen On May 27, 2015, 6:20 p.m., B

Re: Review Request 34736: Implemented 'updateSlave()' call in the master.

2015-05-28 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34736/#review85604 --- Looks good. One high level comment is that we may need to _not_ type

Review Request 34782: Convert Non-POD static variables in fq_codel and ingress to constexpr

2015-05-28 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34782/ --- Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Bugs: M

Re: Review Request 34687: (De)Serializing MasterInfo PB to JSON

2015-05-28 Thread Niklas Nielsen
> On May 26, 2015, 7:18 p.m., Niklas Nielsen wrote: > > src/common/protobuf_utils.hpp, line 83 > > > > > > Shouldn't parse() go in > > https://github.com/apache/mesos/blob/master/src/common/parse.hpp? > > Marco Massen

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-28 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/#review85613 --- src/examples/test_resource_estimator_module.cpp

Re: Review Request 33825: Added executor implementation for the new HTTP API

2015-05-28 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33825/#review85616 --- Is anything happening on this one? it's now been more than three wee

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-28 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/ --- (Updated May 28, 2015, 9:23 p.m.) Review request for mesos, Jie Yu, Niklas Niel

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-28 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/ --- (Updated May 28, 2015, 9:27 p.m.) Review request for mesos, Jie Yu, Niklas Niel

Re: Review Request 34782: Convert Non-POD static variables in fq_codel and ingress to constexpr

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

Re: Review Request 33823: Protocol file with the HTTP API messages between executor and slave

2015-05-28 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33823/#review85627 --- Ship it! Hey @vinod - I think Alex made all the suggested fixes: if

Re: Review Request 34137: Add support for container image provisioners.

2015-05-28 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/#review85624 --- src/slave/containerizer/mesos/containerizer.hpp

Re: Review Request 30612: Added /master/frameworks/{framework}/tasks/{task} endpoint.

2015-05-28 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30612/#review85628 --- Ship it! Folks - this thing has been out for almost four months now

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-05-28 Thread Adam B
> On May 14, 2015, 6:07 a.m., Alexander Rukletsov wrote: > > src/master/master.cpp, line 1315 > > > > > > This looks like a drive-by bug fix, should it be included in this diff? > > At least, let's mention it in the

Re: Review Request 34730: Added 'updateSlave()' in master to handle oversubscribed resources.

2015-05-28 Thread Vinod Kone
> On May 28, 2015, 12:02 a.m., Niklas Nielsen wrote: > > src/master/master.cpp, lines 3469-3470 > > > > > > Was there some additional math, only to rescind if the estimate is > > lower? If not now, should we add a to

Re: Review Request 34193: Refactored common functionality into FlagsBase

2015-05-28 Thread Marco Massenzio
> On May 28, 2015, 11:45 a.m., Benjamin Hindman wrote: > > 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp, line 517 > > > > > > This looks like a duplicated test with the 'Usage' test above? great catch! mod

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-05-28 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/ --- (Updated May 28, 2015, 4:13 p.m.) Review request for mesos, Ben Mahler and Nikl

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-05-28 Thread Adam B
> On May 21, 2015, 9:41 a.m., Niklas Nielsen wrote: > > Hey Adam, is this ready for review? Yup. Ready for another round of review again. - Adam --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/

Re: Review Request 34195: Refactoring to use FlagsBase common functionality

2015-05-28 Thread Marco Massenzio
> On May 28, 2015, 11:45 a.m., Benjamin Hindman wrote: > > src/cli/execute.cpp, line 321 > > > > > > I'm not convinced the 'errorMessage' is more help here. You're > > basically saving doing a 'return EXIT_FAILURE;' i

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

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

Re: Review Request 34729: Updated slave to send total amount of oversubscribed resources.

2015-05-28 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34729/ --- (Updated May 29, 2015, 12:30 a.m.) Review request for mesos, Jie Yu, Niklas Nie

Re: Review Request 34736: Implemented 'updateSlave()' call in the master.

2015-05-28 Thread Vinod Kone
> On May 28, 2015, 8:21 p.m., Niklas Nielsen wrote: > > src/master/master.cpp, line 3471 > > > > > > Do we use semi-colon in log lines? Have mostly seen ':' > > > > Want to add the resources which are now _no

Re: Review Request 34736: Implemented 'updateSlave()' call in the master.

2015-05-28 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34736/ --- (Updated May 29, 2015, 12:31 a.m.) Review request for mesos, Jie Yu, Joris Van

Re: Review Request 34736: Implemented 'updateSlave()' call in the master.

2015-05-28 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34736/#review85658 --- Ship it! Ship It! - Niklas Nielsen On May 28, 2015, 5:31 p.m., V

Re: Review Request 34193: Refactored common functionality into FlagsBase

2015-05-28 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34193/ --- (Updated May 29, 2015, 12:51 a.m.) Review request for mesos, Benjamin Hindman a

Re: Review Request 34729: Updated slave to send total amount of oversubscribed resources.

2015-05-28 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34729/#review85659 --- Ship it! Ship It! - Niklas Nielsen On May 28, 2015, 5:30 p.m., V

Re: Review Request 34195: Refactoring to use FlagsBase common functionality

2015-05-28 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34195/ --- (Updated May 29, 2015, 1:03 a.m.) Review request for mesos, Benjamin Hindman an

Re: Review Request 34195: Refactoring to use FlagsBase common functionality

2015-05-28 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34195/ --- (Updated May 29, 2015, 1:10 a.m.) Review request for mesos, Benjamin Hindman an

Re: Review Request 34736: Implemented 'updateSlave()' call in the master.

2015-05-28 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34736/#review85661 --- Patch looks great! Reviews applied: [34729, 34730, 34736] All test

Re: Review Request 34687: (De)Serializing MasterInfo PB to JSON

2015-05-28 Thread Marco Massenzio
> On May 27, 2015, 2:18 a.m., Niklas Nielsen wrote: > > src/common/protobuf_utils.hpp, line 83 > > > > > > Shouldn't parse() go in > > https://github.com/apache/mesos/blob/master/src/common/parse.hpp? > > Marco Massen

Re: Review Request 34195: Refactoring to use FlagsBase common functionality

2015-05-28 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34195/#review85671 --- Patch looks great! Reviews applied: [34193, 34195] All tests passe