Re: Review Request 40114: Windows: Began adding Windows support to `process/future.hpp`

2015-12-01 Thread Daniel Pravat
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40114/ --- (Updated Dec. 1, 2015, 8:38 p.m.) Review request for mesos, Artem Harutyunyan,

Re: Review Request 40115: Windows: Added support for `slave/gc.cpp'.

2015-12-01 Thread Daniel Pravat
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40115/ --- (Updated Dec. 1, 2015, 9:06 p.m.) Review request for mesos, Artem Harutyunyan,

Re: Review Request 40818: Fixed flaky test (AvailableResourcesAfterRescinding).

2015-12-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40818/#review108553 --- Patch looks great! Reviews applied: [40818] Passed command:

Re: Review Request 40620: Windows: Added suppport for `slave/monitor.cpp'.

2015-12-01 Thread Daniel Pravat
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40620/ --- (Updated Dec. 1, 2015, 9:02 p.m.) Review request for mesos, Artem Harutyunyan,

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-01 Thread Greg Mann
> On Dec. 1, 2015, 2:24 p.m., Michael Park wrote: > > src/master/master.cpp, line 2771 > > > > > > Why is it that we need to perform validation within authorization? We > > perform `authorizeTask` with a

Re: Review Request 40799: Removed non-ASCII characters from test case comment.

2015-12-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40799/#review108558 --- Patch looks great! Reviews applied: [40799] Passed command:

Re: Review Request 38234: Check if swap is enabled before running memory pressure related tests.

2015-12-01 Thread Vinod Kone
> On Nov. 17, 2015, 7:27 p.m., Vinod Kone wrote: > > src/tests/containerizer/cgroups_tests.cpp, lines 560-568 > > > > > > instead of asserting, it would be better if we can disable the test > > automatically if we

Re: Review Request 40618: Include the header for boost::hash_combine explicitly

2015-12-01 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40618/#review108561 --- Ship it! Ship It! - Vinod Kone On Nov. 23, 2015, 10:42 p.m.,

Re: Review Request 40822: Updated the pattern for `_teardown` authorization continuation.

2015-12-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40822/#review108574 --- Patch looks great! Reviews applied: [40822] Passed command:

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-01 Thread Greg Mann
> On Dec. 1, 2015, 2:24 p.m., Michael Park wrote: > > src/master/master.cpp, line 2771 > > > > > > Why is it that we need to perform validation within authorization? We > > perform `authorizeTask` with a

Re: Review Request 40620: Windows: Added suppport for `slave/monitor.cpp'.

2015-12-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40620/#review108593 --- Patch looks great! Reviews applied: [40114, 40620] Passed

Re: Review Request 40224: Fix wrong flags infos in /state and /flags

2015-12-01 Thread Jian Qiu
> On Nov. 23, 2015, 4:08 p.m., haosdent huang wrote: > > I think keep the sync of these files when flags add or delete Protobuf > > fields a bit hard. Is it possible to change flags stringify protobuf > > objects as json string Thanks hasdent. I do not think we can overload or have a

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-01 Thread Jie Yu
> On Dec. 1, 2015, 2:24 p.m., Michael Park wrote: > > src/master/master.cpp, line 2771 > > > > > > Why is it that we need to perform validation within authorization? We > > perform `authorizeTask` with a

Review Request 40849: [WIP] Fix flaky MemoryPressureMesosTests

2015-12-01 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40849/ --- Review request for mesos, Bernd Mathiske, Artem Harutyunyan, and Till Toenshoff.

Re: Review Request 40829: Improved authorization documentation.

2015-12-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40829/#review108586 --- Patch looks great! Reviews applied: [40829] Passed command:

Re: Review Request 40115: Windows: Added support for `slave/gc.cpp'.

2015-12-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40115/#review108597 --- Bad patch! Reviews applied: [40114, 39537, 39538, 39539, 39540,

Re: Review Request 40849: [WIP] Fix flaky MemoryPressureMesosTests

2015-12-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40849/#review108601 --- Patch looks great! Reviews applied: [40849] Passed command:

Re: Review Request 40818: Fixed flaky test (AvailableResourcesAfterRescinding).

2015-12-01 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40818/ --- (Updated Dec. 1, 2015, 4:52 p.m.) Review request for mesos, Joris Van

Re: Review Request 40799: Removed non-ASCII characters from test case comment.

2015-12-01 Thread Alexander Rukletsov
> On Dec. 1, 2015, 1:49 p.m., Alexander Rukletsov wrote: > > I propose to replace the comment in https://reviews.apache.org/r/40818/ > > > > Just curious, how did you spot this? it looks fine in my editor. Also, is > > it something we should add as a commit hook? > > Neil Conway wrote: >

Re: Review Request 40799: Removed non-ASCII characters from test case comment.

