Re: Review Request 62762: Added `LocalResourceProvider::principal()` for authentication.

2017-11-22 Thread Chun-Hung Hsiao
> On Nov. 23, 2017, 5:27 a.m., Jie Yu wrote: > > src/resource_provider/local.cpp > > Lines 45 (patched) > > > > > > We try to avoid non-POD (plain old data) global variables because it'll > > cause issues during

Re: Review Request 64014: Added default reservations in `ResourceProviderInfo`.

2017-11-22 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64014/#review191781 --- include/mesos/mesos.proto Line 1035 (original)

Re: Review Request 63904: Updated the top-level `CSIPluginInfo` proto.

2017-11-22 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63904/#review191780 --- src/common/type_utils.cpp Lines 102 (patched)

Re: Review Request 62762: Added `LocalResourceProvider::principal()` for authentication.

2017-11-22 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62762/#review191779 --- src/resource_provider/local.cpp Lines 35-40 (patched)

Review Request 64044: Killed unused CSI plugin containers for storage local resource provider.

2017-11-22 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64044/ --- Review request for mesos, Benjamin Bannier, Jie Yu, Joseph Wu, and Jan Schlicht.

Re: Review Request 63021: Added `getService()` function to launch CSI plugins.

2017-11-22 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63021/ --- (Updated Nov. 23, 2017, 2:21 a.m.) Review request for mesos, James DeFelice,

Re: Review Request 64014: Added default reservations in `ResourceProviderInfo`.

2017-11-22 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64014/ --- (Updated Nov. 23, 2017, 1:38 a.m.) Review request for mesos, Benjamin Bannier,

Re: Review Request 64012: Added new --configuration_compatibility slave flag and implementation.

2017-11-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64012/#review191769 --- Partial review. src/Makefile.am Lines 1015 (patched)

Re: Review Request 64011: Updated master behaviour to update agent state on reregistration.

2017-11-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64011/#review191767 --- src/master/master.hpp Lines 1105 (patched)

Re: Review Request 64014: Added default reservations in `ResourceProviderInfo`.

2017-11-22 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64014/ --- (Updated Nov. 23, 2017, 12:47 a.m.) Review request for mesos, Benjamin

Re: Review Request 63798: Added resource provider support for all offer operations.

2017-11-22 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63798/#review191768 --- Ship it! Ship It! - Jie Yu On Nov. 22, 2017, 12:15 p.m.,

Re: Review Request 63751: Triggered 'ApplyOfferOperationMessage' for agent local resources.

2017-11-22 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63751/#review191766 --- Fix it, then Ship it! src/slave/slave.cpp Lines 3763-3764

Review Request 64012: Added new --configuration_compatibility slave flag and implementation.

2017-11-22 Thread Benno Evers
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64012/ --- Review request for mesos and Vinod Kone. Repository: mesos Description

Review Request 64011: Updated master behaviour to update agent state on reregistration.

2017-11-22 Thread Benno Evers
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64011/ --- Review request for mesos and Vinod Kone. Repository: mesos Description

Re: Review Request 60471: Added tests for pruneImages for containerizer and provisioner.

2017-11-22 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60471/ --- (Updated Nov. 22, 2017, 10:50 p.m.) Review request for mesos, Gilbert Song and

Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

2017-11-22 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56722/ --- (Updated Nov. 22, 2017, 10:50 p.m.) Review request for mesos, Gilbert Song,

Re: Review Request 62853: Added test for `PRUNE_IMAGES` operator API call.

2017-11-22 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62853/ --- (Updated Nov. 22, 2017, 10:50 p.m.) Review request for mesos, Gilbert Song and

Re: Review Request 63989: Added the OFFER_OPERATION_DROPPED state.

2017-11-22 Thread Gaston Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63989/#review191758 --- Ship it! Ship It! - Gaston Kleiman On Nov. 21, 2017, 9:21

Re: Review Request 63988: Fixed indentation in a test header.

2017-11-22 Thread Gaston Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63988/#review191757 --- Ship it! Ship It! - Gaston Kleiman On Nov. 21, 2017, 9:10

Re: Review Request 64028: Removed duplicate resources conversions.

