Re: Review Request 32999: Added a document for engineering principles and practices.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32999/#review81429 --- Patch looks great! Reviews applied: [32995, 32996, 32997, 32998, 32

Re: Review Request 28257: Allow prefix paths in libprocess

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28257/ --- (Updated April 23, 2015, 11:32 p.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 32999: Added a document for engineering principles and practices.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32999/ --- (Updated April 23, 2015, 11:15 p.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 32998: Split committer's guide into code reviewing and committing documents.

> On April 20, 2015, 7:28 p.m., Niklas Nielsen wrote: > > docs/effective-code-reviewing.md, line 19 > > > > > > s/, this is not meant to be a sparring match!/./ Hm.. this was copy/pasted from benh's original committer'

Re: Review Request 32998: Split committer's guide into code reviewing and committing documents.

> On April 9, 2015, 12:51 a.m., Adam B wrote: > > docs/committing.md, line 18 > > > > > > Would like to formalize what kinds of changes you "don't worry about > > going through a review cycle". I'd propose that typo/co

Re: Review Request 32998: Split committer's guide into code reviewing and committing documents.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32998/ --- (Updated April 23, 2015, 11:06 p.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 32998: Split committer's guide into code reviewing and committing documents.

> On April 13, 2015, 6:30 p.m., haosdent huang wrote: > > docs/effective-code-reviewing.md, line 14 > > > > > > Should we add a requirement that "pass unit tests before post reviews" > > here? Thanks! Haven't seen thi

Re: Review Request 32999: Added a document for engineering principles and practices.

> On April 20, 2015, 7:32 p.m., Niklas Nielsen wrote: > > docs/engineering-principles-and-practices.md, line 7 > > > > > > Long lines :) Do you think it is worth applying the 80 col style? If > > so, we should do a scan

Re: Review Request 33491: Set the supplementary groups list when switching users.

> On April 23, 2015, 9:16 p.m., Ben Mahler wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp, line 730 > > > > > > I'm curious why you look at EPERM here, but not in the call to > > `::setgid` above a

Re: Review Request 33491: Set the supplementary groups list when switching users.

> On April 23, 2015, 9:16 p.m., Ben Mahler wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp, line 730 > > > > > > I'm curious why you look at EPERM here, but not in the call to > > `::setgid` above a

Re: Review Request 33193: Warn if g++ < 4.8, C++ standard library is too old

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33193/#review81414 --- Ship it! I'll add the period and get this committed! configure.ac

Re: Review Request 33193: Warn if g++ < 4.8, C++ standard library is too old

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33193/ --- (Updated April 23, 2015, 9:38 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 33491: Set the supplementary groups list when switching users.

> On April 23, 2015, 9:16 p.m., Ben Mahler wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp, line 730 > > > > > > I'm curious why you look at EPERM here, but not in the call to > > `::setgid` above a

Re: Review Request 33491: Set the supplementary groups list when switching users.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33491/#review81407 --- Patch looks great! Reviews applied: [33491] All tests passed. - M

Re: Review Request 33491: Set the supplementary groups list when switching users.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33491/#review81404 --- Thanks James! 3rdparty/libprocess/3rdparty/stout/include/stout/os.

Re: Review Request 32150: Enabled the master to handle reservation operations.

> On April 8, 2015, 10:38 p.m., Jie Yu wrote: > > src/master/validation.cpp, line 61 > > > > > > Do you want to add a `CHECK_NE(resource.role(), "*");` here. > > > > That makes me wonder should we move the chec

Re: Review Request 33490: MESOS-2633 Avoid adding duplicate tasks

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33490/#review81394 --- Patch looks great! Reviews applied: [33376, 33490] All tests passe

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

> 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 32140: Enabled 'Resources' to handle 'Resource::ReservationInfo'.

> On April 23, 2015, 5:35 p.m., Jie Yu wrote: > > src/common/resources.cpp, lines 445-449 > > > > > > See my comments in https://reviews.apache.org/r/32150/ > > > > Can we move this to master validation? Thoug

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

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

Review Request 33491: Set the supplementary groups list when switching users.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33491/ --- Review request for mesos. Bugs: MESOS-719 https://issues.apache.org/jira/br

Re: Review Request 33490: MESOS-2633 Avoid adding duplicate tasks

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33490/ --- (Updated April 23, 2015, 8:03 p.m.) Review request for mesos, Adam B and Joris

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

> On April 23, 2015, 5:35 p.m., Jie Yu wrote: > > src/tests/resources_tests.cpp, line 835 > > > > > > Move this to src/tests/mesos.hpp close to createPersistentVolume? I believe you're referring to `createDiskResource

Re: Review Request 33490: MESOS-2633 Avoid adding duplicate tasks

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33490/#review81379 --- Bad patch! Reviews applied: [33490] Failed command: ./support/appl

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32140/#review81378 --- Ship it! Discussed with Mpark offline. We agreed that rule for Reso

Review Request 33490: MESOS-2633 Avoid adding duplicate tasks

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33490/ --- Review request for mesos, Adam B and Joris Van Remoortere. Repository: mesos

Re: Review Request 32996: Updated roadmap document to link to 'Epic' tickets.

> On April 20, 2015, 6:56 p.m., Niklas Nielsen wrote: > > docs/mesos-roadmap.md, line 11 > > > > > > It sounds a bit hostile (it may just be me), how about something like > > "For comments and suggestions for the Mesos

Re: Review Request 32996: Updated roadmap document to link to 'Epic' tickets.

> On April 20, 2015, 7:01 p.m., Dave Lester wrote: > > Since you're updating this doc, can you add a link to the release planning > > schedule? > > https://cwiki.apache.org/confluence/display/MESOS/Mesos+Release+Planning > > Seems directly related to this. Will do. - Ben -

Re: Review Request 33241: docs: Add documentation on observability metrics.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33241/#review81373 --- Thanks for sending this! Can you include a link to the rendered mar

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33465/#review81372 --- src/scheduler/scheduler.cpp

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

> 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. > > Michael Park wrote: > I kept

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32149/#review81364 --- src/tests/resources_tests.cpp

Re: Review Request 33295: Added firewall mechanism to control access on libprocess http endpoints.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33295/#review81357 --- 3rdparty/libprocess/src/process.cpp

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32149/#review81349 --- Ship it! Ship It! - Timothy Chen On April 23, 2015, 4:38 p.m., M

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

> On April 23, 2015, 1:48 p.m., Michael Park wrote: > > As of now, the tests seem to take a long time to complete. We should > > investigate what the issue is before committing this patch. I suspect this is due to the default allocation interval (1 secs by default). - Jie --

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32140/#review81342 --- src/common/resources.cpp

Re: Review Request 33296: Added a flag which controls libprocess firewall initialzation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33296/#review81343 --- src/common/parse.hpp

Re: Review Request 33295: Added firewall mechanism to control access on libprocess http endpoints.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33295/#review81327 --- 3rdparty/libprocess/include/process/firewall.hpp

Re: Review Request 33296: Added a flag which controls libprocess firewall initialzation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33296/#review81340 --- src/common/parse.hpp

Re: Review Request 33296: Added a flag which controls libprocess firewall initialzation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33296/#review81337 --- src/master/flags.cpp

Re: Review Request 33295: Added firewall mechanism to control access on libprocess http endpoints.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33295/#review81334 --- 3rdparty/libprocess/src/process.cpp

Re: Review Request 33295: Added firewall mechanism to control access on libprocess http endpoints.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33295/#review81329 --- Please move the `*` for pointers to the left as per our style guide:

Re: Review Request 33295: Added firewall mechanism to control access on libprocess http endpoints.

> On April 23, 2015, 5:49 a.m., Till Toenshoff wrote: > > 3rdparty/libprocess/include/process/firewall.hpp, line 37 > > > > > > I forgot the verbally proposed name for this function, but maybe a > > negated result and

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32149/ --- (Updated April 23, 2015, 4:38 p.m.) Review request for mesos, Alexander Ruklets

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

> 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. > > Michael Park wrote: > I kept

Re: Review Request 33263: Extended SlaveTest.ShutdownUnregisteredExecutor test with a reason check.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33263/#review81320 --- Ship it! Ship It! - Till Toenshoff On April 16, 2015, 2:31 p.m.,

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

and='echo $a' > ``` > > The log from mesos, we could see the output is empty > > ``` > Registered executor on localhostI0423 01:27:37.180140 14061 slave.cpp:1540] > Sending queued task 'test' to executor 'test' of framework > 20150423-012731-1677734

Re: Review Request 33296: Added a flag which controls libprocess firewall initialzation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33296/#review81308 --- include/mesos/mesos.proto

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29748/#review81298 --- How about a test trying to unreserve statically reserved resources?

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29748/#review81311 --- As of now, the tests seem to take a long time to complete. We should

Re: Review Request 33295: Added firewall mechanism to control access on libprocess http endpoints.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33295/#review80582 --- This review was pending a couple of days in my outbox, hence it may

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33358/#review81304 --- Ship it! Ship It! - Till Toenshoff On April 22, 2015, 9:09 a.m.,

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

/parse.hpp 547b32041f39f0ff0c38179b66a32b2239134abc > > Diff: https://reviews.apache.org/r/33109/diff/ > > > Testing > --- > > ### Test without env > > ``` > $ ./src/mesos-execute --master=127.0.0.1:5050 --name=test --command='echo $a' > ``` > > The l

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

> 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 33257: Fixed recover tasks only by the intiated containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33257/#review81287 --- Looks reasonable to me, but I'm no containerizer expert. src/slave

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

> On April 22, 2015, 11: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

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

; ``` > $ ./src/mesos-execute --master=127.0.0.1:5050 --name=test --command='echo $a' > ``` > > The log from mesos, we could see the output is empty > > ``` > Registered executor on localhostI0423 01:27:37.180140 14061 slave.cpp:1540] > Sending queued task 'test'

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33452/#review81282 --- Ship it! Ship It! - Alexander Rojas On April 22, 2015, 11:53 p.m