Re: Review Request 62472: Fixed the ordering of Mesos containerizer isolators.

2018-04-19 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62472/#review201608 --- Patch looks great! Reviews applied: [62472] Passed command:

Re: Review Request 66311: Implement recovery of resource provider manager.

2018-04-19 Thread Chun-Hung Hsiao
> On April 19, 2018, 6 a.m., Chun-Hung Hsiao wrote: > > src/resource_provider/registrar.cpp > > Lines 193-203 (original), 194-203 (patched) > > > > > > How about moving this into

Re: Review Request 66310: Passed on registrar when constructing resource provider manager.

2018-04-19 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66310/#review201606 --- Ship it! Ship It! - Chun-Hung Hsiao On April 19, 2018,

Re: Review Request 66308: Delayed construction of the agent's resource provider manager.

2018-04-19 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66308/#review201605 --- src/slave/slave.cpp Lines 8849 (patched)

Re: Review Request 66526: Renamed resource provider `AgentRegistrar` to `GenericRegistrar`.

2018-04-19 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66526/#review201604 --- Ship it! Ship It! - Chun-Hung Hsiao On April 19, 2018,

Re: Review Request 66309: Externalized creation of resource provider manager backing storage.

2018-04-19 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66309/#review201603 --- Ship it! Ship It! - Chun-Hung Hsiao On April 19, 2018, 9:19

Re: Review Request 66309: Externalized creation of resource provider manager backing storage.

2018-04-19 Thread Chun-Hung Hsiao
> On April 18, 2018, 9:31 p.m., Chun-Hung Hsiao wrote: > > src/resource_provider/registrar.hpp > > Lines 71-73 (original), 73-75 (patched) > > > > > > I was wondering that, since we have a generic storage-backed

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-04-19 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66163/ --- (Updated April 20, 2018, 2:34 a.m.) Review request for mesos, Andrew

Re: Review Request 61118: Building gRPC support in libprocess with CMake.

2018-04-19 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61118/ --- (Updated April 20, 2018, 2:31 a.m.) Review request for mesos, Andrew

Re: Review Request 61096: Building gRPC with CMake.

2018-04-19 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61096/ --- (Updated April 20, 2018, 2:28 a.m.) Review request for mesos, Andrew

Review Request 66727: Cherry-picked gRPC PR15128 for Windows compilation.

2018-04-19 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66727/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, and Joseph Wu.

Review Request 66726: Made CMake's `FindZLIB` module be able to find Zlib on Windows.

2018-04-19 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66726/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, and Joseph Wu.

Re: Review Request 66696: Updated documentation for operation feedback.

2018-04-19 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66696/#review201600 --- Patch looks great! Reviews applied: [66696] Passed command:

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-04-19 Thread Chun-Hung Hsiao
> On April 12, 2018, 5:06 p.m., Andrew Schwartzmeyer wrote: > > cmake/CompilationConfigure.cmake > > Lines 387-390 (patched) > > > > > > Do you know which targets actually require `ENABLE_GRPC` set as a > >

Re: Review Request 66227: Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operator API.

2018-04-19 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66227/#review201596 --- FAIL: Failed to apply the dependent review: 66049. Failed

Re: Review Request 66725: Improved error message in the fetcher.

2018-04-19 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66725/#review201595 --- PASS: Mesos patch 66725 was successfully built and tested.

Re: Review Request 66708: Refactored sending a TASK_DROPPED status update.

2018-04-19 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66708/#review201593 --- Ship it! Ship It! - Qian Zhang On April 20, 2018, 2:09

Re: Review Request 66705: Propagated executor sandbox creation errors.

2018-04-19 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66705/#review201592 --- Ship it! Ship It! - Qian Zhang On April 20, 2018, 1:52

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-04-19 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66621/#review201591 --- FAIL: Failed to apply the current review. Failed command:

Re: Review Request 66683: Updated address field of new CLI config to accept URLs.

2018-04-19 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66683/#review201590 --- Patch looks great! Reviews applied: [66683] Passed command:

Re: Review Request 66227: Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operator API.

2018-04-19 Thread Zhitao Li
> On April 17, 2018, 6:25 p.m., Chun-Hung Hsiao wrote: > > src/tests/api_tests.cpp > > Lines 3688 (patched) > > > > > > Let's try to split the test into two so each test verifies one > > operation. > > > >

Re: Review Request 66439: Windows: Made `protobuf::write()` use CRT file descriptor explicitly.

