Re: Review Request 65395: Refactored health checks to cleanly separate each different check.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65395/ --- (Updated Feb. 8, 2018, 5:49 p.m.) Review request for mesos, Alexander

Re: Review Request 65127: Windows: Enabled docker health checks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65127/ --- (Updated Feb. 8, 2018, 5:51 p.m.) Review request for mesos, Alexander

Re: Review Request 65571: Handle 'None' passed from the MasterDetector in 'Master::detect()'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65571/#review197095 --- PASS: Mesos patch 65571 was successfully built and tested.

Re: Review Request 65395: Refactored health checks to cleanly separate each different check.

> On Feb. 7, 2018, 1:35 p.m., Alexander Rukletsov wrote: > > src/checks/checker.cpp > > Line 86 (original), 89 (patched) > > > > > > please no abbreviations like this, it is unclear what this function > > does. Is

Re: Review Request 65395: Refactored health checks to cleanly separate each different check.

> On Feb. 2, 2018, 9:27 p.m., Andrew Schwartzmeyer wrote: > > src/checks/checker_process.hpp > > Lines 143-148 (original) > > > > > > Leaving a note to make sure this comment was kept around. thanks for catching

Re: Review Request 65396: Moved docker command check code inside health check library.

> On Feb. 2, 2018, 9:38 p.m., Andrew Schwartzmeyer wrote: > > src/checks/checker_process.cpp > > Lines 523-525 (patched) > > > > > > It's kind of funny they're quoting this. Not something to fix here, but > > it's

Re: Review Request 65396: Moved docker command check code inside health check library.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65396/ --- (Updated Feb. 8, 2018, 5:50 p.m.) Review request for mesos, Alexander

Re: Review Request 65419: Fixed quoting issues in docker executor command checks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65419/ --- (Updated Feb. 8, 2018, 5:50 p.m.) Review request for mesos, Alexander

Re: Review Request 65419: Fixed quoting issues in docker executor command checks.

> On Feb. 2, 2018, 9:43 p.m., Andrew Schwartzmeyer wrote: > > src/checks/checker_process.cpp > > Line 527 (original), 528-535 (patched) > > > > > > I'm not quite following this. Why do we always push

Review Request 65572: Fixed two slave recovery tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65572/ --- Review request for mesos and Greg Mann. Bugs: MESOS-8552

Review Request 65573: Fixed an error in the 1.3.2 CHANGELOG.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65573/ --- Review request for mesos, Jie Yu, Michael Park, and Vinod Kone. Repository:

Re: Review Request 65573: Fixed an error in the 1.3.2 CHANGELOG.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65573/#review197101 --- Ship it! Doesn't need to depend on the earlier chain? - Vinod

Re: Review Request 65407: Windows: Enabled tests that were blocked by MESOS-7604.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65407/ --- (Updated Feb. 8, 2018, 11:52 a.m.) Review request for mesos, Akash Gupta, Jie

Re: Review Request 65566: Returned profiles based on provider selectors in UriDiskProfileAdaptor.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65566/#review197122 --- Ship it! Ship It! - Jie Yu On Feb. 8, 2018, 6:39 a.m.,

Re: Review Request 65572: Fixed two slave recovery tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65572/#review197098 --- PASS: Mesos patch 65572 was successfully built and tested.

Re: Review Request 65469: Windows: Updated `internal::process:createChildProcess`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65469/ --- (Updated Feb. 8, 2018, 11:50 a.m.) Review request for mesos, Akash Gupta, Jie

Re: Review Request 65467: Windows: Added `internal::windows::set_inherit(WindowsFD, bool)`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65467/ --- (Updated Feb. 8, 2018, 11:49 a.m.) Review request for mesos, Akash Gupta, Jie

Re: Review Request 65469: Windows: Updated `internal::process:createChildProcess`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65469/#review197110 --- PASS: Mesos patch 65469 was successfully built and tested.

Re: Review Request 65553: Passed `ResourceProviderInfo` to disk profile adaptor modules.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65553/#review197097 --- Fix it, then Ship it!

Re: Review Request 65497: Added test to verify task-less executor is shutdown when re-subscribing.

> On Feb. 7, 2018, 6:19 p.m., Benjamin Mahler wrote: > > src/tests/mock_slave.cpp > > Lines 107-108 (original), 107-109 (patched) > > > > > > What's going on here? For the agent failover test, I need the failed

