Re: Review Request 30961: Enabled label decorator to override.

2015-04-07 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30961/#review79150 --- Minor suggestions. src/hook/manager.cpp

Re: Review Request 32585: Replaced Framework.id with Framework.id() in Master/Slave.

2015-04-07 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32585/#review79146 --- LGTM, assuming no significant rebases. src/master/master.hpp

Re: Review Request 30961: Enabled label decorator to override.

2015-04-07 Thread Adam B
On March 30, 2015, 3:23 a.m., Adam B wrote: Minor cleanup/suggestions, but otherwise good. We'll definitely need to document te module(-manager) API change of overriding the label set instead of merging. This should probably go in the upgrades doc? Niklas Nielsen wrote: Thanks

Re: Review Request 32583: Marked RunTaskMessage::framework_id as optional.

2015-04-07 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32583/#review79142 --- src/slave/slave.cpp

Re: Review Request 32586: Removed FrameworkID argument from Slave::_runTask.

2015-04-07 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32586/#review79147 --- src/slave/slave.cpp

Re: Review Request 31028: Added slave run task hook tests.

2015-04-07 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31028/#review79152 --- src/examples/test_hook_module.cpp

Re: Review Request 31776: Moved allocator to public headers.

2015-04-07 Thread Alexander Rukletsov
On April 2, 2015, 6:23 p.m., Vinod Kone wrote: include/mesos/master/allocator.proto, lines 19-23 https://reviews.apache.org/r/31776/diff/6/?file=912077#file912077line19 Since allocator is within the same Unix process as Master, what is the compatibility issue here? The comment

Re: Review Request 31263: Refactored TestAllocator and allocator text fixture.

2015-04-07 Thread Alexander Rukletsov
On April 2, 2015, 7:58 p.m., Vinod Kone wrote: src/tests/master_allocator_tests.cpp, lines 96-104 https://reviews.apache.org/r/31263/diff/7/?file=912106#file912106line96 While the TearDown() avoids flakiness by ensuring that an allocator process doesn't exist after a test, it

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-07 Thread Alexander Rukletsov
On April 2, 2015, 7:59 p.m., Vinod Kone wrote: src/master/allocator/mesos/allocator.hpp, line 47 https://reviews.apache.org/r/31265/diff/6/?file=912110#file912110line47 Do you still want the constructor public? I think there is no reason to hide the c-tor. I can imagine somebody

Re: Review Request 31776: Moved allocator to public headers.

2015-04-07 Thread Alexander Rukletsov
On April 2, 2015, 6:23 p.m., Vinod Kone wrote: src/Makefile.am, line 711 https://reviews.apache.org/r/31776/diff/6/?file=912078#file912078line711 not yours, but can you kill the trailing white space here? Sure. - Alexander

Re: Review Request 31267: Added a test allocator module.

2015-04-07 Thread Alexander Rukletsov
On April 2, 2015, 6:33 p.m., Vinod Kone wrote: src/Makefile.am, line 1351 https://reviews.apache.org/r/31267/diff/6/?file=912100#file912100line1351 s/drf// to be consistent (e.g., we didn't name libtestauthentication.la as libcrammd5authentication.la) Will do. On April 2,

Re: Review Request 31776: Moved allocator to public headers.

2015-04-07 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31776/ --- (Updated April 7, 2015, 12:46 p.m.) Review request for mesos, Kapil Arya,

Re: Review Request 31775: Removed Master::Flags dependency from Allocator.

2015-04-07 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31775/ --- (Updated April 7, 2015, 12:46 p.m.) Review request for mesos, Kapil Arya,

Re: Review Request 31266: Added support for allocator modules.

2015-04-07 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31266/ --- (Updated April 7, 2015, 12:47 p.m.) Review request for mesos, Kapil Arya,

Re: Review Request 31267: Added a test allocator module.

2015-04-07 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated April 7, 2015, 12:47 p.m.) Review request for mesos, Kapil Arya,

Re: Review Request 31262: Moved allocator actions before TestAllocator.

2015-04-07 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31262/ --- (Updated April 7, 2015, 12:48 p.m.) Review request for mesos, Kapil Arya,

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-07 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31265/ --- (Updated April 7, 2015, 12:49 p.m.) Review request for mesos, Kapil Arya,

Re: Review Request 31268: Wired up test allocator to allocator tests.

2015-04-07 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31268/ --- (Updated April 7, 2015, 12:49 p.m.) Review request for mesos, Kapil Arya,

Re: Review Request 31268: Wired up test allocator to allocator tests.

2015-04-07 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31268/#review79175 --- Patch looks great! Reviews applied: [31775, 31776, 31266, 31267,

Re: Suggestion: Mesos 0.22.1 point release

2015-04-07 Thread Adam Avilla
On Fri, Apr 3, 2015 at 3:47 PM, Niklas Nielsen nik...@mesosphere.io wrote: Based on input from Vinod and Adam; I will reduce the scope on the point release to focus on MESOS-1795 and MESOS-2583. Can I help test these in any way? -- /adam

Re: Review Request 32911: Fixed sandbox ownership bug for executors without URIs.

