Re: Review Request 33465: Removed 'uuid' field from UPDATE call.

2015-04-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33465/ --- (Updated April 23, 2015, 5:48 a.m.) Review request for mesos and Ben Mahler.

Re: Review Request 33274: Fix capture by reference of temporary strings in Libprocess.

2015-04-22 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33274/#review81267 --- Is it possible to do some quick benchmarking to show that this chang

Re: Review Request 32509: Documented the scheduler Event/Call protobufs.

2015-04-22 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32509/#review81266 --- Patch looks great! Reviews applied: [32500, 32501, 32502, 32504, 32

Re: Review Request 32509: Documented the scheduler Event/Call protobufs.

2015-04-22 Thread Vinod Kone
> On April 22, 2015, 11:26 p.m., Ben Mahler wrote: > > include/mesos/scheduler/scheduler.proto, line 52 > > > > > > Hm, what do you mean by "can be offered"? Do you want to just s/that > > can be// ? done. > On Apri

Re: Review Request 32509: Documented the scheduler Event/Call protobufs.

2015-04-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32509/ --- (Updated April 23, 2015, 2:55 a.m.) Review request for mesos and Ben Mahler.

Review Request 33465: Removed 'uuid' field from UPDATE call.

2015-04-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33465/ --- Review request for mesos and Ben Mahler. Bugs: MESOS-1127 https://issues.ap

Re: Review Request 32845: Renamed UNREGISTER call to TEARDOWN.

2015-04-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32845/ --- (Updated April 23, 2015, 2:52 a.m.) Review request for mesos and Ben Mahler.

Re: Review Request 32845: Renamed UNREGISTER call to UNSUBSCRIBE.

2015-04-22 Thread Vinod Kone
> On April 22, 2015, 10:48 p.m., Ben Mahler wrote: > > include/mesos/scheduler/scheduler.proto, line 113 > > > > > > And removes the framework? > > > > Do you want to align the comments so the `//` characters

Re: Review Request 32844: Added SUBSCRIBE call and SUBSCRIBED event.

2015-04-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32844/ --- (Updated April 23, 2015, 2:51 a.m.) Review request for mesos and Ben Mahler.

Re: Review Request 32843: Updated KILL to include SlaveID.

2015-04-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32843/ --- (Updated April 23, 2015, 2:51 a.m.) Review request for mesos and Ben Mahler.

Re: Review Request 32843: Updated KILL to include SlaveID.

2015-04-22 Thread Vinod Kone
> On April 22, 2015, 10:20 p.m., Ben Mahler wrote: > > src/master/master.cpp, lines 2731-2732 > > > > > > You may want to avoid taking a const reference to the temporary here? fixed. > On April 22, 2015, 10:20 p.m.

Re: Review Request 33257: Fixed recover tasks only by the intiated containerizer.

2015-04-22 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33257/#review81257 --- Ship it! src/slave/slave.cpp

Re: Review Request 33459: Fix containerizers to only recover containers they would launch.

2015-04-22 Thread Benjamin Hindman
> On April 23, 2015, 12:37 a.m., Timothy Chen wrote: > > I already have a fix out for this: > > https://reviews.apache.org/r/33257/ > > > > My patch also handles docker container tasks, since the docker > > containerizer uses the command executor in docker tasks, and without more > > handling

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

2015-04-22 Thread Jie Yu
> On April 8, 2015, 8:16 p.m., Jie Yu wrote: > > src/common/resources.cpp, lines 450-459 > > > > > > The semantics of this function becomes a little weired now. For > > example, for a resource that has `role == "*"` a

Re: Review Request 33459: Fix containerizers to only recover containers they would launch.

2015-04-22 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33459/#review81251 --- Subsumed by this https://reviews.apache.org/r/33257? - Jie Yu On

Re: Review Request 33459: Fix containerizers to only recover containers they would launch.

2015-04-22 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33459/#review81252 --- I already have a fix out for this: https://reviews.apache.org/r/3325

