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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40115/ --- (Updated Dec. 22, 2015, 7:32 a.m.) Review request for Alex Naparu and Alex Clem

Re: Review Request 41378: Logger Module: Update tests that use the DockerContainerizer to pass in a ContainerLogger.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41378/#review111577 --- Bad patch! Reviews applied: [41002, 41003, 41004, 41061, 4] F

Re: Review Request 41618: Edited defer documentation in libprocess.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41618/#review111576 --- Patch looks great! Reviews applied: [41618] Passed command: expor

Re: Review Request 41612: Fixed handling of failed authorization for (un)reserve operations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41612/#review111572 --- Patch looks great! Reviews applied: [41612] Passed command: expor

Re: Review Request 40936: Windows: Unified POSIX and Windows implementation of shell.hpp

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40936/ --- (Updated Dec. 22, 2015, 2:32 a.m.) Review request for mesos, Alex Naparu, Alex

Re: Review Request 41378: Logger Module: Update tests that use the DockerContainerizer to pass in a ContainerLogger.

> On Dec. 20, 2015, 3:24 p.m., Benjamin Hindman wrote: > > So, are you sure we need `CreateContainerLogger` and not just > > `ContainerLogger::create`? If we're not extending it than I don't know that > > we need it for now. Also, this looks nice and mechanical given > > https://reviews.apache

Re: Review Request 41378: Logger Module: Update tests that use the DockerContainerizer to pass in a ContainerLogger.

> On Dec. 14, 2015, 8:07 p.m., Timothy Chen wrote: > > src/tests/environment.cpp, line 226 > > > > > > If we expect NULL to be a valid option we should make it an Option. > > Joseph Wu wrote: > NULL is not a val

Re: Review Request 41378: Logger Module: Update tests that use the DockerContainerizer to pass in a ContainerLogger.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41378/ --- (Updated Dec. 21, 2015, 5:35 p.m.) Review request for mesos, Benjamin Hindman a

Re: Review Request 41370: Logger Module: Update DockerContainerizer test helpers

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41370/ --- (Updated Dec. 21, 2015, 5:35 p.m.) Review request for mesos, Benjamin Hindman a

