Re: Review Request 56530: Prevent offers for old agents being sent to MULTI_ROLE frameworks.

2017-02-14 Thread Jay Guo
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56530/ --- (Updated Feb. 15, 2017, 2:44 p.m.) Review request for mesos, Benjamin Bannier,

Re: Review Request 56692: Silenced a GMock warning in a test.

2017-02-14 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56692/#review165652 --- Patch looks great! Reviews applied: [56692] Passed command:

Re: Review Request 56689: Windows: Prevented crash if sandbox `stdout` file already exists.

2017-02-14 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56689/#review165642 --- Patch looks great! Reviews applied: [55022, 55023, 55024, 55030,

Re: Review Request 56645: Added agent capabilities to `/state`(v0) endpoint of agent.

2017-02-14 Thread Jay Guo
> On Feb. 14, 2017, 5:57 p.m., Benjamin Bannier wrote: > > src/slave/http.cpp, lines 1246-1247 > > > > > > I see we have a type `SlaveInfo::Capability` defined, but don't store > > any values there. Shouldn't we

Re: Review Request 56587: Fixed a crash on the master upon receiving an invalid inverse offer.

2017-02-14 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56587/#review165631 --- Patch looks great! Reviews applied: [56587] Passed command:

Re: Review Request 51624: Implemented 'GetAgent' call in v1 agent API.

2017-02-14 Thread haosdent huang
> On Jan. 31, 2017, 11:45 p.m., Vinod Kone wrote: > > Ship It! > > Vinod Kone wrote: > Can you commit this @haosdent? Sure, let me commit. - haosdent --- This is an automatically generated e-mail. To reply, visit:

Re: Review Request 51624: Implemented 'GetAgent' call in v1 agent API.

2017-02-14 Thread Vinod Kone
> On Jan. 31, 2017, 11:45 p.m., Vinod Kone wrote: > > Ship It! Can you commit this @haosdent? - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51624/#review163743

Re: Review Request 56591: Stout: Removed MSVC compiler warnings.

2017-02-14 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56591/#review165596 --- 3rdparty/stout/include/stout/abort.hpp (lines 45 - 54)

Re: Review Request 55547: Windows: Standardize on win32 environment, transition away from CRT.

2017-02-14 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55547/#review165625 --- 3rdparty/stout/include/stout/os/windows/environment.hpp (line

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

2017-02-14 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56288/#review165624 --- Ship it! Ship It! - Vinod Kone On Feb. 10, 2017, 11:17

Re: Review Request 56681: Use glog to log EXIT() messages.

2017-02-14 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56681/#review165610 --- Patch looks great! Reviews applied: [56680, 56681] Passed

Review Request 56692: Silenced a GMock warning in a test.

2017-02-14 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56692/ --- Review request for mesos, Benjamin Bannier and Michael Park. Repository: mesos

Review Request 56689: Windows: Prevented crash if sandbox `stdout` file already exists.

2017-02-14 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56689/ --- Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.

Re: Review Request 56587: Fixed a crash on the master upon receiving an invalid inverse offer.

2017-02-14 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56587/#review165594 --- Fix it, then Ship it! src/master/master.cpp (line 4731)

Re: Review Request 56505: Added Windows support for Docker executor.

2017-02-14 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56505/#review165583 --- src/docker/docker.hpp (lines 41 - 47)

Re: Review Request 56504: Stout: Made `os::mktemp` fully cross-platform.

2017-02-14 Thread Alex Clemmer
> On Feb. 14, 2017, 11:05 p.m., Joseph Wu wrote: > > When we switched `mkdtemp` (directory version), that exposed some odd > > behavior on OSX due to how lengthy OSX's default temporary directory is. > > The IO Switchboard tests there ran into a 108 character limit for Unix > > sockets. It

Re: Review Request 56367: Stout: Windows: patch `os::killtree` to terminate job objects.

2017-02-14 Thread Andrew Schwartzmeyer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56367/#review165590 --- Test again review bot! - Andrew Schwartzmeyer On Feb. 8, 2017,

Re: Review Request 56368: Libprocess: Windows: Remove `CREATE_JOB` parent hook.

2017-02-14 Thread Andrew Schwartzmeyer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56368/#review165589 --- Test again review bot! - Andrew Schwartzmeyer On Feb. 8, 2017,

Re: Review Request 56504: Stout: Made `os::mktemp` fully cross-platform.

2017-02-14 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56504/#review165582 --- Ship it! When we switched `mkdtemp` (directory version), that

