Re: Review Request 55400: Fix compilation error for clang-3.5 type deduction error.

2017-01-10 Thread Michael Park
> On Jan. 10, 2017, 5:40 p.m., Joseph Wu wrote: > > src/slave/containerizer/mesos/io/switchboard.cpp, line 1598 > > > > > > Technically, we expect the return type to be `Future`, > > to match the return type of

Re: Review Request 55400: Fix compilation error for clang-3.5 type deduction error.

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

Re: Review Request 55025: Transitioned `BUILD_DIR` to use Unix-style paths on Windows.

2017-01-10 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55025/#review161172 --- Ship it! Ship It! - Joseph Wu On Dec. 24, 2016, 3:09 a.m.,

Re: Review Request 55024: Windows: Start the socket stack in `process::initialize`.

2017-01-10 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55024/#review161171 --- 3rdparty/libprocess/src/process.cpp (lines 1040 - 1041)

Re: Review Request 55022: Windows: Cause errors to be correctly reported in `io::read`.

2017-01-10 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55022/#review161170 --- The change LGTM, but it might need a rebase due to some changes

Re: Review Request 55400: Fix compilation error for clang-3.5 type deduction error.

2017-01-10 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55400/#review161169 --- Fix it, then Ship it!

Re: Review Request 54794: Cleaned up teardown tests slightly.

2017-01-10 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54794/#review161168 --- Ship it! Ship It! - Vinod Kone On Dec. 15, 2016, 9:13 p.m.,

Re: Review Request 54793: Prevented task launches that reuse unreachable task IDs.

2017-01-10 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54793/#review161167 --- src/tests/partition_tests.cpp (line 3384)

Re: Review Request 54408: Replaced `Master::Framework::active` with a new `state` enum value.

2017-01-10 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54408/#review161163 --- Fix it, then Ship it! Thanks for following through.

Review Request 55400: Fix compilation error for clang-3.5 type deduction error.

2017-01-10 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55400/ --- Review request for mesos and Joseph Wu. Repository: mesos Description

Re: Review Request 54590: Removed unused peek function.

2017-01-10 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54590/ --- (Updated Jan. 10, 2017, 5:22 p.m.) Review request for mesos, Daniel Pravat and

Re: Review Request 54590: Removed unused `peek` function.

2017-01-10 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54590/#review161164 --- Ship it! LGTM. I'd mention the original review in the commit

Re: Review Request 54407: Refactored Master::removeFramework to use Master::deactivate.

2017-01-10 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54407/#review161159 --- src/master/master.cpp

Re: Review Request 53803: Added new libprocess socket tests.

2017-01-10 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53803/ --- (Updated Jan. 11, 2017, 12:52 a.m.) Review request for mesos, Benjamin Mahler

Re: Review Request 53802: Eliminated an EOF race condition in libprocess SSL socket.

2017-01-10 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53802/ --- (Updated Jan. 11, 2017, 12:51 a.m.) Review request for mesos, Benjamin

Re: Review Request 54232: Shutdown tasks of completed frameworks on agent re-registration.

2017-01-10 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54232/#review161152 --- src/master/master.cpp (lines 5689 - 5728)

Re: Review Request 53712: Added `system` environment variables in ` execvpe.cpp`.

2017-01-10 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53712/#review161154 --- Looks like this is a diff-of-a-diff. I'd expect the actual diff

Re: Review Request 54232: Shutdown tasks of completed frameworks on agent re-registration.

2017-01-10 Thread Vinod Kone
> On Jan. 3, 2017, 10:51 p.m., Vinod Kone wrote: > > src/master/master.cpp, line 5512 > > > > > > inline this? > > Neil Conway wrote: > To me, using a separate function was more readable than writing the logic

Re: Review Request 53802: Eliminated an EOF race condition in libprocess SSL socket.

2017-01-10 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53802/ --- (Updated Jan. 11, 2017, 12:11 a.m.) Review request for mesos, Benjamin Mahler

Re: Review Request 55395: Cleaned up `Master::updateTask` slightly.

2017-01-10 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55395/#review161149 --- Ship it! Ship It! - Vinod Kone On Jan. 10, 2017, 10:35

Re: Review Request 54183: Improved management of unreachable and completed tasks in master.

2017-01-10 Thread Neil Conway
> On Jan. 3, 2017, 10:25 p.m., Vinod Kone wrote: > > src/master/master.cpp, line 5507 > > > > > > can we inline this? > > Neil Conway wrote: > We could, but personally I find it more readable to make it a

Re: Review Request 55394: Changed `Master::updateTask` to allow terminal tasks to be updated.

2017-01-10 Thread Neil Conway
> On Jan. 10, 2017, 11:51 p.m., Vinod Kone wrote: > > What's the motivation for this change? This is needed by https://reviews.apache.org/r/54232/ -- if we're going to update the task from `UNREACHABLE` to `KILLED`, we need `updateTask()` to be able to transition a task from one "terminal"

Re: Review Request 55394: Changed `Master::updateTask` to allow terminal tasks to be updated.

2017-01-10 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55394/#review161145 --- What's the motivation for this change? - Vinod Kone On Jan.

Re: Review Request 54183: Improved management of unreachable and completed tasks in master.

2017-01-10 Thread Vinod Kone
> On Jan. 3, 2017, 10:25 p.m., Vinod Kone wrote: > > src/master/master.cpp, line 5496 > > > > > > it's not necessary that the agent has failed over if we are here right? > > Neil Conway wrote: > Do you mean

