Re: Review Request 66293: Tested default executor support of `max_completion_time`.

2018-04-12 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66293/#review201085 --- Patch looks great! Reviews applied: [66481, 66258, 66591, 66259,

Re: Review Request 66230: Added test for adding/removing framework roles.

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

Re: Review Request 66229: Implemented UPDATE_FRAMEWORK call.

2018-04-12 Thread Kapil Arya
> On April 12, 2018, 8:19 p.m., Till Toenshoff wrote: > > src/master/validation.cpp > > Lines 532 (patched) > > > > > > s/is/are/ > > > > How do we make sure the role is "valid" - isn't that implicit when

Re: Review Request 66229: Implemented UPDATE_FRAMEWORK call.

2018-04-12 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66229/ --- (Updated April 12, 2018, 9:38 p.m.) Review request for mesos, Benjamin Mahler

Re: Review Request 66230: Added test for adding/removing framework roles.

2018-04-12 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66230/ --- (Updated April 12, 2018, 9:38 p.m.) Review request for mesos, Benjamin Mahler

Re: Review Request 66577: Enabled CSI proto compilation by default.

2018-04-12 Thread Chun-Hung Hsiao
> On April 12, 2018, 4:44 p.m., Andrew Schwartzmeyer wrote: > > src/CMakeLists.txt > > Lines 32-35 (patched) > > > > > > Nit: can this be moved down so it goes "no options", "grpc option", > > "java option",

Re: Review Request 66418: Handled failed publish and unpublish CSI calls properly.

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

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

2018-04-12 Thread Chun-Hung Hsiao
> On April 13, 2018, 12:27 a.m., Greg Mann wrote: > > include/mesos/mesos.proto > > Lines 1975 (patched) > > > > > > Hmm I wonder if we should just use a `double` here? Does the > > `Value.Scalar` type provide

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

2018-04-12 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66049/#review201074 --- include/mesos/mesos.proto Lines 2008 (patched)

Re: Review Request 53267: Added a gauge to track active subscribers.

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

Re: Review Request 66050: Implemented grow and shrink of persistent volumes.

2018-04-12 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66050/#review201055 --- src/master/master.cpp Lines 4949 (patched)

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

2018-04-12 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66049/#review201067 --- include/mesos/mesos.proto Lines 1975 (patched)

Re: Review Request 66229: Implemented UPDATE_FRAMEWORK call.

2018-04-12 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66229/#review201053 --- Looks great Kapil - just some minor comments on logging and

Re: Review Request 65974: Added comments and made some renaming in SLRP.

2018-04-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65974/#review201063 --- Ship it! Ship It! - Jie Yu On April 12, 2018, 3:33 a.m.,

Re: Review Request 66576: Added `STAGE_UNSTAGE_VOLUME` capability to the test CSI plugin.

2018-04-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66576/#review201062 --- Ship it! Ship It! - Jie Yu On April 12, 2018, 1:36 a.m.,

Re: Review Request 66575: Supported `STAGE_UNSTAGE_VOLUME` CSI node capability in SLRP.

2018-04-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66575/#review201059 --- Fix it, then Ship it!

Re: Review Request 53267: Added a gauge to track active subscribers.

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

Re: Review Request 66574: Updated filesystem layout for staging and mounting CSI volumes.

2018-04-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66574/#review201057 --- Ship it! Ship It! - Jie Yu On April 12, 2018, 1:33 a.m.,

Re: Review Request 66418: Handled failed publish and unpublish CSI calls properly.

2018-04-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66418/#review201054 --- Fix it, then Ship it!

Re: Review Request 66293: Tested default executor support of `max_completion_time`.

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

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

2018-04-12 Thread Zhitao Li
> On March 29, 2018, 5:07 a.m., Benjamin Bannier wrote: > > include/mesos/mesos.proto > > Lines 1920-1921 (patched) > > > > > > These enum values are not handled in a number of switch statements in > > the code so

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

2018-04-12 Thread Chun-Hung Hsiao
> On March 29, 2018, 12:07 p.m., Benjamin Bannier wrote: > > include/mesos/mesos.proto > > Lines 1920-1921 (patched) > > > > > > These enum values are not handled in a number of switch statements in > > the code

Re: Review Request 53267: Added a gauge to track active subscribers.

2018-04-12 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53267/ --- (Updated April 12, 2018, 2:32 p.m.) Review request for mesos, Anand Mazumdar,

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

2018-04-12 Thread Greg Mann
> On March 29, 2018, 12:07 p.m., Benjamin Bannier wrote: > > include/mesos/mesos.proto > > Lines 1920-1921 (patched) > > > > > > These enum values are not handled in a number of switch statements in > > the code

Review Request 66591: Added a validation that `max_completion_time` must be non-negative.

2018-04-12 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66591/ --- Review request for mesos, Jason Lai and James Peach. Bugs: MESOS-8725

Re: Review Request 66218: Ensured that agent does not delete volume upon grow or shrink.

2018-04-12 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66218/#review201045 --- Ship it! Ship It! - Greg Mann On March 28, 2018, 6:25 p.m.,