Re: Review Request 65465: Windows: Fixed recovery of Mesos containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65465/ --- (Updated Feb. 8, 2018, 11:54 a.m.) Review request for mesos, Akash Gupta, Jie

Review Request 65574: Windows: Fixed handle inheritance in `create_process` wrapper.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65574/ --- Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu. Repository:

Re: Review Request 65504: Made master set `launch_executor` in the RunTask(Group)Message.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65504/#review197099 --- src/master/master.cpp Line 3832 (original), 3832 (patched)

Re: Review Request 59987: Added protobuf map support.

> On Sept. 22, 2017, 11:14 p.m., Chun-Hung Hsiao wrote: > > 3rdparty/stout/include/stout/protobuf.hpp > > Line 607 (original), 640 (patched) > > > > > > How about a partial specialization for

Re: Review Request 65449: Fixed an issue where executor info linger on master if failed to launch.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65449/#review197107 --- Description: s/linger/lingers/ src/slave/slave.cpp Lines 1816

Re: Review Request 65558: Added helpers for supporting resource provider selectors.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65558/#review197115 --- Ship it! Ship It! - Jie Yu On Feb. 8, 2018, 4:37 a.m.,

Re: Review Request 65559: Updated tests for resource provider selector support.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65559/#review197116 --- Ship it! Ship It! - Jie Yu On Feb. 8, 2018, 6:38 a.m.,

Re: Review Request 65467: Windows: Added `internal::windows::set_inherit(WindowsFD, bool)`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65467/ --- (Updated Feb. 8, 2018, 2:35 p.m.) Review request for mesos, Akash Gupta, Jie

Re: Review Request 65409: Fixed `SlaveRecoveryTest.ReconcileTasksMissingFromSlave`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65409/ --- (Updated Feb. 8, 2018, 11:53 a.m.) Review request for mesos, Akash Gupta, Jie

Re: Review Request 65408: Windows: Ported `slave_recovery_tests.cpp`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65408/ --- (Updated Feb. 8, 2018, 11:53 a.m.) Review request for mesos, Akash Gupta, Jie

Re: Review Request 59988: Added a new protobuf message `MapMessage` for protobuf tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59988/#review197106 --- Ship it! Ship It! - Chun-Hung Hsiao On Dec. 20, 2017, 1:04

Re: Review Request 59987: Added protobuf map support.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/#review197100 --- Overall good. Thanks for your efforts!

Re: Review Request 65508: Allow curl program to be properly executed on Windows platform.

> On Feb. 6, 2018, 11:12 a.m., Andrew Schwartzmeyer wrote: > > Nit: Fix summary to be in past tense, and fix description to not be a copy > > of the summary. > > Jeff Coffler wrote: > This was a one-line commit, but I went ahead and added a second line. I'm > not sure why you said that

Re: Review Request 65403: Windows: Disabled `O_CLOEXEC` semantic mapping on Windows.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65403/ --- (Updated Feb. 8, 2018, 2:36 p.m.) Review request for mesos, Akash Gupta, Jie

Re: Review Request 65571: Handle 'None' passed from the MasterDetector in 'Master::detect()'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65571/#review197103 --- src/master/master.cpp Lines 2186 (patched)

Re: Review Request 65405: Implemented `net::socket()` for Windows using `WSASocket`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65405/ --- (Updated Feb. 8, 2018, 2:36 p.m.) Review request for mesos, Akash Gupta, Jie

Re: Review Request 65394: Added separate structs for health check runtime and check types.

> On Feb. 7, 2018, 1:18 p.m., Alexander Rukletsov wrote: > > src/checks/checks_runtime.hpp > > Lines 55-57 (patched) > > > > > > This is not quite true: you only mean command health checks. HTTP and > > TCP are

Re: Review Request 65394: Added separate structs for health check runtime and check types.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65394/ --- (Updated Feb. 8, 2018, 7:44 p.m.) Review request for mesos, Alexander

Re: Review Request 65405: Implemented `net::socket()` for Windows using `WSASocket`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65405/ --- (Updated Feb. 8, 2018, 11:51 a.m.) Review request for mesos, Akash Gupta, Jie

Re: Review Request 65573: Fixed an error in the 1.3.2 CHANGELOG.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65573/#review197102 --- PASS: Mesos patch 65573 was successfully built and tested.