2017-11-22 Thread Benjamin Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64028/#review191755 --- Should we be implementing a += overload against

Re: Review Request 63959: Optimized resources logging in master.

2017-11-22 Thread Benjamin Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63959/#review191754 --- Fix it, then Ship it! Thanks! Left a comment below but will

Re: Review Request 64033: Terminated executors if kill task received before launch task.

2017-11-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64033/#review191748 --- can you write tests please? src/docker/executor.cpp Lines 390

Re: Review Request 64032: Promoted log level to warning for disconnected events in exec.cpp.

2017-11-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64032/#review191747 --- Ship it! Ship It! - Vinod Kone On Nov. 22, 2017, 4:15 p.m.,

Re: Review Request 64033: Terminated executors if kill task received before launch task.

2017-11-22 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64033/ --- (Updated Nov. 22, 2017, 5:05 p.m.) Review request for mesos, Andrei Budnik,

Review Request 64032: Promoted log level to warning for disconnected events in exec.cpp.

2017-11-22 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64032/ --- Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone. Bugs:

Review Request 64033: Terminated executors if kill task received before launch task.

2017-11-22 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64033/ --- Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone. Bugs:

Re: Review Request 63953: Added logging based on container class.

2017-11-22 Thread Armand Grillet
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63953/ --- (Updated Nov. 22, 2017, 4 p.m.) Review request for mesos and Alexander

Re: Review Request 63795: Made `mesos-tcp-connect` support IPv6.

2017-11-22 Thread Qian Zhang
> On Nov. 22, 2017, 8:16 p.m., Avinash sridharan wrote: > > src/checks/tcp_connect.cpp > > Lines 90 (patched) > > > > > > Should have bought this up earlier. I think this code will be a lot > > more simpler if you

Re: Review Request 63795: Made `mesos-tcp-connect` support IPv6.

2017-11-22 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63795/ --- (Updated Nov. 22, 2017, 11:14 p.m.) Review request for mesos, Alexander

Re: Review Request 63636: Added placeholder implementation.

2017-11-22 Thread Dmitry Zhuk
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63636/ --- (Updated Nov. 22, 2017, 2:13 p.m.) Review request for mesos, Benjamin Mahler

Re: Review Request 63633: Added ability to protect nested bind expressions from composition.

2017-11-22 Thread Dmitry Zhuk
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63633/ --- (Updated Nov. 22, 2017, 2:12 p.m.) Review request for mesos, Benjamin Mahler

Re: Review Request 63630: Added support for callable once functors.

2017-11-22 Thread Dmitry Zhuk
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63630/ --- (Updated Nov. 22, 2017, 2:10 p.m.) Review request for mesos, Benjamin Mahler

Re: Review Request 63629: Added partial function application implementation.

2017-11-22 Thread Dmitry Zhuk
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63629/ --- (Updated Nov. 22, 2017, 2:09 p.m.) Review request for mesos, Benjamin Mahler

Re: Review Request 63628: Implemented index_sequence and related functionality.

2017-11-22 Thread Dmitry Zhuk
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63628/ --- (Updated Nov. 22, 2017, 2:07 p.m.) Review request for mesos, Benjamin Mahler

Review Request 64028: Removed duplicate resources conversions.

2017-11-22 Thread Dmitry Zhuk
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64028/ --- Review request for mesos, Benjamin Mahler and Michael Park. Repository: mesos

Re: Review Request 63959: Optimized resources logging in master.

2017-11-22 Thread Dmitry Zhuk
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63959/ --- (Updated Nov. 22, 2017, 1:24 p.m.) Review request for mesos, Benjamin Mahler

Re: Review Request 63910: Added 3 tests for TCP/HTTP(S) health check support for Docker container.

2017-11-22 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63910/#review191724 --- Ship it! Ship It! - Avinash sridharan On Nov. 19, 2017,

Re: Review Request 63910: Added 3 tests for TCP/HTTP(S) health check support for Docker container.

2017-11-22 Thread Avinash sridharan
> On Nov. 17, 2017, 7:25 p.m., Avinash sridharan wrote: > > src/tests/health_check_tests.cpp > > Lines 2450 (patched) > > > > > > We should try and see if we can publish this image to > >