Re: Review Request 65666: Added a unit test for SLRP operation state metrics.

2018-04-12 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65666/#review201042 --- Bad patch! Reviews applied: [65666, 65665, 65640, 65976, 65975,

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

2018-04-12 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66493/#review201039 --- Fix it, then Ship it! cmake/CompilationConfigure.cmake Lines

Re: Review Request 66410: Adapted storage local resource provider to use CSI v0.2.

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

Re: Review Request 66408: Updated CSI helpers for v0.2.

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

Re: Review Request 65665: Added operation state metrics in SLRP.

2018-04-12 Thread Chun-Hung Hsiao
> On March 21, 2018, 1:12 p.m., Jan Schlicht wrote: > > src/resource_provider/storage/provider.cpp > > Lines 2713 (patched) > > > > > > `statusUpdateManager.update` below might still fail to drop the > > message.

Re: Review Request 65666: Added a unit test for SLRP operation state metrics.

2018-04-12 Thread Chun-Hung Hsiao
> On March 26, 2018, 2:27 p.m., Jan Schlicht wrote: > > This is missing a test for `operations_pending`. You could wait with a > > `DROP_PROTOBUF(UpdateOperationStatusMessage)` after applying an operation, > > then check that `operations_pending` is "1", then (manually) send the > >

Re: Review Request 65665: Added operation state metrics in SLRP.

2018-04-12 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65665/#review200979 --- Haven't managed to make a full review of the chain, yet, one

Re: Review Request 66408: Updated CSI helpers for v0.2.

2018-04-12 Thread Chun-Hung Hsiao
> On April 12, 2018, 5:47 p.m., Jie Yu wrote: > > src/csi/state.proto > > Line 45 (original), 45 (patched) > > > > > > Please s/VolumeInfo/Volume/ here > > > > ALso, a git grep shows some other references to

Re: Review Request 66411: Made the test CSI plugin compatible to CSI v0.2.

2018-04-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66411/#review201026 --- Ship it! Ship It! - Jie Yu On April 11, 2018, 2:36 a.m.,

Re: Review Request 65666: Added a unit test for SLRP operation state metrics.

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

Re: Review Request 66408: Updated CSI helpers for v0.2.

2018-04-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66408/#review201024 --- src/csi/state.proto Line 45 (original), 45 (patched)

Re: Review Request 65666: Added a unit test for SLRP operation state metrics.

2018-04-12 Thread Chun-Hung Hsiao
> On March 26, 2018, 2:27 p.m., Jan Schlicht wrote: > > src/tests/storage_local_resource_provider_tests.cpp > > Lines 3127 (patched) > > > > > > We should wait for an `UpdateSlaveMessage` here. Otherwise we can't

Re: Review Request 66410: Adapted storage local resource provider to use CSI v0.2.

2018-04-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66410/#review201018 --- Fix it, then Ship it!

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

2018-04-12 Thread Andrew Schwartzmeyer
> On April 12, 2018, 10:06 a.m., Andrew Schwartzmeyer wrote: > > src/examples/CMakeLists.txt > > Lines 55-59 (patched) > > > > > > What about non-Linux but not Windows platforms, e.g. MacOS and FreeBSD? Saw in prior

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

2018-04-12 Thread Andrew Schwartzmeyer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66163/#review201017 --- cmake/CompilationConfigure.cmake Lines 387-390 (patched)

Re: Review Request 66462: Added new operation states to be used for status reconciliation.

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

Re: Review Request 61118: Libprocess: Building gRPC support with CMake.

2018-04-12 Thread Andrew Schwartzmeyer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61118/#review201014 --- 3rdparty/libprocess/src/tests/CMakeLists.txt Lines 91 (patched)

Re: Review Request 66409: Used `csi::v0::VolumeCapability` in disk profile adaptors.

2018-04-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66409/#review201016 --- Ship it! Ship It! - Jie Yu On April 3, 2018, 7:18 p.m.,

Re: Review Request 66408: Updated CSI helpers for v0.2.

2018-04-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66408/#review201015 --- Ship it! Ship It! - Jie Yu On April 4, 2018, 3:59 a.m.,

Re: Review Request 66407: Updated CSI client to support v0.2.

2018-04-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66407/#review201013 --- Ship it! Ship It! - Jie Yu On April 3, 2018, 7:12 p.m.,

Re: Review Request 66577: Enabled CSI proto compilation by default.

2018-04-12 Thread Andrew Schwartzmeyer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66577/#review201012 --- Note: I did not review the Autotools part at all ;)

Re: Review Request 61096: Building gRPC with CMake.

2018-04-12 Thread Andrew Schwartzmeyer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61096/#review201007 --- I want to test on Windows and see if we can get it building there

Re: Review Request 66541: Added default executor test for agent recovery without metadata.

2018-04-12 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66541/#review201009 --- Patch looks great! Reviews applied: [66538, 66539, 66540, 66541]

Re: Review Request 66398: Bumped the CSI spec bundle to 0.2.0.

2018-04-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66398/#review201008 --- Ship it! Ship It! - Jie Yu On April 3, 2018, 6:45 p.m.,