2015-12-01 Thread Alexander Rukletsov
> On Dec. 1, 2015, 1:49 p.m., Alexander Rukletsov wrote: > > I propose to replace the comment in https://reviews.apache.org/r/40818/ > > > > Just curious, how did you spot this? it looks fine in my editor. Also, is > > it something we should add as a commit hook? > > Neil Conway wrote: >

Re: Review Request 40631: Move "using mesos::fetcher::FetcherInfo" into internal namespace in "fetcher.hpp"

2015-12-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40631/#review108534 --- Patch looks great! Reviews applied: [40631] Passed command:

Re: Review Request 40746: Quota: Removed quota from registry for remove request.

2015-12-01 Thread Joseph Wu
> On Nov. 30, 2015, 10:47 a.m., Joseph Wu wrote: > > src/master/quota_handler.cpp, lines 393-394 > > > > > > `RemoveQuota` appears idempotent, so what is the problem with trying to > > remove a quota that is

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-01 Thread Jie Yu
> On Dec. 1, 2015, 2:24 p.m., Michael Park wrote: > > src/master/master.cpp, line 2771 > > > > > > Why is it that we need to perform validation within authorization? We > > perform `authorizeTask` with a

Re: Review Request 40821: Quota: Ensured the sorter for quota'ed roles does not contain revocable resources.

2015-12-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40821/#review108546 --- Patch looks great! Reviews applied: [40586, 40551, 40332, 40795,

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-01 Thread Greg Mann
> On Dec. 1, 2015, 2:24 p.m., Michael Park wrote: > > src/master/master.cpp, line 2771 > > > > > > Why is it that we need to perform validation within authorization? We > > perform `authorizeTask` with a

Re: Review Request 40822: Updated the pattern for `_teardown` authorization continuation.

2015-12-01 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40822/#review108545 --- Ship it! Ship It! - Jie Yu On Dec. 1, 2015, 3:16 p.m., Michael

Re: Review Request 40332: Quota: Implemented recovery in hierarchical allocator.

2015-12-01 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40332/#review108615 --- Ship it! Ship It! - Qian Zhang On Nov. 30, 2015, 11:32 p.m.,

Re: Review Request 40795: Quota: Properly initialized the sorter for quota'ed roles in the allocator.

2015-12-01 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40795/#review108617 --- Ship it! Ship It! - Qian Zhang On Nov. 30, 2015, 11:32 p.m.,

Re: Review Request 40829: Improved authorization documentation.

2015-12-01 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40829/ --- (Updated Dec. 1, 2015, 7:10 p.m.) Review request for mesos and Adam B.

Re: Review Request 40811: Fixed flakiness in tests using the Scheduler library

2015-12-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40811/#review108549 --- Patch looks great! Reviews applied: [40811] Passed command:

Review Request 40829: Fixed reference to deprecated endpoint in docs.

2015-12-01 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40829/ --- Review request for mesos and Adam B. Repository: mesos Description ---

Re: Review Request 37853: Overlay filesystem provisioning backend

2015-12-01 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37853/#review108544 --- src/Makefile.am (line 776)

Re: Review Request 40806: Made HDFS::exists asynchronous.

2015-12-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40806/#review108465 --- Patch looks great! Reviews applied: [40250, 40251, 40252, 40274,

Re: Review Request 40808: Made the order of `EXPECT_CALL()`s match the expected order of events.

2015-12-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40808/#review108473 --- Patch looks great! Reviews applied: [40808] Passed command:

Re: Review Request 40806: Made HDFS::exists asynchronous.

2015-12-01 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40806/#review108475 --- src/hdfs/hdfs.cpp (line 82)

Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

2015-12-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40379/#review107883 --- Patch looks great! Reviews applied: [40375, 40379] Passed

Re: Review Request 37853: Overlay filesystem provisioning backend

2015-12-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37853/#review108504 --- Patch looks great! Reviews applied: [37853] Passed command:

Review Request 40822: Updated the pattern for `_teardown` authorization continuation.

2015-12-01 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40822/ --- Review request for mesos, Greg Mann and Jie Yu. Repository: mesos

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

2015-12-01 Thread Michael Park
> On Nov. 30, 2015, 9:44 p.m., Jie Yu wrote: > > src/master/http.cpp, line 2313 > > > > > > Instead of adding a new parameter to `_operation`. Can we put the > > logics of checking authorization result to

Re: Review Request 40799: Removed non-ASCII characters from test case comment.

2015-12-01 Thread Neil Conway
> On Dec. 1, 2015, 1:49 p.m., Alexander Rukletsov wrote: > > I propose to replace the comment in https://reviews.apache.org/r/40818/ > > > > Just curious, how did you spot this? it looks fine in my editor. Also, is > > it something we should add as a commit hook? Hi Alex -- note that the

Re: Review Request 40818: Fixed flaky test (AvailableResourcesAfterRescinding).

2015-12-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40818/#review108517 --- Patch looks great! Reviews applied: [40818] Passed command:

Re: Review Request 40631: Move "using mesos::fetcher::FetcherInfo" into internal namespace in "fetcher.hpp"