Re: Review Request 41560: Change Docker::run to take Subprocess::IO instead of Option

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41560/ --- (Updated Dec. 21, 2015, 5:34 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 41294: Logger Module: Adds the ContainerLogger into the DockerContainerizer.

> On Dec. 20, 2015, 3:17 p.m., Benjamin Hindman wrote: > > We shouldn't pass `ContainerLogger` into `Docker` while also keeping it > > `Owned` inside `DockerContainerizer`. It's confusing to people, we're > > explicitly breaking the contract of `Owned`. > > > > Instead, why not change the `Doc

Re: Review Request 41560: Change Docker::run to take Subprocess::IO instead of Option

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

Re: Review Request 41294: Logger Module: Adds the ContainerLogger into the DockerContainerizer.

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

Re: Review Request 39492: Added status endpoint for quota master endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39492/#review111565 --- Patch looks great! Reviews applied: [39211, 39018, 39102, 36913, 3

Re: Review Request 41167: Logger Module: Add support for the ContainerLogger to the Mesos Containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41167/ --- (Updated Dec. 21, 2015, 5:21 p.m.) Review request for mesos, Benjamin Hindman a

Re: Review Request 41560: Change Docker::run to take Subprocess::IO instead of Option

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41560/#review111563 --- Ship it! Ship It! - Timothy Chen On Dec. 21, 2015, 11:49 p.m.,

Re: Review Request 41002: Logger Module: Introduce the ContainerLogger interface for logging the stdout/stderr of executors and tasks.

> On Dec. 20, 2015, 6:21 p.m., Benjamin Hindman wrote: > > include/mesos/slave/container_logger.hpp, lines 85-93 > > > > > > I'm a little confused. Who executes this? The comment makes it read > > like there is some

Re: Review Request 41004: Logger Module: Introduce the ContainerLogger module.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41004/ --- (Updated Dec. 21, 2015, 4:49 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 41002: Logger Module: Introduce the ContainerLogger interface for logging the stdout/stderr of executors and tasks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41002/ --- (Updated Dec. 21, 2015, 4:49 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 41003: Logger Module: Add the SandboxContainerLogger, the default ContainerLogger implementation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41003/ --- (Updated Dec. 21, 2015, 4:49 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 41616: Proto buf definition for GPUInfo. This is defined to introduce GPU as first class resource within Mesos.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41616/#review111557 --- Patch looks great! Reviews applied: [41616] Passed command: expor

Review Request 41618: Edited defer documentation in libprocess.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41618/ --- Review request for mesos, Benjamin Hindman and Neil Conway. Repository: mesos

Re: Review Request 41560: Change Docker::run to take Subprocess::IO instead of Option

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41560/ --- (Updated Dec. 21, 2015, 3:49 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 41603: Cleaned up quota HTTP handling code and tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41603/#review111555 --- src/master/quota_handler.cpp (lines 57 - 60)

Re: Review Request 41514: Accepted a single JSON object for quota set request.

> On Dec. 21, 2015, 10:03 p.m., Guangya Liu wrote: > > src/master/quota_handler.cpp, lines 358-359 > > > > > > What will happen if end user input TRUE, True, FALSE, False? Shall we > > document those keyword in quot

Re: Review Request 41613: Added `defaultRole` constant to persistent volume tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41613/#review111553 --- Patch looks great! Reviews applied: [41613] Passed command: expor

Re: Review Request 41612: Fixed handling of failed authorization for (un)reserve operations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41612/#review111552 --- Ship it! Ship It! - Guangya Liu On 十二月 21, 2015, 7:12 p.m., Gre

Re: Review Request 41444: Cleaned up Authorizer interface.

> On Dec. 21, 2015, 11:20 p.m., Greg Mann wrote: > > Thanks, this is a big improvement! Just a few small nits below: - Greg --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41444/#review111528

Re: Review Request 41444: Cleaned up Authorizer interface.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41444/#review111528 --- Thanks, this is a big improvement! include/mesos/authorizer/autho

Re: Review Request 41539: Enabled agent do not kill exeucutor for some error cases.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41539/#review111550 --- Ship it! Ship It! - Joris Van Remoortere On Dec. 18, 2015, 5:38

Re: Review Request 39492: Added status endpoint for quota master endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39492/#review111547 --- Ship it! I assume that you will have another RR to cover the unit

Re: Review Request 41403: Removed include tuple from values.cpp.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41403/#review111544 --- Ship it! Ship It! - Joris Van Remoortere On Dec. 17, 2015, 7:53

Re: Review Request 41514: Accepted a single JSON object for quota set request.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41514/#review111543 --- src/master/quota_handler.cpp (lines 333 - 334)

Re: Review Request 41444: Cleaned up Authorizer interface.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41444/#review111542 --- Patch looks great! Reviews applied: [41444] Passed command: expor

Review Request 41616: Proto buf definition for GPUInfo. This is defined to introduce GPU as first class resource within Mesos.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41616/ --- Review request for mesos and Ben Mahler. Bugs: MESOS-4232 https://issues.ap

Re: Review Request 41514: Accepted a single JSON object for quota set request.

> On Dec. 18, 2015, 12:04 a.m., Anand Mazumdar wrote: > > src/master/quota_handler.cpp, line 353 > > > > > > hmmm .. Should we return a `BadRequest` for all other non-allowed > > values of `force` other then `true`

Re: Review Request 41603: Cleaned up quota HTTP handling code and tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41603/#review111531 --- Patch looks great! Reviews applied: [41514, 41515, 41603] Passed

Re: Review Request 41294: Logger Module: Adds the ContainerLogger into the DockerContainerizer.

> On Dec. 14, 2015, 8:06 p.m., Timothy Chen wrote: > > src/docker/docker.cpp, line 603 > > > > > > Just a general question, what is the reasoning behind allowing the > > logger to modify the path and argv of the ori

Re: Review Request 41169: Logger Module: Update tests that use the MesosContainerizer to pass in an ContainerLogger

> On Dec. 20, 2015, 3:01 p.m., Benjamin Hindman wrote: > > This looks fine, assuming that we actually need `CreateContainerLogger` > > (see below). > > Benjamin Hindman wrote: > I looked ahead in the review chain and it doesn't look like you ever > extend `CreateContainerLogger`, so why wa

Re: Review Request 41169: Logger Module: Update tests that use the MesosContainerizer to pass in an ContainerLogger

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41169/ --- (Updated Dec. 21, 2015, 11:37 a.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 41167: Logger Module: Add support for the ContainerLogger to the Mesos Containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41167/ --- (Updated Dec. 21, 2015, 11:37 a.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 41166: Logger Module: Explicitly disallow use of the ContainerLogger with the External Containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41166/ --- (Updated Dec. 21, 2015, 11:37 a.m.) Review request for mesos, Benjamin Hindman

Review Request 41613: Added `defaultRole` constant to persistent volume tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41613/ --- Review request for mesos, Alexander Rukletsov, Jie Yu, and Michael Park. Reposi

Re: Review Request 39492: Added status endpoint for quota master endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39492/ --- (Updated Dec. 21, 2015, 7:02 p.m.) Review request for mesos, Alexander Rukletso

Re: Review Request 40544: Added quota remove handling.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40544/ --- (Updated Dec. 21, 2015, 6:54 p.m.) Review request for mesos, Alexander Rukletso

Re: Review Request 41461: stout: Added SFINAE-friendly `result_of`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41461/ --- (Updated Dec. 21, 2015, 6:10 p.m.) Review request for mesos, Alex Clemmer and J

Re: Review Request 41604: CMake: Added missing protobuf files to CMake build.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41604/#review111514 --- Patch looks great! Reviews applied: [41604] Passed command: expor

Review Request 41612: Fixed handling of failed authorization for (un)reserve operations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41612/ --- Review request for mesos, Alexander Rukletsov, Jie Yu, and Michael Park. Reposi

Re: Review Request 39492: Added status endpoint for quota master endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39492/#review111507 --- Ship it! src/master/master.hpp (line 945)

Re: Review Request 39614: Quota: Added Status Validation Tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39614/ --- (Updated Dec. 21, 2015, 5:16 p.m.) Review request for mesos, Alexander Rukletso

Re: Review Request 41002: Logger Module: Introduce the ContainerLogger interface for logging the stdout/stderr of executors and tasks.

> On Dec. 20, 2015, 6:21 p.m., Benjamin Hindman wrote: > > include/mesos/slave/container_logger.hpp, lines 85-93 > > > > > > I'm a little confused. Who executes this? The comment makes it read > > like there is some

Re: Review Request 39614: Quota: Added Status Validation Tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39614/ --- (Updated Dec. 21, 2015, 5:13 p.m.) Review request for mesos, Alexander Rukletso

Re: Review Request 41608: Removed extra '; ' after inline class function definitions.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41608/#review111498 --- Ship it! Ship It! - Michael Park On Dec. 21, 2015, 4:48 p.m., A

Re: Review Request 41609: Cleaned up formatting for multiple inheritance.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41609/#review111499 --- Ship it! Ship It! - Michael Park On Dec. 21, 2015, 4:58 p.m., A

Re: Review Request 41609: Cleaned up formatting for multiple inheritance.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41609/ --- (Updated Dec. 21, 2015, 4:58 p.m.) Review request for mesos, Bernd Mathiske, Jo

Re: Review Request 41347: Added a TODO to remove a `friend class` declaration.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41347/#review111493 --- Ship it! Ship It! - Joris Van Remoortere On Dec. 14, 2015, 2:28

Re: Review Request 41609: Cleaned up formatting for multiple inheritance.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41609/ --- (Updated Dec. 21, 2015, 4:56 p.m.) Review request for mesos, Bernd Mathiske, Jo

Review Request 41608: Removed extra '; ' after inline class function definitions.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41608/ --- Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway. Reposit

Review Request 41609: Cleaned up formatting for multiple inheritance.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41609/ --- Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway. Reposit

Re: Review Request 41444: Cleaned up Authorizer interface.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41444/ --- (Updated Dec. 21, 2015, 4:04 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 41472: Disallowed dynamic reservations for roles not on the role whitelist.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41472/#review111483 --- src/master/http.cpp (line 1001)

Re: Review Request 41444: Cleaned up Authorizer interface.

> On Dec. 18, 2015, 8:42 a.m., Adam B wrote: > > include/mesos/authorizer/authorizer.hpp, line 66 > > > > > > s/on/of/ killed the entire phrase. - Alexander ---

Re: Review Request 41444: Cleaned up Authorizer interface.

> On Dec. 18, 2015, 8:42 a.m., Adam B wrote: > > Thank you for cleaning this up. It looked like an overwhelming amount of > > documentation for what is not really that complex of an API. It still looks > > a bit verbose/repetitive, so I've made some suggestions of what else to cut > > out. > >

Re: Review Request 41444: Cleaned up Authorizer interface.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41444/ --- (Updated Dec. 21, 2015, 3:42 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 41472: Disallowed dynamic reservations for roles not on the role whitelist.

> On Dec. 17, 2015, 1:17 a.m., Michael Park wrote: > > src/master/http.cpp, lines 1000-1006 > > > > > > Hm, should this be added to `validation::operation::validate` instead? > > That way if/when we have frameworks

Re: Review Request 39492: Added status endpoint for quota master endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39492/ --- (Updated Dec. 21, 2015, 2:56 p.m.) Review request for mesos, Alexander Rukletso

Re: Review Request 39614: Quota: Added Status Validation Tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39614/ --- (Updated Dec. 21, 2015, 2:49 p.m.) Review request for mesos, Alexander Rukletso

Re: Review Request 41438: Added documentation on using network proxy for mesos fetcher

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41438/#review111467 --- Ship it! docs/fetcher.md (line 243)

Re: Review Request 41603: Cleaned up quota HTTP handling code and tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41603/ --- (Updated Dec. 21, 2015, 2:17 p.m.) Review request for mesos, Bernd Mathiske, Jo

Re: Review Request 40271: [7/7] Added documentation for RESERVE, UNRESERVE, CREATE, and DESTROY authorization.

> On Dec. 16, 2015, 2:39 p.m., Alexander Rukletsov wrote: > > docs/authorization.md, line 43 > > > > > > As I have already mentioned in a previous review, we do not really > > enforce it, do we? That's why I think "v

Re: Review Request 41603: Cleaned up quota HTTP handling code and tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41603/#review111462 --- src/master/quota_handler.cpp (line 60)

Re: Review Request 41603: Cleaned up quota HTTP handling code and tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41603/#review111461 --- Ship it! Ship It! - Joerg Schad On Dec. 21, 2015, 1:30 p.m., Al

Re: Review Request 41514: Accepted a single JSON object for quota set request.

> On Dec. 18, 2015, 12:04 a.m., Anand Mazumdar wrote: > > src/master/quota_handler.cpp, line 299 > > > > > > hmm, why don't we do a `using google::protobuf::RepeatedPtrField` and > > get rid of all this jaggedness

Review Request 41603: Cleaned up quota HTTP handling code and tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41603/ --- Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van Remoortere.

Re: Review Request 41515: Refactored error messages to reduce jaggedness.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41515/ --- (Updated Dec. 21, 2015, 1:30 p.m.) Review request for mesos, Bernd Mathiske, Jo

Re: Review Request 41514: Accepted a single JSON object for quota set request.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41514/ --- (Updated Dec. 21, 2015, 1:27 p.m.) Review request for mesos, Anand Mazumdar, Be

Review Request 41604: CMake: Added missing protobuf files to CMake build.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41604/ --- Review request for mesos, Diana Arroyo, Artem Harutyunyan, Joris Van Remoortere,

Re: Review Request 41514: Accepted a single JSON object for quota set request.

> On Dec. 18, 2015, 12:04 a.m., Anand Mazumdar wrote: > > LGTM.. Just some nits around: > > > > - using `const` for test strings. > > - reducing jaggedness for some blocks. > > > > Also, a query regarding just accepting `true/false` as `force` field values. Good points, thanks for the review,

Re: Review Request 41598: FreeBSD: use BSD cp in copy provisioner backend.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41598/#review111451 --- Patch looks great! Reviews applied: [41598] Passed command: expor

Re: Review Request 41597: Extending allocator interface to support dynamic weights

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41597/#review111447 --- Patch looks great! Reviews applied: [41597] Passed command: expor