Re: Review Request 56052: Added the 'Secret' protobuf message.

2017-02-14 Thread Albert Strasheim
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56052/#review165587 --- include/mesos/mesos.proto (line 1994)

Re: Review Request 56675: Silence MSVC compiler warnings in libmesos.

2017-02-14 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56675/#review165586 --- Patch looks great! Reviews applied: [55022, 55023, 55024, 55030,

Re: Review Request 55749: Added CMake to standard documentation.

2017-02-14 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55749/#review165578 --- Ship it! Whitespace nits that I can fix before committing.

Re: Review Request 55547: Windows: Standardize on win32 environment, transition away from CRT.

2017-02-14 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55547/#review165576 --- Ship it! A few minor nits below that I will fix before

Review Request 56680: Apply glog to agent exit messages.

2017-02-14 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56680/ --- Review request for mesos and haosdent huang. Bugs: MESOS-7115

Review Request 56681: Use glog to log EXIT() messages.

2017-02-14 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56681/ --- Review request for mesos and haosdent huang. Bugs: MESOS-7115

Re: Review Request 56493: Port_mapping isolator: do not depend on interface speed.

2017-02-14 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56493/#review165572 --- Patch looks great! Reviews applied: [56493] Passed command:

Re: Review Request 56213: Added check tests for command executor.

2017-02-14 Thread Gastón Kleiman
> On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp, line 177 > > > > > > s/agent/slave/ > > > > here and everywhere else. > > > > we are not doing this change yet. We

Re: Review Request 56587: Fixed a crash on the master upon receiving an invalid inverse offer.

2017-02-14 Thread Anand Mazumdar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56587/ --- (Updated Feb. 14, 2017, 8:22 p.m.) Review request for mesos and Joseph Wu.

Re: Review Request 56587: Fixed a crash on the master upon receiving an invalid inverse offer.

2017-02-14 Thread Joseph Wu
> On Feb. 13, 2017, 11:51 a.m., Joseph Wu wrote: > > src/master/master.cpp, lines 4756-4777 > > > > > > Moving the error above the CHECK leaves out the log message in one case: > > > > *

Re: Review Request 56675: Silence MSVC compiler warnings in libmesos.

2017-02-14 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56675/#review165563 --- I am not sure what other solutions you explored, but I believe a

Re: Review Request 56505: Added Windows support for Docker executor.

2017-02-14 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56505/ --- (Updated Feb. 14, 2017, 7:06 p.m.) Review request for mesos, Andrew

Re: Review Request 56657: Add support for local resolution of Docker images.

2017-02-14 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56657/#review165557 --- Patch looks great! Reviews applied: [56657] Passed command:

Re: Review Request 56212: Added support for general checks to command executor.

2017-02-14 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56212/#review165550 --- src/launcher/executor.cpp (lines 848 - 854)

Re: Review Request 55547: Windows: Standardize on win32 environment, transition away from CRT.

2017-02-14 Thread Alex Clemmer
> On Jan. 24, 2017, 2:23 a.m., Joseph Wu wrote: > > 3rdparty/stout/include/stout/os/raw/environment.hpp, lines 108-120 > > > > > > Should we also move away from functions like `os::execvpe`? If so, we > > would be

Re: Review Request 55547: Windows: Standardize on win32 environment, transition away from CRT.

2017-02-14 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55547/ --- (Updated Feb. 14, 2017, 6:47 p.m.) Review request for mesos, Andrew

Review Request 56675: Silence MSVC compiler warnings in libmesos.

2017-02-14 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56675/ --- Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.

Re: Review Request 56210: Reused previous task status to generate a new one in command executor.

2017-02-14 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56210/#review165539 --- src/launcher/executor.cpp (line 828)

Re: Review Request 56493: Port_mapping isolator: do not depend on interface speed.

2017-02-14 Thread Pierre Cheynier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56493/ --- (Updated Feb. 14, 2017, 6:35 p.m.) Review request for mesos and Jie Yu.

Review Request 56657: Add support for local resolution of Docker images.

2017-02-14 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56657/ --- Review request for mesos and Jie Yu. Bugs: MESOS-7089

Re: Review Request 56208: Updated checks library with general check support.

2017-02-14 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56208/#review165521 --- Many of the comments here also apply to the current code in

Re: Review Request 56611: Relax perf version check for Arch Linux.

2017-02-14 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56611/#review165532 --- Patch looks great! Reviews applied: [56611] Passed command:

Re: Review Request 56611: Relax perf version check for Arch Linux.