2015-12-01 Thread Klaus Ma
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40631/ --- (Updated Dec. 1, 2015, 8:51 p.m.) Review request for mesos, Benjamin Bannier,

Re: Review Request 40631: Move "using mesos::fetcher::FetcherInfo" into internal namespace in "fetcher.hpp"

2015-12-01 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40631/#review108487 --- src/slave/containerizer/fetcher.hpp (line 41)

Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

2015-12-01 Thread Klaus Ma
> On Dec. 1, 2015, 10:10 a.m., Joseph Wu wrote: > > src/slave/slave.cpp, lines 4464-4466 > > > > > > Can you add a comment about why this is purposefully and unilaterally > > done here (potentially overwriting

Re: Review Request 39985: [1/5] Introduced ACL protobuf definitions for dynamic reservation.

2015-12-01 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39985/#review108483 --- Ship it! Ship It! - Michael Park On Nov. 30, 2015, 8:32 p.m.,

Re: Review Request 40754: Fixed flaky test (AvailableResourcesAfterRescinding).

2015-12-01 Thread Alexander Rukletsov
> On Nov. 27, 2015, 10:28 p.m., Joris Van Remoortere wrote: > > src/tests/master_quota_tests.cpp, line 964 > > > > > > Can we settle, and then resume here? > > I think with Joseph's cleanups we shouldn't exit a

Re: Review Request 40706: Clarified a comment for `Accept` call.

2015-12-01 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40706/#review108481 --- include/mesos/scheduler/scheduler.proto (line 229)

Re: Review Request 40631: Move "using mesos::fetcher::FetcherInfo" into internal namespace in "fetcher.hpp"

2015-12-01 Thread Klaus Ma
> On Nov. 24, 2015, 5:42 p.m., Benjamin Bannier wrote: > > src/tests/mesos.hpp, line 87 > > > > > > This seems like a weird addition for this patch: while the existing > > `using` decls above could be justified

Re: Review Request 40524: Used string directly in resources.cpp.

2015-12-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40524/#review108490 --- Patch looks great! Reviews applied: [40524] Passed command:

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-01 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/#review108496 --- src/master/master.hpp (line 706)

Re: Review Request 40631: Move "using mesos::fetcher::FetcherInfo" into internal namespace in "fetcher.hpp"

2015-12-01 Thread Klaus Ma
> On Dec. 1, 2015, 9:08 p.m., Michael Park wrote: > > src/slave/containerizer/fetcher.hpp, line 43 > > > > > > I agree with the intent to remove the using declaration out of > > `fetcher.hpp`, but I don't agree with

Re: Review Request 39986: [2/5] Enabled the Authorizer to handle Reserve/Unreserve ACLs.

2015-12-01 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39986/#review108492 --- Ship it! Ship It! - Michael Park On Nov. 30, 2015, 8:33 p.m.,

Re: Review Request 40799: Removed non-ASCII characters from test case comment.

2015-12-01 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40799/#review108494 --- I propose to replace the comment in

Re: Review Request 40811: Fixed flakiness in tests using the Scheduler library

2015-12-01 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40811/#review108495 --- Ship it! Ship It! - Alexander Rojas On Dec. 1, 2015, 4:38

Re: Review Request 40706: Clarified a comment for `Accept` call.

2015-12-01 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40706/ --- (Updated Dec. 1, 2015, 1:33 p.m.) Review request for mesos, Ben Mahler,

Re: Review Request 40746: Quota: Removed quota from registry for remove request.

2015-12-01 Thread Alexander Rukletsov
> On Nov. 30, 2015, 6:47 p.m., Joseph Wu wrote: > > src/master/quota_handler.cpp, lines 393-394 > > > > > > `RemoveQuota` appears idempotent, so what is the problem with trying to > > remove a quota that is

Review Request 40821: Quota: Ensured the sorter for quota'ed roles does not contain revocable resources.

2015-12-01 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40821/ --- Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere,

Re: Review Request 40811: Fixed flakiness in tests using the Scheduler library

2015-12-01 Thread Alexander Rojas
> On Dec. 1, 2015, 2:56 p.m., Alexander Rojas wrote: > > Ship It! I have to revert this ship-it, after playing with it longer, I manage to reproduce at least two types of crashes after applying this patch. - Alexander --- This is an

Re: Review Request 39989: [5/5] Added framework authorization for dynamic reservation.

2015-12-01 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39989/#review108515 --- src/master/master.cpp (lines 3190 - 3191)

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

2015-12-01 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39988/#review108510 --- src/master/http.cpp (lines 1037 - 1040)

Re: Review Request 40445: Added linter for license headers in some file types.

2015-12-01 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40445/#review108523 --- Why do we want to introduce a `mesos-license.py` rather than

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-01 Thread Greg Mann
> On Dec. 1, 2015, 2:24 p.m., Michael Park wrote: > > src/master/master.cpp, line 2771 > > > > > > Why is it that we need to perform validation within authorization? We > > perform `authorizeTask` with a