2018-04-19 Thread Andrew Schwartzmeyer
> On April 19, 2018, 2:07 p.m., Joseph Wu wrote: > > 3rdparty/stout/include/stout/protobuf.hpp > > Line 83 (original), 87-95 (patched) > > > > > > I'm a bit surprised this doesn't cause problems with tests like > >

Re: Review Request 66568: Dropped combined operations with GROW and SHRINK volumes.

2018-04-19 Thread Greg Mann
> On April 19, 2018, 9:58 p.m., Greg Mann wrote: > > src/master/master.cpp > > Line 4512 (original), 4515 (patched) > > > > > > Nit: there's an extra space after the comma. after the colon I mean :) - Greg

Re: Review Request 66219: Added helper functions to create grow and shrink volume in test.

2018-04-19 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66219/#review201587 --- Ship it! Ship It! - Greg Mann On April 11, 2018, 9:18 p.m.,

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-04-19 Thread Clément Michaud
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66621/ --- (Updated avr. 19, 2018, 11:10 après-midi) Review request for mesos, Alexander

Review Request 66725: Improved error message in the fetcher.

2018-04-19 Thread Meng Zhu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66725/ --- Review request for mesos and Gilbert Song. Repository: mesos Description

Re: Review Request 62472: Fixed the ordering of Mesos containerizer isolators.

2018-04-19 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62472/#review201581 --- PASS: Mesos patch 62472 was successfully built and tested.

Re: Review Request 66431: Windows: Fixed `os::read()` to use `ReadFile()`.

2018-04-19 Thread Andrew Schwartzmeyer
> On April 18, 2018, 11:55 a.m., Joseph Wu wrote: > > 3rdparty/stout/include/stout/os/windows/read.hpp > > Lines 39-44 (patched) > > > > > > So you're saying that `ReadFile` reads all the existing data on the > >

Re: Review Request 66049: Added offer operation to grow and shrink persistent volumes.

2018-04-19 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66049/#review201577 --- Ship it! Ship It! - Greg Mann On April 19, 2018, 4:28 p.m.,

Re: Review Request 66568: Dropped combined operations with GROW and SHRINK volumes.

2018-04-19 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66568/#review201570 --- src/master/master.cpp Line 4512 (original), 4515 (patched)

Re: Review Request 66531: Added new authorization for `UpdateVolume`.

2018-04-19 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66531/#review201569 --- Fix it, then Ship it! include/mesos/authorizer/acls.proto

Re: Review Request 66531: Added new authorization for `UpdateVolume`.

2018-04-19 Thread Greg Mann
> On April 17, 2018, 11:14 p.m., Greg Mann wrote: > > src/master/master.cpp > > Lines 4924-4925 (original), 4980-4981 (patched) > > > > > > You need to finish implementing authorization here, and below for > >

Re: Review Request 66532: Added test for authorization actions for `UPDATE_VOLUME`.

2018-04-19 Thread Greg Mann
> On April 17, 2018, 11:24 p.m., Greg Mann wrote: > > src/tests/authorization_tests.cpp > > Line 1979 (original), 1979 (patched) > > > > > > Could you also add an end-to-end test of authorization for these > >

Re: Review Request 62472: Fixed the ordering of Mesos containerizer isolators.