Re: Review Request 66462: Added new operation states to be used for status reconciliation.

2018-04-12 Thread Gaston Kleiman
> On April 11, 2018, 7:06 p.m., Chun-Hung Hsiao wrote: > > Are all of these necessary for now? Yeah, they are all generated/sent by the {{ReconcileOperations}} handler. > On April 11, 2018, 7:06 p.m., Chun-Hung Hsiao wrote: > > src/slave/slave.cpp > > Lines 8034-8038 (patched) > >

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

2018-04-12 Thread Andrew Schwartzmeyer
> On April 9, 2018, 6:13 a.m., Benjamin Bannier wrote: > > cmake/CompilationConfigure.cmake > > Lines 315 (patched) > > > > > > This conditional seems not needed as cmake will automatically detect if > >

Re: Review Request 64970: Use tox for linting and testing code living uder src/python.

2018-04-12 Thread Eric Chung
> On April 6, 2018, 9:46 a.m., Armand Grillet wrote: > > src/python/lib/tox.ini > > Lines 19 (patched) > > > > > > As this is here, the line `pylint==1.6.4` in > > support/pip-requirements.txt should be removed.

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

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

Re: Review Request 64211: Added options to build the Python CLI and run unit tests.

2018-04-12 Thread Kevin Klues
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64211/#review201000 --- Ship it! Ship It! - Kevin Klues On March 24, 2018, 11:39

Re: Review Request 65705: Fixed CLI bootstrap script to work with long workspace paths.

2018-04-12 Thread Armand Grillet
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65705/ --- (Updated April 12, 2018, 12:39 p.m.) Review request for mesos, Benjamin

Re: Review Request 65585: Improved documentation regarding the new CLI setup.

2018-04-12 Thread Kevin Klues
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65585/#review200999 --- src/python/cli_new/README.md Lines 40-55 (patched)

Re: Review Request 65585: Improved documentation regarding the new CLI setup.

2018-04-12 Thread Armand Grillet
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65585/ --- (Updated April 12, 2018, 12:36 p.m.) Review request for mesos, Benjamin

Re: Review Request 65585: Improved documentation regarding the new CLI setup.

2018-04-12 Thread Kevin Klues
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65585/#review200997 --- src/python/cli_new/bootstrap Lines 120 (patched)

Re: Review Request 65705: Fixed CLI bootstrap script to work with long workspace paths.

2018-04-12 Thread Kevin Klues
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65705/#review200996 --- Ship it! Ship It! - Kevin Klues On March 24, 2018, 11:39

Re: Review Request 64211: Added options to build the Python CLI and run unit tests.

2018-04-12 Thread Kevin Klues
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64211/#review200995 --- Ship it! Ship It! - Kevin Klues On March 24, 2018, 11:39

Re: Review Request 65705: Fixed CLI bootstrap script to work with long workspace paths.

2018-04-12 Thread Kevin Klues
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65705/#review200994 --- src/python/cli_new/bootstrap Line 27 (original), 27 (patched)

Re: Review Request 65585: Improved documentation regarding the new CLI setup.

2018-04-12 Thread Kevin Klues
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65585/#review200993 --- src/python/cli_new/README.md Lines 40 (patched)

Re: Review Request 66578: Windows: Ported more unit tests from `os_tests.cpp`.

2018-04-12 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66578/#review200991 --- Patch looks great! Reviews applied: [66420, 66421, 66422, 66423,

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

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

Re: Review Request 66541: Added default executor test for agent recovery without metadata.

2018-04-12 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66541/#review200982 --- Fix it, then Ship it! src/tests/default_executor_tests.cpp

Re: Review Request 66538: Added unit test slave recovery for default executor tests.

2018-04-12 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66538/#review200987 --- Fix it, then Ship it! src/tests/default_executor_tests.cpp

Re: Review Request 66545: Added admitted resource providers to the manager's registry.

2018-04-12 Thread Benjamin Bannier
> On April 11, 2018, 2:47 p.m., Jan Schlicht wrote: > > src/resource_provider/manager.cpp > > Lines 668 (patched) > > > > > > Shouldn't this be `false` here? If there's no registrar to admit a > > resource provider

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

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

Re: Review Request 66545: Added admitted resource providers to the manager's registry.

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

Re: Review Request 66554: Updated Web UI controllers to have one file per controller.

2018-04-12 Thread Armand Grillet
> On April 11, 2018, 10:42 p.m., Benjamin Mahler wrote: > > src/webui/app/agents/agent-browse-controller.js > > Lines 35 (patched) > > > > > > I noticed these .js files just assume the right js files are already > >

Re: Review Request 66545: Added admitted resource providers to the manager's registry.

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

Re: Review Request 66540: Added unit test for recovering nested container without slave state.

2018-04-12 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66540/#review200977 --- Fix it, then Ship it!

Re: Review Request 66539: Fixed the agent recovery crash if metadata is missing.

2018-04-12 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66539/#review200976 --- Ship it! Ship It! - Qian Zhang On April 11, 2018, 7:41

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

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