Re: Review Request 33358: Moved implementation of StatusUpdateStream to a compilation unit.

2015-04-22 Thread Ben Mahler
> On April 22, 2015, 9:24 a.m., Alexander Rukletsov wrote: > > Let's not repeat summary in description. How about the following: > > "StatusUpdateStream is not a template, hence we can reduce compilation time > > by moving method definitions into a `.cpp` file. As a drive-by change > > headers

Review Request 33459: Fix containerizers to only recover containers they would launch.

2015-04-22 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33459/ --- Review request for mesos, Joris Van Remoortere, Michael Park, and Timothy Chen.

Re: Review Request 32509: Documented the scheduler Event/Call protobufs.

2015-04-22 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32509/#review81234 --- Ship it! Looks great, only 1 question which we can tackle separatel

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

2015-04-22 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29889/#review81235 --- Bad patch! Reviews applied: [29327, 29328] Failed command: ./suppo

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-04-22 Thread Jie Yu
> On April 21, 2015, 11:25 p.m., Jie Yu wrote: > > src/slave/slave.cpp, lines 3065-3078 > > > > > > Instead of doing that in your way, can we just try to make sure > > `containerizer->wait` here will return a failure

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

2015-04-22 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29889/ --- (Updated April 22, 2015, 10:50 p.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 32845: Renamed UNREGISTER call to UNSUBSCRIBE.

2015-04-22 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32845/#review81232 --- Ship it! TEARDOWN sounds good, thanks! include/mesos/scheduler/sc

Re: Review Request 32844: Added SUBSCRIBE call and SUBSCRIBED event.

2015-04-22 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32844/#review81227 --- Ship it! src/examples/low_level_scheduler_libprocess.cpp

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-04-22 Thread Jay Buffington
> On April 21, 2015, 4:25 p.m., Jie Yu wrote: > > src/slave/slave.cpp, lines 3065-3078 > > > > > > Instead of doing that in your way, can we just try to make sure > > `containerizer->wait` here will return a failure

Re: Review Request 32509: Documented the scheduler Event/Call protobufs.

2015-04-22 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32509/#review81225 --- Patch looks great! Reviews applied: [32500, 32501, 32502, 32504, 32

Re: Review Request 32843: Updated KILL to include SlaveID.

2015-04-22 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32843/#review81222 --- Ship it! src/master/master.cpp

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

2015-04-22 Thread Michael Park
> On April 8, 2015, 8:16 p.m., Jie Yu wrote: > > src/common/resources.cpp, lines 450-459 > > > > > > The semantics of this function becomes a little weired now. For > > example, for a resource that has `role == "*"` a

Re: Review Request 33452: Fixed the python bindings to use implicit acknoweldgements by default.

2015-04-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33452/#review81221 --- Ship it! Ship It! - Vinod Kone On April 22, 2015, 9:53 p.m., Ben

Review Request 33452: Fixed the python bindings to use implicit acknoweldgements by default.

2015-04-22 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33452/ --- Review request for mesos, Alexander Rojas and Vinod Kone. Bugs: MESOS-2643

Re: Review Request 32506: Added output stream operators for scheduler Calls and Events.

2015-04-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32506/ --- (Updated April 22, 2015, 9:46 p.m.) Review request for mesos and Ben Mahler.

Re: Review Request 32509: Documented the scheduler Event/Call protobufs.

2015-04-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32509/ --- (Updated April 22, 2015, 9:44 p.m.) Review request for mesos and Ben Mahler.

Re: Review Request 32845: Renamed UNREGISTER call to UNSUBSCRIBE.

2015-04-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32845/ --- (Updated April 22, 2015, 9:43 p.m.) Review request for mesos and Ben Mahler.

Re: Review Request 32844: Added SUBSCRIBE call and SUBSCRIBED event.

2015-04-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32844/ --- (Updated April 22, 2015, 9:42 p.m.) Review request for mesos and Ben Mahler.

Re: Review Request 32845: Renamed UNREGISTER call to UNSUBSCRIBE.

2015-04-22 Thread Vinod Kone
> On April 21, 2015, 1:23 a.m., Ben Mahler wrote: > > include/mesos/scheduler/scheduler.proto, line 113 > > > > > > Is UNSUBSCRIBE the right name for this? It seems like "unsubscribe" > > would stop the subscription f

Re: Review Request 32505: Added SHUTDOWN scheduler call.

2015-04-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32505/ --- (Updated April 22, 2015, 9:41 p.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 32844: Added SUBSCRIBE call and SUBSCRIBED event.

2015-04-22 Thread Vinod Kone
> On April 20, 2015, 11:18 p.m., Ben Mahler wrote: > > include/mesos/scheduler/scheduler.proto, line 117 > > > > > > Might help to have a hint pointing to the framework id semantics: > > > > ``` > > SUBSCR

Re: Review Request 32843: Updated KILL to include SlaveID.

2015-04-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32843/ --- (Updated April 22, 2015, 9:40 p.m.) Review request for mesos and Ben Mahler.

Re: Review Request 33448: Added Eclipse-specific files to the gitignore list

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

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

2015-04-22 Thread Michael Park
> On April 15, 2015, 5:36 p.m., Alexander Rukletsov wrote: > > src/common/resources.cpp, lines 93-105 > > > > > > Can we replace it by > > ``` > > if !addable(left, right) { > > return false; > > } > >

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

2015-04-22 Thread Michael Park
> On April 15, 2015, 5:36 p.m., Alexander Rukletsov wrote: > > src/tests/mesos.hpp, lines 376-378 > > > > > > Why not putting the definition into `tests/mesos.cpp`? Here and below. I kept it consistent with other util

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

2015-04-22 Thread Michael Park
> On April 22, 2015, 12:14 p.m., Alexander Rukletsov wrote: > > src/common/resources.cpp, lines 44-49 > > > > > > Won't this be equivalent to the auto-generated version? I'm not sure what auto-generated version you're

Review Request 33448: Added Eclipse-specific files to the gitignore list

2015-04-22 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33448/ --- Review request for mesos and Joris Van Remoortere. Repository: mesos Descript

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

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

Re: Review Request 33376: MESOS-2633 Moved struct Framework methods to their own implementation class.

2015-04-22 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33376/ --- (Updated April 22, 2015, 8:35 p.m.) Review request for mesos and Joris Van Remo

Re: Review Request 32975: MESOS-1790 Adds chown option to CommandInfo.URI

2015-04-22 Thread Jim Klucar
> On April 8, 2015, 9:57 p.m., Vinod Kone wrote: > > src/tests/fetcher_tests.cpp, lines 112-116 > > > > > > Add a note/todo here mentioning why you are setting these fields but > > not doing any asserts/expects on the

Re: Review Request 33446: Link to mesos-containerizer from the main documentation page.

2015-04-22 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33446/#review81208 --- Ship it! Thanks James! - Ben Mahler On April 22, 2015, 7:22 p.m.

Re: Review Request 33275: Fix capture by reference of temporaries in Stout.

2015-04-22 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33275/#review81206 --- Patch looks great! Reviews applied: [33271, 33272, 33275] All test

Re: Review Request 33446: Link to mesos-containerizer from the main documentation page.

2015-04-22 Thread Joerg Schad
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33446/#review81204 --- LGTM docs/home.md

Re: Review Request 33446: Link to mesos-containerizer from the main documentation page.

2015-04-22 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33446/ --- (Updated April 22, 2015, 7:22 p.m.) Review request for mesos and Joerg Schad.

Review Request 33446: Link to mesos-containerizer from the main documentation page.

2015-04-22 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33446/ --- Review request for mesos and Joerg Schad. Bugs: TS-2524 https://issues.apac

Re: Review Request 33376: MESOS-2633 Moved struct Framework methods to their own implementation class.

2015-04-22 Thread Marco Massenzio
> On April 21, 2015, 3:27 a.m., Joris Van Remoortere wrote: > > .gitignore-template, lines 22-33 > > > > > > I think this got added to your commit accidentaly? If you want to add > > this to the repository we should ma

Re: Review Request 33276: Fix capture by reference of temporaries in Libprocess.

2015-04-22 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33276/#review81202 --- Patch looks great! Reviews applied: [33271, 33274, 33276] All test

Re: Review Request 33271: Update style guide to disallow capturing temporaries by reference.

2015-04-22 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33271/ --- (Updated April 22, 2015, 6:36 p.m.) Review request for mesos, Bernd Mathiske, C

Re: Review Request 33271: Update style guide to disallow capturing temporaries by reference.

2015-04-22 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33271/#review81196 --- docs/mesos-c++-style-guide.md

Re: Review Request 33109: Allow setting environment variables in mesos-execute

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

Re: Review Request 33276: Fix capture by reference of temporaries in Libprocess.

2015-04-22 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33276/ --- (Updated April 22, 2015, 6:11 p.m.) Review request for mesos, Bernd Mathiske, C

Re: Review Request 33275: Fix capture by reference of temporaries in Stout.

2015-04-22 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33275/ --- (Updated April 22, 2015, 6:11 p.m.) Review request for mesos, Bernd Mathiske, C

Re: Review Request 33272: Fix capture by reference of temporary strings in Stout.

2015-04-22 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33272/ --- (Updated April 22, 2015, 6:11 p.m.) Review request for mesos, Bernd Mathiske, C

Re: Review Request 33274: Fix capture by reference of temporary strings in Libprocess.

2015-04-22 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33274/ --- (Updated April 22, 2015, 6:11 p.m.) Review request for mesos, Bernd Mathiske, C

Re: Review Request 33271: Update style guide to disallow capturing temporaries by reference.

2015-04-22 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33271/ --- (Updated April 22, 2015, 6:11 p.m.) Review request for mesos, Bernd Mathiske, C

Re: Review Request 32975: MESOS-1790 Adds chown option to CommandInfo.URI

2015-04-22 Thread Adam B
> On April 8, 2015, 2:57 p.m., Vinod Kone wrote: > > src/tests/fetcher_tests.cpp, lines 112-116 > > > > > > Add a note/todo here mentioning why you are setting these fields but > > not doing any asserts/expects on the

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-04-22 Thread Jie Yu
> On April 21, 2015, 11:25 p.m., Jie Yu wrote: > > src/slave/slave.cpp, lines 3065-3078 > > > > > > Instead of doing that in your way, can we just try to make sure > > `containerizer->wait` here will return a failure

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

2015-04-22 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29889/ --- (Updated April 22, 2015, 5:33 p.m.) Review request for mesos and Benjamin Hindm

Re: Review Request 33109: Allow setting environment variables in mesos-execute

2015-04-22 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33109/ --- (Updated April 22, 2015, 5:38 p.m.) Review request for mesos and Adam B. Bugs

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

2015-04-22 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29889/#review81180 --- Bad patch! Reviews applied: [29327, 29328] Failed command: ./suppo

Re: Review Request 33109: Allow setting environment variables in mesos-execute

2015-04-22 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33109/ --- (Updated April 22, 2015, 5:37 p.m.) Review request for mesos and Adam B. Bugs

Re: Review Request 33109: Allow setting environment variables in mesos-execute

2015-04-22 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33109/ --- (Updated April 22, 2015, 5:35 p.m.) Review request for mesos and Adam B. Bugs

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-04-22 Thread Jay Buffington
> On April 21, 2015, 4:25 p.m., Jie Yu wrote: > > src/slave/slave.cpp, lines 3065-3078 > > > > > > Instead of doing that in your way, can we just try to make sure > > `containerizer->wait` here will return a failure