2018-04-19 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62472/ --- (Updated April 19, 2018, 9:22 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 66439: Windows: Made `protobuf::write()` use CRT file descriptor explicitly.

2018-04-19 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66439/#review201562 --- 3rdparty/stout/include/stout/protobuf.hpp Line 83 (original),

Re: Review Request 66568: Dropped combined operations with GROW and SHRINK volumes.

2018-04-19 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66568/#review201561 --- src/master/master.cpp Lines 4511 (patched)

Re: Review Request 66438: Windows: Made `libevent` use CRT file descriptor explicitly.

2018-04-19 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66438/#review201558 --- Ship it! Ship It! - Joseph Wu On April 4, 2018, 12:19 p.m.,

Re: Review Request 66708: Refactored sending a TASK_DROPPED status update.

2018-04-19 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66708/#review201557 --- Patch looks great! Reviews applied: [66704, 66705, 66706, 66707,

Re: Review Request 66433: Windows: Made `net::download()` use CRT file descriptor explicitly.

2018-04-19 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66433/#review201556 --- Ship it! Ship It! - Joseph Wu On April 4, 2018, 12:18 p.m.,

Re: Review Request 66696: Updated documentation for operation feedback.

2018-04-19 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66696/#review201554 --- FAIL: Mesos binaries failed to build. Reviews applied:

Re: Review Request 66532: Added test for authorization actions for `UPDATE_VOLUME`.

2018-04-19 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66532/ --- (Updated April 19, 2018, 1:01 p.m.) Review request for mesos, Chun-Hung Hsiao

Re: Review Request 66532: Added test for authorization actions for `UPDATE_VOLUME`.

2018-04-19 Thread Zhitao Li
> On April 17, 2018, 4:24 p.m., Greg Mann wrote: > > src/tests/authorization_tests.cpp > > Line 1979 (original), 1979 (patched) > > > > > > Could you also add an end-to-end test of authorization for these > >

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-04-19 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66051/ --- (Updated April 19, 2018, 1 p.m.) Review request for mesos, Chun-Hung Hsiao,

Re: Review Request 66531: Added new authorization for `UpdateVolume`.

2018-04-19 Thread Zhitao Li
> On April 18, 2018, 8:04 a.m., Greg Mann wrote: > > src/master/master.cpp > > Line 4432 (original), 4474 (patched) > > > > > > We should also add authorization of these operations in the operator > > API. Fixed

Re: Review Request 66531: Added new authorization for `UpdateVolume`.

2018-04-19 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66531/ --- (Updated April 19, 2018, 12:57 p.m.) Review request for mesos, Chun-Hung Hsiao

Re: Review Request 66708: Refactored sending a TASK_DROPPED status update.

2018-04-19 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66708/#review201549 --- FAIL: Some of the unit tests failed. Please check the relevant

Re: Review Request 66683: Updated address field of new CLI config to accept URLs.

2018-04-19 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66683/#review201547 --- FAIL: Mesos binaries failed to build. Reviews applied:

Re: Review Request 66531: Added new authorization for `UpdateVolume`.

2018-04-19 Thread Zhitao Li
> On April 17, 2018, 4:14 p.m., Greg Mann wrote: > > src/master/master.cpp > > Lines 4924-4925 (original), 4980-4981 (patched) > > > > > > You need to finish implementing authorization here, and below for > >

Re: Review Request 66696: Updated documentation for operation feedback.

2018-04-19 Thread Gaston Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66696/ --- (Updated April 19, 2018, 11:12 a.m.) Review request for mesos and Greg Mann.

Re: Review Request 66708: Refactored sending a TASK_DROPPED status update.

2018-04-19 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66708/ --- (Updated April 19, 2018, 6:09 p.m.) Review request for mesos, Gilbert Song,

Re: Review Request 66531: Added new authorization for `UpdateVolume`.

2018-04-19 Thread Zhitao Li
> On April 17, 2018, 4:14 p.m., Greg Mann wrote: > > src/master/master.cpp > > Lines 4924-4925 (original), 4980-4981 (patched) > > > > > > You need to finish implementing authorization here, and below for > >

Re: Review Request 66708: Refactored sending a TASK_DROPPED status update.

2018-04-19 Thread James Peach
> On April 19, 2018, 1:58 p.m., Qian Zhang wrote: > > src/slave/slave.cpp > > Lines 2543 (patched) > > > > > > Nit: Switch the order of these two parameters since we use `message` > > before `reason` in the code

Re: Review Request 66707: Added a test for launching a task as an unknown user.

2018-04-19 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66707/ --- (Updated April 19, 2018, 5:56 p.m.) Review request for mesos, Gilbert Song,

Re: Review Request 66706: Handled failing to create the executor sandbox.

2018-04-19 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66706/ --- (Updated April 19, 2018, 5:54 p.m.) Review request for mesos, Gilbert Song,

Re: Review Request 66706: Handled failing to create the executor sandbox.

2018-04-19 Thread James Peach
> On April 19, 2018, 1:58 p.m., Qian Zhang wrote: > > src/slave/slave.cpp > > Lines 2994 (patched) > > > > > > Kill this newline. I actually prefer this newline. Because the error handling is a big block I think

Re: Review Request 66705: Propagated executor sandbox creation errors.

2018-04-19 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66705/ --- (Updated April 19, 2018, 5:52 p.m.) Review request for mesos, Gilbert Song,

Re: Review Request 66705: Propagated executor sandbox creation errors.

2018-04-19 Thread James Peach
> On April 19, 2018, 1:56 p.m., Qian Zhang wrote: > > src/slave/paths.cpp > > Line 761 (original), 763 (patched) > > > > > > Do you want to kill this `CHECK_SOME` too? We could return the error for consistency, but

Re: Review Request 66704: Refactored the executor launch path.

2018-04-19 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66704/ --- (Updated April 19, 2018, 5:52 p.m.) Review request for mesos, Gilbert Song,

Re: Review Request 66649: Added pb2gen.sh for generating python protobuf bindings.

2018-04-19 Thread Eric Chung
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66649/#review201538 --- src/python/lib/pb2gen.sh Lines 66 (patched)

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-04-19 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66621/#review201536 --- Patch looks great! Reviews applied: [66621] Passed command:

Re: Review Request 66696: Updated Scheduler HTTP API doc for operation feedback.

2018-04-19 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66696/#review201535 --- Fix it, then Ship it! docs/scheduler-http-api.md Lines 179

Re: Review Request 66694: Updated the 1.6.0 CHANGELOG.

2018-04-19 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66694/#review201534 --- Ship it! Looks good to me - James Peach On April 18, 2018,

Re: Review Request 66220: Added tests for `GROW_VOLUME` and `SHRINK_VOLUME` operations.

2018-04-19 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66220/#review201532 --- FAIL: Failed to apply the dependent review: 66049. Failed

Re: Review Request 66220: Added tests for `GROW_VOLUME` and `SHRINK_VOLUME` operations.

2018-04-19 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66220/ --- (Updated April 19, 2018, 9:39 a.m.) Review request for mesos, Chun-Hung Hsiao,

Re: Review Request 66220: Added tests for `GROW_VOLUME` and `SHRINK_VOLUME` operations.

2018-04-19 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66220/ --- (Updated April 19, 2018, 9:30 a.m.) Review request for mesos, Chun-Hung Hsiao,

Re: Review Request 66049: Added offer operation to grow and shrink persistent volumes.

2018-04-19 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66049/ --- (Updated April 19, 2018, 9:28 a.m.) Review request for mesos, Chun-Hung Hsiao,

Re: Review Request 66220: Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operations.

2018-04-19 Thread Zhitao Li
> On April 17, 2018, 3:52 p.m., Gaston Kleiman wrote: > > src/tests/persistent_volume_tests.cpp > > Lines 474-475 (patched) > > > > > > I see that Benjamin recently added this line and the corresponding > >

Re: Review Request 66220: Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operations.

2018-04-19 Thread Zhitao Li
> On April 15, 2018, 5:39 p.m., Chun-Hung Hsiao wrote: > > src/tests/persistent_volume_tests.cpp > > Lines 455-459 (patched) > > > > > > Or alternatively, we can do the following: > > ``` > > class

Re: Review Request 66220: Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operations.

2018-04-19 Thread Zhitao Li
> On April 15, 2018, 5:39 p.m., Chun-Hung Hsiao wrote: > > src/tests/persistent_volume_tests.cpp > > Lines 453 (patched) > > > > > > If I'm not mistaken, this test does the following steps: > > > > 1.

Re: Review Request 66220: Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operations.

2018-04-19 Thread Zhitao Li
> On April 13, 2018, 9:43 a.m., Greg Mann wrote: > > src/tests/persistent_volume_tests.cpp > > Lines 455-459 (patched) > > > > > > Is this enforced somewhere in validation code? Can we check for > > expected

Re: Review Request 66569: Added a test to verify that grow and shrink cannot be combined.

2018-04-19 Thread Zhitao Li
> On April 16, 2018, 3:05 p.m., Chun-Hung Hsiao wrote: > > Thanks for the review. I'll combine this patch into r/66220 and address your comment in that process. > On April 16, 2018, 3:05 p.m., Chun-Hung Hsiao wrote: > > src/tests/persistent_volume_tests.cpp > > Lines 691-695 (patched) > >

Re: Review Request 66711: Made 3rdparty jemalloc only build when enabled.

2018-04-19 Thread Andrew Schwartzmeyer
> On April 19, 2018, 5:43 a.m., Benno Evers wrote: > > I'm a bit surprised by the behaviour you describe, i.e. cmake downloading > > and configuring third-party dependencies that are not going to be used. Am > > I correct in thinking that for a proper fix, we should wrap *every* > >

Re: Review Request 66708: Refactored sending a TASK_DROPPED status update.

2018-04-19 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66708/#review201517 --- src/slave/slave.cpp Lines 2543 (patched)

Re: Review Request 66707: Added a test for launching a task as an unknown user.

2018-04-19 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66707/#review201516 --- Fix it, then Ship it! src/tests/slave_tests.cpp Lines 1271

Re: Review Request 66706: Handled failing to create the executor sandbox.

2018-04-19 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66706/#review201512 --- Fix it, then Ship it! src/slave/slave.cpp Lines 2994

Re: Review Request 66705: Propagated executor sandbox creation errors.

2018-04-19 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66705/#review201511 --- src/slave/paths.cpp Line 761 (original), 763 (patched)

Re: Review Request 66704: Refactored the executor launch path.

2018-04-19 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66704/#review201510 --- Fix it, then Ship it! src/slave/slave.cpp Line 2912

Re: Review Request 66546: Prevent resubscription of resource providers with unknown IDs.

2018-04-19 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66546/#review201519 --- FAIL: Some of the unit tests failed. Please check the relevant

Re: Review Request 66711: Made 3rdparty jemalloc only build when enabled.

2018-04-19 Thread Benno Evers
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66711/#review201515 --- Ship it! I'm a bit surprised by the behaviour you describe,

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-04-19 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66621/#review201514 --- 3rdparty/libprocess/include/process/jwt.hpp Lines 19 (patched)

Re: Review Request 66310: Passed on registrar when constructing resource provider manager.

2018-04-19 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66310/ --- (Updated April 19, 2018, 2:28 p.m.) Review request for mesos, Chun-Hung Hsiao,

Re: Review Request 66526: Renamed resource provider `AgentRegistrar` to `GenericRegistrar`.

2018-04-19 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66526/ --- (Updated April 19, 2018, 2:29 p.m.) Review request for mesos, Chun-Hung Hsiao,

Re: Review Request 66311: Implement recovery of resource provider manager.

2018-04-19 Thread Benjamin Bannier
> On April 19, 2018, 8 a.m., Chun-Hung Hsiao wrote: > > src/resource_provider/manager.cpp > > Lines 237 (patched) > > > > > > `::recover` This is not used anywhere else in this file, let's just keep this instance

Re: Review Request 66311: Implement recovery of resource provider manager.

2018-04-19 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66311/ --- (Updated April 19, 2018, 2:29 p.m.) Review request for mesos, Chun-Hung Hsiao,

Re: Review Request 66310: Passed on registrar when constructing resource provider manager.

2018-04-19 Thread Benjamin Bannier
> On April 19, 2018, 12:25 a.m., Chun-Hung Hsiao wrote: > > src/resource_provider/manager.cpp > > Line 160 (original), 161 (patched) > > > > > > `Owned&& _registrar`?

Re: Review Request 66711: Made 3rdparty jemalloc only build when enabled.

2018-04-19 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66711/#review201513 --- Patch looks great! Reviews applied: [66711] Passed command:

Re: Review Request 66309: Externalized creation of resource provider manager backing storage.

2018-04-19 Thread Benjamin Bannier
> On April 18, 2018, 11:31 p.m., Chun-Hung Hsiao wrote: > > src/resource_provider/registrar.hpp > > Line 69 (original), 71 (patched) > > > > > > Does it make sense to use `process::Owned&& storage`? I don't think

Re: Review Request 66309: Externalized creation of resource provider manager backing storage.

2018-04-19 Thread Benjamin Bannier
> On April 19, 2018, 12:19 a.m., Chun-Hung Hsiao wrote: > > src/resource_provider/registrar.hpp > > Line 113 (original), 114 (patched) > > > > > > `process::Owned&& storage`? See above. > On April 19, 2018, 12:19

Re: Review Request 66309: Externalized creation of resource provider manager backing storage.

2018-04-19 Thread Benjamin Bannier
> On April 18, 2018, 11:51 p.m., Chun-Hung Hsiao wrote: > > src/resource_provider/registrar.hpp > > Lines 67-73 (original), 69-75 (patched) > > > > > > Not yours, but if each `create()` corresponds to a different

Re: Review Request 66308: Delayed construction of the agent's resource provider manager.

2018-04-19 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66308/ --- (Updated April 19, 2018, 11:19 a.m.) Review request for mesos, Chun-Hung

Re: Review Request 66308: Delayed construction of the agent's resource provider manager.

2018-04-19 Thread Benjamin Bannier
> On April 18, 2018, 8:58 p.m., Chun-Hung Hsiao wrote: > > src/slave/slave.cpp > > Lines 839 (patched) > > > > > > One line apart. Actually, let's move this back to it's original place > > so all `/api/v1/*`

Re: Review Request 66309: Externalized creation of resource provider manager backing storage.

2018-04-19 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66309/ --- (Updated April 19, 2018, 11:19 a.m.) Review request for mesos, Jie Yu and Jan

Re: Review Request 66708: Refactored sending a TASK_DROPPED status update.

2018-04-19 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66708/#review201499 --- Patch looks great! Reviews applied: [66704, 66705, 66706, 66707,

Re: Review Request 66311: Implement recovery of resource provider manager.

2018-04-19 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66311/#review201488 --- src/resource_provider/manager.cpp Lines 89 (patched)