Re: Review Request 54595: Introduced an `os::dup` abstraction in stout.

2017-02-05 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54595/#review164277 --- 3rdparty/stout/include/Makefile.am (line 68)

Re: Review Request 54591: Introduced `WindowsFD` class which is analogous to an `int` in POSIX.

2017-02-05 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54591/#review164276 --- 3rdparty/stout/include/stout/os.hpp (line 49)

Re: Review Request 54591: Introduced `WindowsFD` class which is analogous to an `int` in POSIX.

2017-02-05 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54591/#review164273 --- 3rdparty/stout/include/stout/os/windows/fd.hpp (line 20)

Re: Review Request 54592: Introduced an `os::lseek` abstraction in stout.

2017-02-05 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54592/#review164278 --- Fix it, then Ship it! 3rdparty/stout/include/Makefile.am

Re: Review Request 54762: Introduced an `os::pipe` abstraction to stout.

2017-02-05 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54762/#review164279 --- Fix it, then Ship it! 3rdparty/stout/include/Makefile.am

Re: Review Request 54601: Replaced `int` with `int_fd` in stout.

2017-02-05 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54601/#review164280 --- Ship it! Ship It! - Alex Clemmer On Feb. 5, 2017, 1:39

Re: Review Request 54591: Introduced `WindowsFD` class which is analogous to an `int` in POSIX.

2017-02-05 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54591/ --- (Updated Feb. 5, 2017, 1:24 p.m.) Review request for mesos, Daniel Pravat and

Re: Review Request 56247: Switched to implicit roles in master quota tests.

2017-02-05 Thread Neil Conway
> On Feb. 4, 2017, 12:09 a.m., Michael Park wrote: > > src/tests/master_quota_tests.cpp, lines 165-182 > > > > > > I don't understand what's happening here... we end up using the same > > flags, right? Why didn't

Re: Review Request 51052: Made mesos-docker-execute understand cgroups_enable_cfs.

2017-02-05 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51052/ --- (Updated Feb. 5, 2017, 11:01 p.m.) Review request for mesos, Gilbert Song,

Re: Review Request 56249: Cleaned up code style slightly in DRF sorter.

2017-02-05 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56249/ --- (Updated Feb. 5, 2017, 10:01 p.m.) Review request for mesos and Michael Park.

Re: Review Request 51052: Made mesos-docker-execute understand cgroups_enable_cfs.

2017-02-05 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51052/ --- (Updated Feb. 5, 2017, 11:09 p.m.) Review request for mesos, Gilbert Song,

Re: Review Request 54821: Refactored Docker::run() to make it only aware of docker cli options.

2017-02-05 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54821/ --- (Updated Feb. 5, 2017, 11:09 p.m.) Review request for mesos, Xiaojian Huang,

Re: Review Request 55160: Added test for DockerContainerizer when `cgroups_enable_cfs` is set.

2017-02-05 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55160/ --- (Updated Feb. 5, 2017, 11:09 p.m.) Review request for mesos, Gilbert Song,

Re: Review Request 55160: Added test for DockerContainerizer when `cgroups_enable_cfs` is set.

2017-02-05 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55160/#review164289 --- Patch looks great! Reviews applied: [54821, 51052, 55160]

Re: Review Request 54821: Refactored Docker::run() to make it only aware of docker cli options.

2017-02-05 Thread Zhitao Li
> On Feb. 3, 2017, 7:28 p.m., Jie Yu wrote: > > src/docker/docker.hpp, line 194 > > > > > > I'd try to avoid mesos:: prefixed type in this file. Can this just be a > > map? > > Zhitao Li wrote: > This could be

Re: Review Request 54591: Introduced `WindowsFD` class which is analogous to an `int` in POSIX.

2017-02-05 Thread Michael Park
> On Feb. 5, 2017, 12:48 a.m., Alex Clemmer wrote: > > 3rdparty/stout/include/stout/os/windows/fd.hpp, line 306 > > > > > > I'm a bit confused about the conversion logic here... if `left` is a > > CRT type, can't

Re: Review Request 54591: Introduced `WindowsFD` class which is analogous to an `int` in POSIX.

2017-02-05 Thread Alex Clemmer
> On Feb. 5, 2017, 8:48 a.m., Alex Clemmer wrote: > > 3rdparty/stout/include/stout/os/windows/fd.hpp, line 59 > > > > > > I'm not super up on the semantics of these newfangled C++11 constructor > > things, but is

Re: Review Request 54591: Introduced `WindowsFD` class which is analogous to an `int` in POSIX.

2017-02-05 Thread Michael Park
> On Feb. 5, 2017, 12:48 a.m., Alex Clemmer wrote: > > 3rdparty/stout/include/stout/os/windows/fd.hpp, line 306 > > > > > > I'm a bit confused about the conversion logic here... if `left` is a > > CRT type, can't

Re: Review Request 52382: Added stubs for OCI store.

2017-02-05 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52382/#review164298 --- Ship it! Ship It! - Jie Yu On Feb. 1, 2017, 1:17 a.m., Qian

Re: Review Request 54591: Introduced `WindowsFD` class which is analogous to an `int` in POSIX.

2017-02-05 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54591/ --- (Updated Feb. 5, 2017, 5:04 p.m.) Review request for mesos, Daniel Pravat and