Re: Review Request 55296: Used `jsonify` in `operator<<` for `JSON::*` to reduce duplicate code.

2017-01-10 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55296/ --- (Updated Jan. 10, 2017, 3:22 p.m.) Review request for mesos, Alexander Rojas

Re: Review Request 54232: Shutdown tasks of completed frameworks on agent re-registration.

2017-01-10 Thread Neil Conway
> On Jan. 3, 2017, 10:51 p.m., Vinod Kone wrote: > > src/master/master.cpp, line 5512 > > > > > > inline this? To me, using a separate function was more readable than writing the logic inline. Happy to change it

Re: Review Request 54793: Prevented task launches that reuse unreachable task IDs.

2017-01-10 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54793/ --- (Updated Jan. 10, 2017, 10:36 p.m.) Review request for mesos and Vinod Kone.

Re: Review Request 54407: Refactored Master::removeFramework to use Master::deactivate.

2017-01-10 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54407/ --- (Updated Jan. 10, 2017, 10:35 p.m.) Review request for mesos and Vinod Kone.

Re: Review Request 54312: Added TASK_UNREACHABLE to master's state-summary endpoint.

2017-01-10 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54312/ --- (Updated Jan. 10, 2017, 10:35 p.m.) Review request for mesos and Vinod Kone.

Review Request 55395: Cleaned up `Master::updateTask` slightly.

2017-01-10 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55395/ --- Review request for mesos and Vinod Kone. Bugs: MESOS-6619

Review Request 55394: Changed `Master::updateTask` to allow terminal tasks to be updated.

2017-01-10 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55394/ --- Review request for mesos and Vinod Kone. Bugs: MESOS-6619

Re: Review Request 54232: Shutdown tasks of completed frameworks on agent re-registration.

2017-01-10 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54232/ --- (Updated Jan. 10, 2017, 10:32 p.m.) Review request for mesos and Vinod Kone.

Re: Review Request 54081: Added `--pidfile` option to master and agent binaries.

2017-01-10 Thread Greg Mann
> On Dec. 6, 2016, 10:42 p.m., Benjamin Mahler wrote: > > Thanks Ilya! > > > > Have you looked at other pidfile related libraries? Looks like BSD provides > > some functions for this (they're also available on Linux): > > https://www.freebsd.org/cgi/man.cgi?query=pidfile=3=FreeBSD+6.1-RELEASE

Re: Review Request 55388: Added `system` environment variables in `mesos-docker-executor`.

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

Re: Review Request 55177: Fixed scheme handling in URL::parse().

2017-01-10 Thread Benjamin Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55177/#review161107 --- Ship it! Ship It! - Benjamin Mahler On Jan. 6, 2017, 11:37

Re: Review Request 55381: Added test for framework upgrading to multi-role capability.

2017-01-10 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55381/#review161102 --- Bad patch! Reviews applied: [55381] Failed command:

Review Request 55388: Added `system` environment variables in `mesos-docker-executor`.

2017-01-10 Thread Daniel Pravat
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55388/ --- Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joseph

Re: Review Request 53712: Added `system` environment variables in ` execvpe.cpp`.

2017-01-10 Thread Daniel Pravat
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53712/ --- (Updated Jan. 10, 2017, 7:01 p.m.) Review request for mesos, Alex Naparu,

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

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

Review Request 55381: Added test for framework upgrading to multi-role capability.

2017-01-10 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55381/ --- Review request for mesos, Benjamin Mahler, Jay Guo, and Guangya Liu. Bugs:

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-10 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- (Updated Jan. 10, 2017, 3:49 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55269: Avoided use of CHECK macros.

2017-01-10 Thread Benjamin Bannier
> On Jan. 7, 2017, 12:28 a.m., Kapil Arya wrote: > > LGTM. Have you run any tests where the existing `CHECK_SOME` would fail and > > the new code will behave properly? It might be worth running it at least > > for one test. Maybe, in one of the containerizer value, just set it to > >

Re: Review Request 55269: Avoided use of CHECK macros.

2017-01-10 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55269/ --- (Updated Jan. 10, 2017, 2:19 p.m.) Review request for mesos and Kapil Arya.

Re: Review Request 55268: Avoided use of CHECK macros in libprocess.

2017-01-10 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55268/ --- (Updated Jan. 10, 2017, 2:19 p.m.) Review request for mesos and Kapil Arya.

Re: Review Request 55269: Avoided use of CHECK macros.

2017-01-10 Thread Benjamin Bannier
> On Jan. 7, 2017, 12:28 a.m., Kapil Arya wrote: > > LGTM. Have you run any tests where the existing `CHECK_SOME` would fail and > > the new code will behave properly? It might be worth running it at least > > for one test. Maybe, in one of the containerizer value, just set it to > >

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-10 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- (Updated Jan. 10, 2017, 1:29 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55296: Used `jsonify` in `operator<<` for `JSON::*` to reduce duplicate code.

2017-01-10 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55296/#review161047 --- Ship it! Ship It! - Alexander Rojas On Jan. 10, 2017, 1:36

Re: Review Request 53468: Show the pailer at the right side in WebUI.

2017-01-10 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53468/ --- (Updated Jan. 10, 2017, 8:03 a.m.) Review request for mesos and Vinod Kone.