Re: Review Request 53624: Add a content type option to mesos-execute.

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

Re: Review Request 53645: Added '--task' into mesos-execute.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53645/ --- (Updated Nov. 11, 2016, 11:13 a.m.) Review request for mesos, Avinash sridharan

Re: Review Request 53645: Added '--task' into mesos-execute.

> On Nov. 11, 2016, 12:01 a.m., Avinash sridharan wrote: > > src/cli/execute.cpp, line 1065 > > > > > > Instead of this why can't we just do: > > > > Option taskInfo = flags.task > > > > if (flags.

Re: Review Request 53354: Updated namespace isolators to customize based on 'ContainerClass'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53354/#review155658 --- Fix it, then Ship it! src/slave/containerizer/mesos/isolators/

Re: Review Request 53654: Avoided unnecessary copies of `HttpConnection`.

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

Re: Review Request 53457: Updated Socket::Impl::accept to return std::shared_ptr.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53457/#review155680 --- Fix it, then Ship it! Modulo the leak path described below: 3

Re: Review Request 53461: Inlined function only used one place.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53461/#review155678 --- Ship it! Ship It! - Benjamin Mahler On Nov. 4, 2016, 6:42 a.

Re: Review Request 53492: Added non-const operator-> to Future.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53492/#review155677 --- I've thought about this before as well, since Future is a shared o

Re: Review Request 53624: Add a content type option to mesos-execute.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53624/ --- (Updated Nov. 11, 2016, 12:49 a.m.) Review request for mesos, Anand Mazumdar, J

Re: Review Request 53366: Introduced a streaming gzip::Decompressor.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53366/#review155676 --- Fix it, then Ship it! 3rdparty/stout/include/stout/gzip.hpp (l

Re: Review Request 53366: Introduced a streaming gzip::Decompressor.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53366/ --- (Updated Nov. 11, 2016, 12:36 a.m.) Review request for mesos and Anand Mazumdar

Re: Review Request 53365: Fixed an issue in the gzip error handling.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53365/ --- (Updated Nov. 11, 2016, 12:33 a.m.) Review request for mesos and Anand Mazumdar

Re: Review Request 53490: Added a test for request streaming via the connection abstraction.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53490/ --- (Updated Nov. 11, 2016, 12:20 a.m.) Review request for mesos and Benjamin Mahle

Re: Review Request 53366: Introduced a streaming gzip::Decompressor.

> On Nov. 10, 2016, 11:45 p.m., Anand Mazumdar wrote: > > 3rdparty/stout/include/stout/gzip.hpp, lines 157-168 > > > > > > hmm, this is a little weird. Why can't this be just be a `bool > > failed()` similar to what

Re: Review Request 53366: Introduced a streaming gzip::Decompressor.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53366/#review155666 --- LGTM minus my comment around renaming `finish()` -> `failed()`.

Re: Review Request 53485: Introduced a `readAll()` helper on `http::Pipe::Reader`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53485/#review155667 --- Ship it! 3rdparty/libprocess/src/http.cpp (line 436)

Re: Review Request 53628: Document the namespaces/uts isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53628/#review155663 --- Patch looks great! Reviews applied: [53626, 53627, 53628] Passed

Re: Review Request 53365: Fixed an issue in the gzip error handling.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53365/#review155662 --- Ship it! Ship It! - Anand Mazumdar On Nov. 8, 2016, 4:22 a.m

Re: Review Request 53511: Parameterized existing decoder tests on the type of decoder.

> On Nov. 9, 2016, 12:57 a.m., Benjamin Mahler wrote: > > Very clean update to the test! I left a comment about whether we can do > > this similar parameterization on the response side, perhaps you'll want to > > leave a TODO for this if you don't want to take it on now? I left a TODO for now

Re: Review Request 53490: Added a test for request streaming via the connection abstraction.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53490/ --- (Updated Nov. 10, 2016, 10:31 p.m.) Review request for mesos and Benjamin Mahle

Re: Review Request 53490: Added a test for request streaming via the connection abstraction.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53490/ --- (Updated Nov. 10, 2016, 10:20 p.m.) Review request for mesos and Benjamin Mahle