Re: Review Request 63795: Made `mesos-tcp-connect` support IPv6.

2017-11-22 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63795/#review191722 --- src/checks/tcp_connect.cpp Lines 90 (patched)

Re: Review Request 63798: Added resource provider support for all offer operations.

2017-11-22 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63798/ --- (Updated Nov. 22, 2017, 1:15 p.m.) Review request for mesos, Benjamin Bannier

Re: Review Request 63751: Triggered 'ApplyOfferOperationMessage' for agent local resources.

2017-11-22 Thread Jan Schlicht
> On Nov. 16, 2017, 12:37 a.m., Jie Yu wrote: > > src/slave/slave.cpp > > Lines 3743 (patched) > > > > > > For old operations, I think we should apply the conversion (i.e., > > change `totalResources`) in

Re: Review Request 63751: Triggered 'ApplyOfferOperationMessage' for agent local resources.

2017-11-22 Thread Jan Schlicht
> On Nov. 15, 2017, 10:53 p.m., Jie Yu wrote: > > src/slave/slave.cpp > > Lines 3751-3757 (patched) > > > > > > Are we leaking `OfferOperation` in the agent memory if we don't call > > `updateOfferOperation` and

Re: Review Request 63751: Triggered 'ApplyOfferOperationMessage' for agent local resources.

2017-11-22 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63751/ --- (Updated Nov. 22, 2017, 1:11 p.m.) Review request for mesos, Benjamin Bannier

Re: Review Request 63628: Implemented index_sequence and related functionality.

2017-11-22 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63628/#review191717 --- Fix it, then Ship it! Looks good!

Re: Review Request 62637: Added an object approver to authorize requests from resource providers.

2017-11-22 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62637/#review191714 --- Please add unit tests. - Alexander Rojas On Nov. 21, 2017,

Re: Review Request 62636: Generated authentication tokens for local resource providers.

2017-11-22 Thread Chun-Hung Hsiao
> On Nov. 22, 2017, 8:38 a.m., Alexander Rojas wrote: > > src/resource_provider/daemon.cpp > > Lines 253 (patched) > > > > > > This is a personal preference, but I don't see the reason to add more > > strings to a

Re: Review Request 62636: Generated authentication tokens for local resource providers.

2017-11-22 Thread Chun-Hung Hsiao
> On Nov. 22, 2017, 8:38 a.m., Alexander Rojas wrote: > > src/resource_provider/daemon.cpp > > Lines 78 (patched) > > > > > > This line is not needed. Mostly for consistence reason, you don't see > > this pattern of

Re: Review Request 62636: Generated authentication tokens for local resource providers.

2017-11-22 Thread Chun-Hung Hsiao
> On Nov. 22, 2017, 8:40 a.m., Alexander Rojas wrote: > > There are no tests anywhere in the chain. I disapprove of code written > > without unit tests. Please address that in the whole chain. Will do. Need more time to add more tests :( - Chun-Hung

Re: Review Request 63021: Added `getService()` function to launch CSI plugins.

2017-11-22 Thread Chun-Hung Hsiao
> On Oct. 20, 2017, 12:26 a.m., Jie Yu wrote: > > src/resource_provider/storage/provider.cpp > > Lines 236 (patched) > > > > > > YOu should follow `src/launcher/default_executor.cpp` to see how to do > > reliable

Re: Review Request 63021: Added `getService()` function to launch CSI plugins.

2017-11-22 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63021/ --- (Updated Nov. 22, 2017, 8:47 a.m.) Review request for mesos, James DeFelice,

Re: Review Request 63823: Initialization and registration for storage local resource provider.

2017-11-22 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63823/ --- (Updated Nov. 22, 2017, 8:47 a.m.) Review request for mesos, Jie Yu, Joseph

Re: Review Request 62636: Generated authentication tokens for local resource providers.

2017-11-22 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62636/#review191708 --- There are no tests anywhere in the chain. I disapprove of code

Re: Review Request 62636: Generated authentication tokens for local resource providers.

2017-11-22 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62636/#review191707 --- src/resource_provider/daemon.cpp Lines 78 (patched)