2015-04-07 Thread Niklas Nielsen
On April 6, 2015, 7:22 p.m., Benjamin Hindman wrote: src/slave/slave.cpp, line 4164 https://reviews.apache.org/r/32911/diff/1/?file=918555#file918555line4164 Great comment! Can we also add something to the end of the comment that says that the user is validated when the task goes

Re: Review Request 32586: Removed FrameworkID argument from Slave::_runTask.

2015-04-07 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32586/ --- (Updated April 7, 2015, 12:59 p.m.) Review request for mesos, Adam B and

Re: Review Request 32587: Stopped using RunTaskMessage.framework_id.

2015-04-07 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32587/ --- (Updated April 7, 2015, 1 p.m.) Review request for mesos, Adam B and Niklas

Re: Review Request 32700: Removed FrameworkID from FrameworkState.

2015-04-07 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32700/ --- (Updated April 7, 2015, 1 p.m.) Review request for mesos, Adam B and Niklas

Re: Review Request 32583: Marked RunTaskMessage::framework_id as optional.

2015-04-07 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32583/ --- (Updated April 7, 2015, 12:59 p.m.) Review request for mesos, Adam B and

Re: Review Request 32584: Do not pass FrameworkID to Framework constructor in Master/Slave.

2015-04-07 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32584/ --- (Updated April 7, 2015, 12:59 p.m.) Review request for mesos, Adam B and

Re: Review Request 32585: Replaced Framework.id with Framework.id() in Master/Slave.

2015-04-07 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32585/ --- (Updated April 7, 2015, 12:59 p.m.) Review request for mesos, Adam B and

Re: Review Request 32583: Marked RunTaskMessage::framework_id as optional.

2015-04-07 Thread Kapil Arya
On April 7, 2015, 4:06 a.m., Adam B wrote: src/slave/slave.cpp, line 1108 https://reviews.apache.org/r/32583/diff/4/?file=914631#file914631line1108 If we're always making the copy, should we just pass `frameworkInfo_` by value and force the copy at the call-site? Then there's no

Re: Review Request 32895: Environment variables are case sensitive

2015-04-07 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32895/#review79212 --- I couldn't verify the behavior detailed in the ticket? - Ian

Re: Review Request 32700: Removed FrameworkID from FrameworkState.

2015-04-07 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32700/#review79215 --- Patch looks great! Reviews applied: [32583, 32584, 32585, 32586,

Re: Review Request 32911: Fixed sandbox ownership bug for executors without URIs.

2015-04-07 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32911/#review79198 --- Ship it! src/slave/slave.cpp

Re: Review Request 32911: Fixed sandbox ownership bug for executors without URIs.

2015-04-07 Thread Niklas Nielsen
On April 7, 2015, 10:13 a.m., Ian Downes wrote: src/slave/slave.cpp, line 4166 https://reviews.apache.org/r/32911/diff/1/?file=918555#file918555line4166 getExecutorInfo() will return the ExecutorInfo if the TaskInfo includes it, otherwise it will construct one (for the command

Re: Review Request 32898: Eliminate use of 'echo -n'

2015-04-07 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32898/#review79209 --- Ship it! Ship It! - Ian Downes On April 6, 2015, 3:05 p.m.,

Re: Review Request 32903: Eliminate the use of 'echo -n' in EC2 scripts

2015-04-07 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32903/#review79208 --- Ship it! Ship It! - Ian Downes On April 6, 2015, 3:09 p.m.,

Re: Review Request 32906: Add safety check for staged but uncommitted changes

2015-04-07 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32906/#review79206 --- Ship it! Ship It! - Ian Downes On April 6, 2015, 4:03 p.m.,

Re: Review Request 32911: Fixed sandbox ownership bug for executors without URIs.