Re: Review Request 65446: Added helper function for the agent to send `ExitedExecutorMessage`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65446/#review197105 --- Ship it! Ship It! - Vinod Kone On Feb. 5, 2018, 2:45 a.m.,

Re: Review Request 65554: Updated disk profile tests due to changes in the module interface.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65554/#review197112 --- Ship it! Ship It! - Jie Yu On Feb. 7, 2018, 9:05 p.m.,

Re: Review Request 65538: Added resource provider selectors in `disk_profile.proto`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65538/#review197113 --- Ship it! Ship It! - Jie Yu On Feb. 8, 2018, 4:36 a.m.,

Review Request 65575: Removed redundant comment from the command executor.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65575/ --- Review request for mesos, Anand Mazumdar, Greg Mann, Qian Zhang, and Vinod Kone.

Re: Review Request 59989: Added a test `ProtobufTest.JsonifyMap`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59989/#review197108 --- Ship it! Ship It! - Chun-Hung Hsiao On Dec. 20, 2017, 1:03

Re: Review Request 65575: Removed redundant comment from the command executor.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65575/#review197109 --- PASS: Mesos patch 65575 was successfully built and tested.

Re: Review Request 65553: Passed `ResourceProviderInfo` to disk profile adaptor modules.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65553/ --- (Updated Feb. 8, 2018, 10:22 p.m.) Review request for mesos, James DeFelice,

Re: Review Request 65508: Allow curl program to be properly executed on Windows platform.

> On Feb. 6, 2018, 7:12 p.m., Andrew Schwartzmeyer wrote: > > Nit: Fix summary to be in past tense, and fix description to not be a copy > > of the summary. This was a one-line commit, but I went ahead and added a second line. I'm not sure why you said that the description shouldn't be a copy

Re: Review Request 65508: Allow curl program to be properly executed on Windows platform.

> On Feb. 6, 2018, 7:12 p.m., Andrew Schwartzmeyer wrote: > > Nit: Fix summary to be in past tense, and fix description to not be a copy > > of the summary. > > Jeff Coffler wrote: > This was a one-line commit, but I went ahead and added a second line. I'm > not sure why you said that the

Re: Review Request 65497: Added test to verify task-less executor is shutdown when re-subscribing.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65497/#review197126 --- PASS: Mesos patch 65497 was successfully built and tested.

Re: Review Request 65508: Allowed curl program to be properly executed on Windows platform.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65508/ --- (Updated Feb. 8, 2018, 11:09 p.m.) Review request for mesos, Andrew

Re: Review Request 65518: Reaped the container process directly in Docker executor.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65518/#review197141 --- Ship it! Ship It! - Gaston Kleiman On Feb. 8, 2018, 5:03

Re: Review Request 65504: Made master set `launch_executor` in the RunTask(Group)Message.