Re: Review Request 53490: Added a test for request streaming via the connection abstraction.

> On Nov. 9, 2016, 9:50 p.m., Benjamin Mahler wrote: > > What you have looks good, I would just suggest that you also cover the case > > where the request body does not complete to EOF, but fails. In this case > > the server should not receive the request at all (if > > !RouteOptions.streaming

Re: Review Request 53489: Added support for request streaming to the connection abstraction.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53489/ --- (Updated Nov. 10, 2016, 10:18 p.m.) Review request for mesos and Benjamin Mahle

Re: Review Request 53624: Use JSON content type in mesos-execute.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53624/#review155657 --- src/cli/execute.cpp (line 299)

Re: Review Request 53489: Added support for request streaming to the connection abstraction.

> On Nov. 9, 2016, 3:44 a.m., Benjamin Mahler wrote: > > Unrelated to this particular change, but can you write up a quick test > > (doesn't need to be committed) to ensure that we don't have infinitely > > growing future chains here, as benh mentioned offline? I never tested this > > for http

Re: Review Request 53624: Use JSON content type in mesos-execute.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53624/#review155656 --- Ship it! Ship It! - Vinod Kone On Nov. 10, 2016, 5:46 p.m.,

Re: Review Request 53488: Removed `convert()` continuations in favor of using `io::read()`.

> On Nov. 9, 2016, 1:16 a.m., Benjamin Mahler wrote: > > 3rdparty/libprocess/src/http.cpp, lines 926-928 > > > > > > Before you commit this, can you make sure that this looks consistent > > with the request conversi

Re: Review Request 53488: Removed `convert()` continuations in favor of using `io::read()`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53488/ --- (Updated Nov. 10, 2016, 10:10 p.m.) Review request for mesos and Benjamin Mahle

Re: Review Request 53511: Parameterized existing decoder tests on the type of decoder.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53511/ --- (Updated Nov. 10, 2016, 10:07 p.m.) Review request for mesos and Benjamin Mahle

Re: Review Request 53510: Removed extraneous socket argument from `DataDecoder` constructor.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53510/ --- (Updated Nov. 10, 2016, 10:04 p.m.) Review request for mesos and Benjamin Mahle

Re: Review Request 53485: Introduced a `io::read()` overload for reading from piped reader.

> On Nov. 5, 2016, 1:42 a.m., Benjamin Mahler wrote: > > Nice commit summary! Would you mind adding a test alongside this? Should be > > pretty straightforward to do. Added. > On Nov. 5, 2016, 1:42 a.m., Benjamin Mahler wrote: > > 3rdparty/libprocess/include/process/io.hpp, line 100 > >

Re: Review Request 53485: Introduced a `readAll()` helper on `http::Pipe::Reader`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53485/ --- (Updated Nov. 10, 2016, 10:02 p.m.) Review request for mesos and Benjamin Mahle

Re: Review Request 53484: Introduced `RouteOptions` to support streaming requests.

> On Nov. 5, 2016, 1:37 a.m., Benjamin Mahler wrote: > > 3rdparty/libprocess/include/process/process.hpp, line 262 > > > > > > I wonder if this should be 'streamingRequest' or Request::Type? As it > > stands I wonde

Re: Review Request 53484: Introduced `RouteOptions` to support streaming requests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53484/ --- (Updated Nov. 10, 2016, 10:01 p.m.) Review request for mesos and Benjamin Mahle

Re: Review Request 53483: Introduced a reader member to `Request` to support request streaming.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53483/ --- (Updated Nov. 10, 2016, 10 p.m.) Review request for mesos and Benjamin Mahler.

Re: Review Request 53482: Initialized the POD type in the `Request` struct.

> On Nov. 5, 2016, 1:01 a.m., Benjamin Mahler wrote: > > Actually whoops, we rely on the generated constructor, so this doesn't seem > > like an issue? As discussed offline, we still need this as POD types are not initialized by the default generated constructor. - Anand --

Re: Review Request 53482: Initialized the POD type in the `Request` struct.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53482/ --- (Updated Nov. 10, 2016, 9:59 p.m.) Review request for mesos and Benjamin Mahler

Re: Review Request 53623: Windows: Fixed provisioner copy backend build error.

> On Nov. 9, 2016, 9:38 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/provisioner/backends/copy.cpp, lines 269-272 > > > > > > I'd move this to `provision` and bail out early (before mkdir) Even if we bail o

Re: Review Request 53586: Added special case for entering "mnt" namespaces for DEBUG containers.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53586/ --- (Updated Nov. 10, 2016, 9:07 p.m.) Review request for mesos and Jie Yu. Chang

Re: Review Request 53586: Added special case for entering "mnt" namespaces for DEBUG containers.

> On Nov. 10, 2016, 8:50 p.m., Jie Yu wrote: > > src/tests/containerizer/nested_mesos_containerizer_tests.cpp, lines 433-436 > > > > > > Is this necessary? I think once we get a TASK_RUNNING, the subprocess > > shou

Re: Review Request 53586: Added special case for entering "mnt" namespaces for DEBUG containers.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53586/#review155617 --- Fix it, then Ship it! src/slave/containerizer/mesos/containeri

Re: Review Request 53586: Added special case for entering "mnt" namespaces for DEBUG containers.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53586/ --- (Updated Nov. 10, 2016, 8:25 p.m.) Review request for mesos and Jie Yu. Chang

Re: Review Request 53351: Added 'ContainerClass' to help decide how best to launch a container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53351/ --- (Updated Nov. 10, 2016, 8:11 p.m.) Review request for mesos and Jie Yu. Chang

Re: Review Request 53500: Used an environment variable to pass command environment.

> On Nov. 5, 2016, 2:38 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/containerizer.cpp, line 1405 > > > > > > I'll be nice to use environment variable for all flags here. I think we > > probably need a hel

Re: Review Request 53500: Used an environment variable to pass command environment.

> On Nov. 9, 2016, 2:37 a.m., Gastón Kleiman wrote: > > The Docker containerizer still passes the env variables to the executor > > through cmd line flags, we might want to fix that as well: > > > > https://github.com/apache/mesos/blob/master/src/slave/containerizer/docker.cpp#L244 > > https://

Review Request 53654: Avoided unnecessary copies of `HttpConnection`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53654/ --- Review request for mesos and Anand Mazumdar. Repository: mesos Description --

Re: Review Request 52919: Updated scripts to allow override of MESOS environment variables.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52919/#review155624 --- Patch looks great! Reviews applied: [52856, 52787, 52919] Passed

Re: Review Request 53626: Add net::setDomainname() helper API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53626/ --- (Updated Nov. 10, 2016, 5:52 p.m.) Review request for mesos and Jie Yu. Bugs:

Re: Review Request 53627: Implement a namespaces/uts isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53627/ --- (Updated Nov. 10, 2016, 5:53 p.m.) Review request for mesos and Jie Yu. Bugs:

Re: Review Request 53628: Document the namespaces/uts isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53628/ --- (Updated Nov. 10, 2016, 5:53 p.m.) Review request for mesos and Jie Yu. Bugs:

Re: Review Request 53624: Use JSON content type in mesos-execute.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53624/ --- (Updated Nov. 10, 2016, 5:46 p.m.) Review request for mesos, Anand Mazumdar, Ji

Re: Review Request 52295: Added additional unit tests for shared resources.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52295/#review152420 --- src/tests/hierarchical_allocator_tests.cpp (lines 1512 - 1513)

Re: Review Request 53645: Added '--task' into mesos-execute.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53645/#review155602 --- Patch looks great! Reviews applied: [53644, 53645] Passed comman

Re: Review Request 53644: Added parse function for v1::TaskInfo protobuf.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53644/#review155600 --- Ship it! Ship It! - Avinash sridharan On Nov. 10, 2016, 1:44

Re: Review Request 53645: Added '--task' into mesos-execute.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53645/#review155598 --- src/cli/execute.cpp (line 874)

Re: Review Request 52787: Reorganized the work directory and the runtime directory in local mode.

> On Nov. 9, 2016, 8:57 p.m., Kevin Klues wrote: > > bin/mesos-local-flags.sh.in, lines 19-20 > > > > > > Do we even these anymore since we set the work/runtime dir in the code > > via e.g.: > > > > ``` > >

Re: Review Request 52787: Reorganized the work directory and the runtime directory in local mode.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52787/ --- (Updated Nov. 10, 2016, 3:44 p.m.) Review request for mesos, Jie Yu, Kevin Klue

Re: Review Request 52856: Reverted incorrect changes in 1c2ee5c.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52856/ --- (Updated Nov. 10, 2016, 3:44 p.m.) Review request for mesos, Jie Yu, Kevin Klue

Re: Review Request 52919: Updated scripts to allow override of MESOS environment variables.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52919/ --- (Updated Nov. 10, 2016, 3:44 p.m.) Review request for mesos, Jie Yu, Kevin Klue

Re: Review Request 53308: Added new hook for modifying the executor environment.

> On Nov. 10, 2016, 3:19 p.m., Kapil Arya wrote: > > Before doing full review, I am wondering if we can create a new protobuf > > called `DockerExecutorPrepareInfo` or something similar with the idea that > > we can pass on not only environment variables, but also volumes (and any > > other op

Re: Review Request 53308: Added new hook for modifying the executor environment.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53308/#review155592 --- Before doing full review, I am wondering if we can create a new pr

Re: Review Request 53610: Added health checks documentation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53610/#review155565 --- I am not a native speaker, so please take my suggestions with a gr

Re: Review Request 53641: Refactored `CgroupsAnyHierarchyTest` test cases.

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

Re: Review Request 53644: Added parse function for v1::TaskInfo protobuf.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53644/ --- (Updated Nov. 10, 2016, 9:44 p.m.) Review request for mesos, Avinash sridharan,

Re: Review Request 53529: Improved the readability of the master validation tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53529/#review155278 --- Ship it! src/tests/master_validation_tests.cpp (lines 87 - 91)

Re: Review Request 53645: Added '--task' into mesos-execute.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53645/ --- (Updated Nov. 10, 2016, 9:44 p.m.) Review request for mesos, Avinash sridharan,

Review Request 53645: Added '--task' into mesos-execute.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53645/ --- Review request for mesos, Avinash sridharan and Jie Yu. Repository: mesos Des

Review Request 53644: Added parse function for v1::TaskInfo protobuf.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53644/ --- Review request for mesos, Avinash sridharan and Jie Yu. Repository: mesos Des

Re: Review Request 53641: Refactored `CgroupsAnyHierarchyTest` test cases.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53641/ --- (Updated Nov. 10, 2016, 12:03 p.m.) Review request for mesos, Gilbert Song, Jie

Review Request 53641: Refactored `CgroupsAnyHierarchyTest` test cases.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53641/ --- Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Jiang Yan Xu. R

Re: Review Request 53533: Extended docker flags to pass devices to mesos-docker-executor.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53533/ --- (Updated Nov. 10, 2016, 8:15 a.m.) Review request for mesos, Benjamin Mahler, G

Re: Review Request 50127: Added DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50127/ --- (Updated Nov. 10, 2016, 8:15 a.m.) Review request for mesos, Benjamin Mahler, K

Re: Review Request 50125: Added mesos-docker-executor support for devices control.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50125/ --- (Updated Nov. 10, 2016, 8:15 a.m.) Review request for mesos, Benjamin Mahler, K

Re: Review Request 53532: Added parse helper function to 'Docker::Device'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53532/ --- (Updated Nov. 10, 2016, 8:14 a.m.) Review request for mesos, Benjamin Mahler, G

Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50599/ --- (Updated Nov. 10, 2016, 8:14 a.m.) Review request for mesos, Benjamin Mahler, G

Re: Review Request 50128: Overloaded the << operator for 'Docker::Device'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50128/ --- (Updated Nov. 10, 2016, 8:13 a.m.) Review request for mesos, Benjamin Mahler, G

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50947/ --- (Updated Nov. 10, 2016, 8:13 a.m.) Review request for mesos, Benjamin Mahler, G

Re: Review Request 52735: Updated comment message for docker killing.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52735/ --- (Updated Nov. 10, 2016, 8:13 a.m.) Review request for mesos, Benjamin Mahler, G