2015-04-07 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32911/ --- (Updated April 7, 2015, 11:10 a.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 32911: Fixed sandbox ownership bug for executors without URIs.

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

Question on Docker Containerizer

2015-04-07 Thread Stephen Salinas
Hey Everyone, I have a question on using the docker containerizer. I have a feeling it isn’t possible, but, is there a way to use the docker containerizer within a custom executor? (ie. have it help translate commands to docker and manage the containers within the wrapper of the custom

Re: Review Request 32139: Add 'Resource::ReservationInfo' protobuf message.

2015-04-07 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32139/ --- (Updated April 7, 2015, 9:51 p.m.) Review request for mesos, Alexander

Re: Review Request 32139: Add 'Resource::ReservationInfo' protobuf message.

2015-04-07 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32139/ --- (Updated April 7, 2015, 9:52 p.m.) Review request for mesos, Alexander

Re: Review Request 32140: Enable 'Resources' to handle 'Resource::ReservationInfo'.

2015-04-07 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32140/ --- (Updated April 7, 2015, 9:56 p.m.) Review request for mesos, Alexander

Re: Review Request 32149: Enable 'Resources::apply' to handle reservation operations.

2015-04-07 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32149/ --- (Updated April 7, 2015, 10:25 p.m.) Review request for mesos, Alexander

Re: Regarding old frameworks in Mesos repository

2015-04-07 Thread Yan Xu
You are right. Then the question is which CLI should it go to? Seems like src/cli is superseded by https://github.com/mesosphere/mesos-cli too and should be on the spring cleaning list? I personally don't mind cli to be inside the core source tree but it seems like

Review Request 32939: Bumped the log level for dropped messages.

2015-04-07 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32939/ --- Review request for mesos, Ben Mahler and Cody Maloney. Repository: mesos

Re: Review Request 29748: Added tests for dynamic reservation.

2015-04-07 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29748/ --- (Updated April 7, 2015, 9:44 p.m.) Review request for mesos, Alexander

Re: Review Request 32939: Bumped the log level for dropped messages.

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

Re: Review Request 31645: Added upgrade path test script

2015-04-07 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31645/#review79271 --- support/test-upgrade.py

Re: Review Request 31016: Added slave run task decorator.

2015-04-07 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31016/ --- (Updated April 7, 2015, 5:57 p.m.) Review request for mesos, Ben Mahler and

Re: Review Request 30961: Enabled label decorator to override.

2015-04-07 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30961/ --- (Updated April 7, 2015, 5:57 p.m.) Review request for mesos, Ben Mahler and

Re: Review Request 30962: Enabled environment decorator to override.

2015-04-07 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30962/ --- (Updated April 7, 2015, 5:57 p.m.) Review request for mesos, Ben Mahler and

Re: Review Request 31028: Added slave run task hook tests.

2015-04-07 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31028/ --- (Updated April 7, 2015, 5:57 p.m.) Review request for mesos, Ben Mahler and

Review Request 32948: Refactored VerifyMasterLaunchTaskHook to _not_ use command executor.

2015-04-07 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32948/ --- Review request for mesos, Adam B and Kapil Arya. Repository: mesos

Re: Review Request 31017: Fixed comment for remove executor hook.

2015-04-07 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31017/ --- (Updated April 7, 2015, 5:58 p.m.) Review request for mesos and Kapil Arya.

Re: Review Request 30961: Enabled label decorator to override.

2015-04-07 Thread Niklas Nielsen
On April 7, 2015, 1:41 a.m., Adam B wrote: src/hook/manager.cpp, line 104 https://reviews.apache.org/r/30961/diff/6/?file=914058#file914058line104 Would it make sense to make taskInfo a pass-by-value param, forcing the copy at the call? That unfortunately changes the module API

Re: Review Request 30961: Enabled label decorator to override.

2015-04-07 Thread Niklas Nielsen
On April 7, 2015, 1:41 a.m., Adam B wrote: Minor suggestions. Thanks for the review Adam! - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30961/#review79150

Re: Review Request 31016: Added slave run task decorator.

2015-04-07 Thread Niklas Nielsen
On March 30, 2015, 3:44 a.m., Adam B wrote: src/slave/slave.cpp, line 1173 https://reviews.apache.org/r/31016/diff/3/?file=894746#file894746line1173 +1 to introducing this as high up in the method as possible, to reduce risk of using the wrong taskInfo in future nearby calls.

Re: Review Request 31016: Added slave run task decorator.

2015-04-07 Thread Niklas Nielsen
On Feb. 22, 2015, 4:48 p.m., Kapil Arya wrote: src/slave/slave.cpp, line https://reviews.apache.org/r/31016/diff/1/?file=863935#file863935line Can we instead swap its name with `task_` and move the label decorator higher (right after entry into this function)?

Re: Review Request 32948: Refactored VerifyMasterLaunchTaskHook to _not_ use command executor.

2015-04-07 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32948/ --- (Updated April 7, 2015, 5:57 p.m.) Review request for mesos, Adam B and Kapil

Re: Review Request 32939: Bumped the log level for dropped messages.

2015-04-07 Thread Ben Mahler
On April 8, 2015, 1:13 a.m., Ben Mahler wrote: 3rdparty/libprocess/src/process.cpp, line 2048 https://reviews.apache.org/r/32939/diff/1/?file=920067#file920067line2048 While you're here, how about: ``` VLOG(2) Dropping event for process: to; ```

Re: Review Request 32956: Simplified MemUsage test.

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

Re: Review Request 32955: Simplified ROOT_CGROUPS_Listen test.

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

Re: Review Request 32939: Bumped the log level for dropped messages.

2015-04-07 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32939/#review79288 --- Ship it! 3rdparty/libprocess/src/process.cpp

Review Request 32955: Simplified ROOT_CGROUPS_Listen test.

2015-04-07 Thread Chi Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32955/ --- Review request for mesos, Ian Downes and Jie Yu. Bugs: mesos-2573

Review Request 32956: Simplified MemUsage test.

2015-04-07 Thread Chi Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32956/ --- Review request for mesos, Ian Downes and Jie Yu. Bugs: mesos-2573

Review Request 32954: Added a 'slave_shutdowns_completed' metric.

2015-04-07 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32954/ --- Review request for mesos and Vinod Kone. Repository: mesos Description

Re: Review Request 32954: Added a 'slave_shutdowns_completed' metric.

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