> On Feb. 8, 2018, 12:03 p.m., Vinod Kone wrote: > > src/master/master.cpp > > Lines 5185-5186 (patched) > > > > > > this doesn't seem correct? > > > > shouldn't this be > > > > ``` > >

Re: Review Request 52064: Support for multiple versions of docs.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52064/#review197144 --- FAIL: Failed to apply the current review. Failed command:

Re: Review Request 65465: Windows: Fixed recovery of Mesos containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65465/#review197135 --- Re-ran ctest on Windows, make check on Linux, and manually tested

Re: Review Request 65509: Fixed CURL_ prefix in unit test name to succeed on Windows.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65509/ --- (Updated Feb. 8, 2018, 11:09 p.m.) Review request for mesos, Andrew

Re: Review Request 65508: Allow curl program to be properly executed on Windows platform.

> On Feb. 6, 2018, 7:12 p.m., Andrew Schwartzmeyer wrote: > > Nit: Fix summary to be in past tense, and fix description to not be a copy > > of the summary. > > Jeff Coffler wrote: > This was a one-line commit, but I went ahead and added a second line. I'm > not sure why you said that the

Re: Review Request 65578: Updated documentation for `UriDiskProfileAdaptor`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65578/#review197137 --- Ship it! Ship It! - Jie Yu On Feb. 8, 2018, 11:06 p.m.,

Re: Review Request 65448: Added a test to ensure master removes executors that never launched.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65448/#review197139 --- Fix it, then Ship it! src/tests/slave_tests.cpp Lines 4643

Re: Review Request 65448: Added a test to ensure master removes executors that never launched.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65448/#review197146 --- PASS: Mesos patch 65448 was successfully built and tested.

Review Request 65578: Updated documentation for `UriDiskProfileAdaptor`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65578/ --- Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu. Bugs:

Re: Review Request 65509: Fixed CURL_ prefix in unit test name to succeed on Windows.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65509/#review197133 --- src/tests/environment.cpp Line 219 (original), 219-224 (patched)

Re: Review Request 65572: Fixed two slave recovery tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65572/#review197140 --- Ship it! Ship It! - Vinod Kone On Feb. 9, 2018, 1:12 a.m.,

Re: Review Request 65419: Fixed quoting issues in docker executor command checks.

> On Feb. 2, 2018, 1:43 p.m., Andrew Schwartzmeyer wrote: > > src/checks/checker_process.cpp > > Lines 535-537 (original), 543-547 (patched) > > > > > > Can you explain why this was changed a little more? Maybe an

Re: Review Request 64387: Windows: Ported docker health check tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64387/#review197130 --- src/tests/health_check_tests.cpp Lines 109 (patched)

Re: Review Request 65575: Removed redundant comment from the command executor.

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

Re: Review Request 52064: Support for multiple versions of docs.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52064/ --- (Updated Feb. 9, 2018, 1:46 a.m.) Review request for mesos, haosdent huang and

Re: Review Request 65580: Improved logging for disk resources.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65580/#review197143 --- Ship it! Ship It! - Jie Yu On Feb. 9, 2018, 1:09 a.m.,

Re: Review Request 65395: Refactored health checks to cleanly separate each different check.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65395/#review197118 --- Thanks for this refactoring, the code looks much better and is

Re: Review Request 65395: Refactored health checks to cleanly separate each different check.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65395/#review197123 --- src/checks/checker_process.cpp Lines 97-100 (original), 100-103

Re: Review Request 65571: Handle 'None' passed from the MasterDetector in 'Master::detect()'.

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

Re: Review Request 62800: Created cgroups under systemd hierarchy in LinuxLauncher.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62800/#review197079 --- Patch looks great! Reviews applied: [62798, 62799, 62800]

Re: Review Request 65127: Windows: Enabled docker health checks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65127/#review197127 --- src/checks/checker_process.cpp Lines 1017-1020 (patched)

Re: Review Request 65510: Enabled `curl_uri_fetcher.cpp` tests on Windows platform.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65510/ --- (Updated Feb. 8, 2018, 11:13 p.m.) Review request for mesos, Andrew

Review Request 65580: Improved logging for disk resources.

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

Re: Review Request 65569: Install ping for docker build.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65569/#review197092 --- PASS: Mesos patch 65569 was successfully built and tested.

Re: Review Request 65570: WIP: Attached/detached volume directory for task which has volume specified.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65570/#review197093 --- PASS: Mesos patch 65570 was successfully built and tested.

Re: Review Request 65127: Windows: Enabled docker health checks.

> On Feb. 2, 2018, 9:53 p.m., Andrew Schwartzmeyer wrote: > > src/checks/checker_process.hpp > > Lines 47 (patched) > > > > > > Should we tag this so a push to their image doesn't break us? I'm not sure how to solve

Re: Review Request 65469: Windows: Updated `internal::process:createChildProcess`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65469/#review197152 --- Patch looks great! Reviews applied: [65397, 65398, 65399, 65400,

Re: Review Request 65314: Removed code which is not used.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65314/#review197083 --- PASS: Mesos patch 65314 was successfully built and tested.

Re: Review Request 65314: Removed code which is not used.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65314/ --- (Updated Feb. 8, 2018, 1:59 p.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 65313: Refactored authorization logic in the agent.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65313/ --- (Updated Feb. 8, 2018, 1:59 p.m.) Review request for mesos, Benjamin Hindman

Review Request 65569: Install ping for docker build.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65569/ --- Review request for mesos, Michael Park and Vinod Kone. Repository: mesos

Review Request 65570: WIP: Attached/detached volume directory for task which has volume specified.

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

Re: Review Request 65570: WIP: Attached/detached volume directory for task which has volume specified.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65570/ --- (Updated Feb. 8, 2018, 11:41 p.m.) Review request for mesos. Bugs:

Re: Review Request 65556: Made the default executor treat agent disconnections more gracefully.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65556/#review197088 --- Patch looks great! Reviews applied: [65548, 65549, 65550, 65551,