2017-02-14 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56611/ --- (Updated Feb. 14, 2017, 3:59 p.m.) Review request for mesos and Neil Conway.

Re: Review Request 56618: Updated common Mesos code to use 'AuthenticationContext'.

2017-02-14 Thread Alexander Rojas
> On Feb. 14, 2017, 4:12 p.m., Jan Schlicht wrote: > > src/common/http.hpp, line 132 > > > > > > Why `const` when you're returning a value? so you cannot assign to the returned value, i.e. you cannot do

Re: Review Request 56619: Updated Mesos handlers to use 'AuthenticationContext'.

2017-02-14 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56619/#review165509 --- This look great already. But I'll need more time to deeply review

Re: Review Request 56618: Updated common Mesos code to use 'AuthenticationContext'.

2017-02-14 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56618/#review165511 --- This look great already. But I'll need more time to deeply review

Re: Review Request 56601: Used single quotes instead of backticks in error message.

2017-02-14 Thread Benjamin Bannier
> On Feb. 14, 2017, 2:48 p.m., Gastón Kleiman wrote: > > I think that we should also update `checks/checker.cpp` and > > `checks/health_checker.cpp`: > > > > ``` > > $ ag --cpp --nomultiline '^\s*[^/"]+".*`' > > checks/checker.cpp > > 60:"Check's `CommandInfo` is invalid: " +

Re: Review Request 56652: Windows: Change ZK patch to fix macro redefinition warnings on MSVC.

2017-02-14 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56652/#review165506 --- Bad patch! Reviews applied: [56652, 56594, 56593, 56592, 56591,

Re: Review Request 56601: Used single quotes instead of backticks in error message.

2017-02-14 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56601/#review165502 --- Fix it, then Ship it! I think that we should also update

Re: Review Request 56646: Fixed indentations in master/http.cpp.

2017-02-14 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56646/#review165498 --- Patch looks great! Reviews applied: [56644, 56645, 56646]

Re: Review Request 54846: Removed `docker exec` when perform health checks in docker executor.

2017-02-14 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54846/#review165493 --- Ship it! Ship It! - Gastón Kleiman On Dec. 18, 2016, 5:30

Review Request 56652: Windows: Change ZK patch to fix macro redefinition warnings on MSVC.

2017-02-14 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56652/ --- Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.

Re: Review Request 56530: Prevent offers for old agents being sent to MULTI_ROLE frameworks.

2017-02-14 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56530/#review165485 --- Patch looks great! Reviews applied: [56530] Passed command:

Re: Review Request 56376: Updated allocator test to support create multi role framework.

2017-02-14 Thread Guangya Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56376/ --- (Updated 二月 14, 2017, 10:40 a.m.) Review request for mesos, Benjamin Mahler

Re: Review Request 56376: Updated allocator test to support create multi role framework.

2017-02-14 Thread Guangya Liu
> On 二月 13, 2017, 11:31 p.m., Benjamin Mahler wrote: > > src/tests/hierarchical_allocator_tests.cpp, lines 221-222 > > > > > > You should probably do this only if the caller did not specify the > > multi role

Re: Review Request 56645: Added agent capabilities to `/state`(v0) endpoint of agent.

2017-02-14 Thread Benjamin Bannier
> On Feb. 14, 2017, 10:57 a.m., Benjamin Bannier wrote: > > src/slave/http.cpp, lines 1246-1247 > > > > > > I see we have a type `SlaveInfo::Capability` defined, but don't store > > any values there. Shouldn't we

Re: Review Request 56645: Added agent capabilities to `/state`(v0) endpoint of agent.

2017-02-14 Thread Jay Guo
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56645/ --- (Updated Feb. 14, 2017, 6:19 p.m.) Review request for mesos, Benjamin Bannier,

Re: Review Request 56644: Added a member variable in Slave to store slave capabilities.

2017-02-14 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56644/#review165480 --- I am not sure we need this here, see

Re: Review Request 56645: Added agent capabilities to `/state`(v0) endpoint of agent.

2017-02-14 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56645/#review165477 --- src/common/http.hpp (lines 123 - 130)

Re: Review Request 56591: Stout: Removed MSVC compiler warnings.

2017-02-14 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56591/ --- (Updated Feb. 14, 2017, 9:05 a.m.) Review request for mesos, Andrew

Re: Review Request 56592: Libprocess: Removed MSVC compiler warnings.

2017-02-14 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56592/ --- (Updated Feb. 14, 2017, 8:58 a.m.) Review request for mesos, Andrew