Re: Review Request 54591: Introduced `WindowsFD` class which is analogous to an `int` in POSIX.

2017-02-05 Thread Michael Park
> On Feb. 5, 2017, 12:48 a.m., Alex Clemmer wrote: > > 3rdparty/stout/include/stout/os/windows/fd.hpp, line 59 > > > > > > I'm not super up on the semantics of these newfangled C++11 constructor > > things, but is

Re: Review Request 55625: Prevented certain kinds of gaming the quota system.

2017-02-05 Thread Benjamin Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55625/#review164287 --- Fix it, then Ship it! Thanks Benjamin! High level thought

Re: Review Request 54591: Introduced `WindowsFD` class which is analogous to an `int` in POSIX.

2017-02-05 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54591/#review164303 --- Ship it! I greatly appreciate the goal of maintaining the

Re: Review Request 54821: Refactored Docker::run() to make it only aware of docker cli options.

2017-02-05 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54821/#review164288 --- Fix it, then Ship it! This is a great refactor! Thanks Zhitao!

Re: Review Request 54638: Added agent flags '--oci_discovery' and '--oci_discovery_prefix'.

2017-02-05 Thread Jie Yu
> On Jan. 9, 2017, 9:11 a.m., Gilbert Song wrote: > > src/slave/flags.cpp, line 173 > > > > > > What elas types we might have? I would imaging 'registry' in the future - Jie

Re: Review Request 54638: Added agent flags '--oci_discovery' and '--oci_discovery_prefix'.

2017-02-05 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54638/#review164296 --- Ship it! Ship It! - Jie Yu On Feb. 1, 2017, 1:16 a.m., Qian

Re: Review Request 54595: Introduced an `os::dup` abstraction in stout.

2017-02-05 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54595/ --- (Updated Feb. 5, 2017, 6:14 p.m.) Review request for mesos, Daniel Pravat and

Re: Review Request 54595: Introduced an `os::dup` abstraction in stout.

2017-02-05 Thread Michael Park
> On Feb. 5, 2017, 12:59 a.m., Alex Clemmer wrote: > > 3rdparty/stout/include/stout/os/windows/dup.hpp, line 20 > > > > > > Tiny nits: Should we `#include` `try.hpp` and `unreachable.hpp` also? > > Also, in this

Re: Review Request 54592: Introduced an `os::lseek` abstraction in stout.

2017-02-05 Thread Michael Park
> On Feb. 5, 2017, 1:03 a.m., Alex Clemmer wrote: > > 3rdparty/stout/include/stout/os/lseek.hpp, line 26 > > > > > > Since the Windows signature uses `long` instead of `off_t`, I wonder if > > it is appropriate to

Re: Review Request 54592: Introduced an `os::lseek` abstraction in stout.

2017-02-05 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54592/ --- (Updated Feb. 5, 2017, 6:21 p.m.) Review request for mesos, Daniel Pravat and

Re: Review Request 54762: Introduced an `os::pipe` abstraction to stout.

2017-02-05 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54762/ --- (Updated Feb. 5, 2017, 6:27 p.m.) Review request for mesos, Daniel Pravat and

Re: Review Request 52382: Added stubs for OCI store.

2017-02-05 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52382/#review164300 --- src/Makefile.am (line 951)

Re: Review Request 54595: Introduced an `os::dup` abstraction in stout.

2017-02-05 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54595/#review164304 --- Ship it! Ship It! - Alex Clemmer On Feb. 6, 2017, 2:14

Re: Review Request 54821: Refactored Docker::run() to make it only aware of docker cli options.

2017-02-05 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54821/#review164301 --- Fix it, then Ship it! Thanks a lot for @zhitao's great

Re: Review Request 54821: Refactored Docker::run() to make it only aware of docker cli options.

2017-02-05 Thread haosdent huang
> On Feb. 6, 2017, 1:36 a.m., Jie Yu wrote: > > src/docker/docker.hpp, lines 141-142 > > > > > > I typically wrap comments in 70 char width. Let's do that in this file > > so that it's consistent with other

Re: Review Request 51052: Made mesos-docker-execute understand cgroups_enable_cfs.

2017-02-05 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51052/#review164307 --- Ship it! Ship It! - haosdent huang On Feb. 5, 2017, 11:09

Re: Review Request 55160: Added test for DockerContainerizer when `cgroups_enable_cfs` is set.

2017-02-05 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55160/#review164308 --- src/tests/containerizer/docker_containerizer_tests.cpp (line 39)

Re: Review Request 55160: Added test for DockerContainerizer when `cgroups_enable_cfs` is set.

2017-02-05 Thread haosdent huang
> On Feb. 6, 2017, 5:50 a.m., haosdent huang wrote: > > Test in CentOS 7.2. This looks pretty good expect some nits. Let's commit it after 1.2.0 release. - haosdent --- This is an automatically generated e-mail. To reply, visit:

Re: Review Request 56288: Improved the wording of what's logged on command health check timeouts.

2017-02-05 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56288/#review164314 --- Ship it! Ship It! - haosdent huang On Feb. 3, 2017, 7:50

Re: Review Request 55900: Improved style in `HealthChecker`.

2017-02-05 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55900/#review164311 --- Ship it! Ship It! - haosdent huang On Feb. 2, 2017, 4:54