Re: Review Request 37827: Added a test for converting JSON arrays to repeated protobufs.

2015-09-15 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37827/ --- (Updated Sept. 15, 2015, 7:58 a.m.) Review request for mesos, haosdent huang, J

Re: Review Request 37827: Added a test for converting JSON arrays to repeated protobufs.

2015-09-15 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37827/#review98996 --- Ship it! Ship It! - Michael Park On Sept. 15, 2015, 7:58 a.m., A

Re: Review Request 38031: [3/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-15 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38031/#review98998 --- 3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp (line 128

Re: Review Request 38201: [MESOS-1187] precision errors with allocation calculations

2015-09-15 Thread Klaus Ma
> On Sept. 9, 2015, 3:46 a.m., Guangya Liu wrote: > > I think that the V1 related should also be updated. Agree with that; I'll address that if no comments on the algorithm. And I've logged a JIRA to address duplicated codes. - Klaus -

Re: Review Request 38102: MESOS-3046 (Stout's UUID re-seeds a new random generator during each call to UUID::random)

2015-09-15 Thread Klaus Ma
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38102/ --- (Updated Sept. 15, 2015, 9:39 a.m.) Review request for mesos and Ben Mahler.

Re: Review Request 38102: MESOS-3046 (Stout's UUID re-seeds a new random generator during each call to UUID::random)

2015-09-15 Thread Klaus Ma
> On Sept. 14, 2015, 11:25 p.m., Ben Mahler wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp, line 31 > > > > > > Please use THREAD_LOCAL from stout/thread_local.hpp and store a heap > > object as

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-15 Thread Klaus Ma
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/ --- (Updated Sept. 15, 2015, 9:43 a.m.) Review request for mesos, Alexander Ruklets

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-15 Thread Klaus Ma
> On Sept. 14, 2015, 5:39 p.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp, line 112 > > > > > > Let's `reserve()` first. Addressed, thanks :). - Klaus -

Review Request 38399: Add ACLs for the maintenance HTTP endpoints

2015-09-15 Thread Zhiwei Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38399/ --- Review request for mesos. Bugs: mesos- https://issues.apache.org/jira/b

Re: Review Request 38375: Fixed process::collect and process::await to do discard propagation.

2015-09-15 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38375/#review99008 --- Bad patch! Reviews applied: [38375] Failed command: ./support/appl

Re: Review Request 38347: Add HELP Message for Reserve/Unreserve endpoint

2015-09-15 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38347/#review99010 --- src/master/http.cpp (line 670)

Re: Review Request 38347: Add HELP Message for Reserve/Unreserve endpoint

2015-09-15 Thread haosdent huang
> On Sept. 15, 2015, 11:23 a.m., haosdent huang wrote: > > src/master/http.cpp, line 670 > > > > > > Could you also document the parameters about the reserve/unreserve > > endpoints? For example: /logging endpoint d

Re: Review Request 38347: Add HELP Message for Reserve/Unreserve endpoint

2015-09-15 Thread Guangya Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38347/ --- (Updated Sept. 15, 2015, 12:56 p.m.) Review request for mesos and Michael Park.

Re: Review Request 38347: Add HELP Message for Reserve/Unreserve endpoint

2015-09-15 Thread Guangya Liu
> On Sept. 15, 2015, 11:23 a.m., haosdent huang wrote: > > src/master/http.cpp, line 670 > > > > > > Could you also document the parameters about the reserve/unreserve > > endpoints? For example: /logging endpoint d

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-15 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/#review99020 --- Please rebase your patch, latest commits in master lots of JSON rela

Re: Review Request 38347: Add HELP Message for Reserve/Unreserve endpoint

2015-09-15 Thread Guangya Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38347/ --- (Updated Sept. 15, 2015, 1:04 p.m.) Review request for mesos and Michael Park.

Re: Review Request 38253: Add containerId to ResourceUsage to enable QoS controller to target a container

2015-09-15 Thread Klaus Ma
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38253/ --- (Updated Sept. 15, 2015, 1:08 p.m.) Review request for mesos and Niklas Nielsen

Re: Review Request 38253: Add containerId to ResourceUsage to enable QoS controller to target a container

2015-09-15 Thread Klaus Ma
> On Sept. 10, 2015, 12:58 p.m., Guangya Liu wrote: > > src/slave/slave.cpp, line 4373 > > > > > > I think that you are still killing executor, what about update as > > following: > > > > Kill executor [id

Re: Review Request 37273: [1/2]Add CMake macro VsBuildCommand in libprocess.

2015-09-15 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37273/ --- (Updated Sept. 15, 2015, 2:08 p.m.) Review request for mesos, Artem Harutyunyan

Re: Review Request 37275: [2/2]Generate make batch file to build project in windows.

2015-09-15 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37275/ --- (Updated Sept. 15, 2015, 2:19 p.m.) Review request for mesos, Artem Harutyunyan

Re: Review Request 37275: [2/2]Generate make batch file to build project in windows.

2015-09-15 Thread haosdent huang
> On Sept. 11, 2015, 10:49 p.m., Alex Clemmer wrote: > > cmake/MesosConfigure.cmake, line 121 > > > > > > Let me just double check that I understand this code. We're building a > > `make.bat` file, but not because w

Re: Review Request 38259: [MESOS-3340] Command-line flags should take precedence over OS Env variables

2015-09-15 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38259/#review99031 --- Ship it! Ship It! - Marco Massenzio On Sept. 13, 2015, 5:30 a.m.

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-15 Thread Jan Schlicht
> On Sept. 15, 2015, 2:59 p.m., Jan Schlicht wrote: > > Please rebase your patch, latest commits in master lots of JSON related > > stuff. Will review this RR after rebase. - Jan --- This is an automatically generated e-mail. To reply,

Re: Review Request 37275: [2/2]Generate make batch file to build project in windows.

2015-09-15 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37275/ --- (Updated Sept. 15, 2015, 3:16 p.m.) Review request for mesos, Artem Harutyunyan

Re: Review Request 37273: [1/2]Add CMake macro VsBuildCommand in libprocess.

2015-09-15 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37273/ --- (Updated Sept. 15, 2015, 3:18 p.m.) Review request for mesos, Artem Harutyunyan

Re: Review Request 37273: [1/2]Add CMake macro VsBuildCommand in libprocess.

2015-09-15 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37273/ --- (Updated Sept. 15, 2015, 4:40 p.m.) Review request for mesos, Artem Harutyunyan

Re: Review Request 37273: [1/2]Add CMake macro VsBuildCommand in libprocess.

2015-09-15 Thread haosdent huang
> On Sept. 11, 2015, 10:41 p.m., Alex Clemmer wrote: > > 3rdparty/libprocess/3rdparty/CMakeLists.txt, line 192 > > > > > > Hmm, does protobuf build for you? This command should fail on VS2015 > > because stdex is no

Re: Review Request 38399: Add ACLs for the maintenance HTTP endpoints

2015-09-15 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38399/#review99042 --- Thanks for picking this up! A few overall things that you missed (c

Re: Review Request 38382: Fixed the perf event isolator to continue sampling in the presence of failures.

2015-09-15 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38382/#review99046 --- Bad patch! Reviews applied: [38377] Failed command: ./support/appl

Re: Review Request 38031: [3/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-15 Thread Joseph Wu
> On Sept. 15, 2015, 1:41 a.m., Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp, line 128 > > > > > > Haha! Perhaps the most subtle C++11 feature. You mean the comma at the end? :)

Re: Review Request 38011: Maintenance Primitives: Use the parse> helper instead of a plural MachineID protobuf.

2015-09-15 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38011/ --- (Updated Sept. 15, 2015, 10:21 a.m.) Review request for mesos, Alexander Ruklet

Re: Review Request 38031: [3/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-15 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38031/ --- (Updated Sept. 15, 2015, 10:24 a.m.) Review request for mesos, Ben Mahler, Arte

Re: Review Request 37273: [1/2]Add CMake macro VsBuildCommand in libprocess.

2015-09-15 Thread haosdent huang
> On Sept. 11, 2015, 10:41 p.m., Alex Clemmer wrote: > > 3rdparty/libprocess/cmake/macros/VsBuildCommand.bat, line 22 > > > > > > Rather than interpolating a `'#'` between the projects, can we pass in > > all the pro

Re: Review Request 38011: Maintenance Primitives: Use the parse> helper instead of a plural MachineID protobuf.

2015-09-15 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38011/ --- (Updated Sept. 15, 2015, 10:47 a.m.) Review request for mesos, Alexander Ruklet

Re: Review Request 38366: Added helper to model Labels message for state.json.

2015-09-15 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38366/#review99051 --- Alrighty; mind adding a comment above the new model() describing tha

Review Request 38407: Moved files to prepare for unifying provisioners.

2015-09-15 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38407/ --- Review request for mesos, Timothy Chen and Jiang Yan Xu. Bugs: MESOS-3432 h

Re: Review Request 38407: Moved files to prepare for unifying provisioners.

2015-09-15 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38407/#review99053 --- Ship it! Ship It! - Timothy Chen On Sept. 15, 2015, 6:07 p.m., J

Review Request 38408: Renamed provisioners::path to provisioner::path.

2015-09-15 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38408/ --- Review request for mesos, Timothy Chen and Jiang Yan Xu. Bugs: MESOS-3432 h

Re: Review Request 38031: [3/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-15 Thread Michael Park
> On Sept. 15, 2015, 8:41 a.m., Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp, line 128 > > > > > > Haha! Perhaps the most subtle C++11 feature. > > Joseph Wu wrote: > You mea

Re: Review Request 38407: Moved files to prepare for unifying provisioners.

2015-09-15 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38407/#review99057 --- Ship it! My only concern is whether this should be checked in befor

Re: Review Request 38376: Fix 'operator=' spacing in the V1 Scheduler interface.

2015-09-15 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38376/#review99056 --- Patch looks great! Reviews applied: [38376] All tests passed. - M

Re: Review Request 38031: [3/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-15 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38031/#review99058 --- Ship it! Ship It! - Michael Park On Sept. 15, 2015, 5:24 p.m., J

Re: Review Request 38407: Moved files to prepare for unifying provisioners.

2015-09-15 Thread Jie Yu
> On Sept. 15, 2015, 6:38 p.m., Jiang Yan Xu wrote: > > My only concern is whether this should be checked in before > > https://reviews.apache.org/r/38137/ as it's already in the review process > > for a long time. As long as Tim's OK with it. r38137 has to fix problems in the provisioner (whi

Re: Review Request 38172: Stout: Simplified hashset implementation.

2015-09-15 Thread Cong Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38172/#review99060 --- 3rdparty/libprocess/3rdparty/stout/include/stout/hashset.hpp (line

Re: Review Request 37275: [2/2]Generate make batch file to build project in windows.

2015-09-15 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37275/ --- (Updated Sept. 15, 2015, 6:42 p.m.) Review request for mesos, Artem Harutyunyan

Re: Review Request 37273: [1/2]Add CMake macro VsBuildCommand in libprocess.

2015-09-15 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37273/ --- (Updated Sept. 15, 2015, 6:42 p.m.) Review request for mesos, Artem Harutyunyan

Re: Review Request 38407: Moved files to prepare for unifying provisioners.

2015-09-15 Thread Jiang Yan Xu
> On Sept. 15, 2015, 11:38 a.m., Jiang Yan Xu wrote: > > My only concern is whether this should be checked in before > > https://reviews.apache.org/r/38137/ as it's already in the review process > > for a long time. As long as Tim's OK with it. > > Jie Yu wrote: > r38137 has to fix problem

Re: Review Request 38407: Moved files to prepare for unifying provisioners.

2015-09-15 Thread Jie Yu
> On Sept. 15, 2015, 6:38 p.m., Jiang Yan Xu wrote: > > My only concern is whether this should be checked in before > > https://reviews.apache.org/r/38137/ as it's already in the review process > > for a long time. As long as Tim's OK with it. > > Jie Yu wrote: > r38137 has to fix problems

Re: Review Request 38408: Renamed provisioners::path to provisioner::path.

2015-09-15 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38408/ --- (Updated Sept. 15, 2015, 6:51 p.m.) Review request for mesos, Timothy Chen and

Re: Review Request 37532: Add QUIESCE call interface to the scheduler

2015-09-15 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37532/#review99061 --- include/mesos/scheduler.hpp (line 272)

Re: Review Request 38408: Renamed provisioners::path to provisioner::path.

2015-09-15 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38408/#review99065 --- src/slave/containerizer/provisioner/paths.hpp (line 32)

Re: Review Request 38408: Renamed provisioners::path to provisioner::path.

2015-09-15 Thread Jie Yu
> On Sept. 15, 2015, 7:01 p.m., Jiang Yan Xu wrote: > > src/slave/paths.cpp, line 378 > > > > > > I don't think this needs to be changed to singular and the convention > > is that the plural form ("slaves", "framewo

Re: Review Request 37873: Add quiesce logic in allocator

2015-09-15 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37873/#review99069 --- Ship it! Ship It! - Vinod Kone On Sept. 6, 2015, 2:40 a.m., Guan

Re: Review Request 37873: Add quiesce logic in allocator

2015-09-15 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37873/#review99066 --- include/mesos/master/allocator.hpp (line 145)

Re: Review Request 38119: Add metrics of messages_quiesce_offers

2015-09-15 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38119/#review99070 --- Ship it! Ship It! - Vinod Kone On Sept. 4, 2015, 12:06 p.m., Gua

Re: Review Request 38120: Add Java Support for QuiesceOffers

2015-09-15 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38120/#review99071 --- Ship it! src/java/src/org/apache/mesos/SchedulerDriver.java (lines

Re: Review Request 38120: Add Java Support for QuiesceOffers

2015-09-15 Thread Vinod Kone
> On Sept. 15, 2015, 7:12 p.m., Vinod Kone wrote: > > can you also update the java test framework to test this call? - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38120/#review99071

Re: Review Request 38121: Add Python Support for QuiesceOffers

2015-09-15 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38121/#review99073 --- Update the python test framework to send this call. src/python/int

Re: Review Request 38408: Renamed provisioners::path to provisioner::path.

2015-09-15 Thread Jiang Yan Xu
> On Sept. 15, 2015, 12:01 p.m., Jiang Yan Xu wrote: > > src/slave/paths.cpp, line 378 > > > > > > I don't think this needs to be changed to singular and the convention > > is that the plural form ("slaves", "framew

Re: Review Request 38408: Renamed provisioners::path to provisioner::path.

2015-09-15 Thread Jie Yu
> On Sept. 15, 2015, 7:01 p.m., Jiang Yan Xu wrote: > > src/slave/paths.cpp, line 378 > > > > > > I don't think this needs to be changed to singular and the convention > > is that the plural form ("slaves", "framewo

Re: Review Request 38124: Add V1 Support for QuiesceOffers

2015-09-15 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38124/#review99076 --- Ship it! Ship It! - Vinod Kone On Sept. 4, 2015, 12:06 p.m., Gua

Re: Review Request 37873: Add quiesce logic in allocator

2015-09-15 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37873/#review99077 --- src/master/allocator/mesos/hierarchical.hpp (line 229)

Re: Review Request 38126: Add UT for QuiesceOffers

2015-09-15 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38126/#review99078 --- you should've merged this test review with the dependent review. i r

Re: Review Request 37969: Maintenance primitives: Tweak validation error messages to return JSON rather than protobuf.

2015-09-15 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37969/#review99079 --- Patch looks great! Reviews applied: [37969] All tests passed. - M

Re: Review Request 38278: Updating unversioned Executor protobuf

2015-09-15 Thread Isabel Jimenez
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38278/ --- (Updated Sept. 15, 2015, 8:13 p.m.) Review request for mesos, Adam B, Anand Maz

Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-15 Thread Isabel Jimenez
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38143/ --- (Updated Sept. 15, 2015, 8:16 p.m.) Review request for mesos, Adam B, Anand Maz

Re: Review Request 38386: Fix 'operator=' spacing in the registry_client.hpp

2015-09-15 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38386/#review99085 --- Patch looks great! Reviews applied: [38386] All tests passed. - M

Re: Review Request 38386: Fix 'operator=' spacing in the registry_client.hpp

2015-09-15 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38386/#review99086 --- Ship it! Ship It! src/slave/containerizer/provisioners/docker/reg

Re: Review Request 38158: Refactored Value::Ranges coalesce().

2015-09-15 Thread Joerg Schad
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38158/ --- (Updated Sept. 15, 2015, 8:46 p.m.) Review request for mesos, Bernd Mathiske an

Re: Review Request 38408: Renamed provisioners::path to provisioner::path.

2015-09-15 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38408/#review99088 --- Ship it! Ship It! - Timothy Chen On Sept. 15, 2015, 6:51 p.m., J

Re: Review Request 38408: Renamed provisioners::path to provisioner::path.

2015-09-15 Thread Timothy Chen
> On Sept. 15, 2015, 8:51 p.m., Timothy Chen wrote: > > Ship It! Wierd, anyway fix it and ship it :) - Timothy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38408/#review99088 -

Re: Review Request 38408: Renamed provisioners::path to provisioner::path.

2015-09-15 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38408/#review99087 --- src/tests/paths_tests.cpp (line 224)

Re: Review Request 38363: Refactored container Launcher to accept namespaces dynamically.

2015-09-15 Thread Kapil Arya
> On Sept. 14, 2015, 6:32 p.m., Niklas Nielsen wrote: > > src/tests/containerizer/isolator_tests.cpp, line 507 > > > > > > Have you verified the status (non-error) of namespaces() before you get > > here? The Isola

Re: Review Request 38364: Minor refactor for MesosContainerizer.

2015-09-15 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38364/ --- (Updated Sept. 15, 2015, 5:02 p.m.) Review request for mesos, Connor Doyle, Jie

Re: Review Request 38364: Minor refactor for MesosContainerizer.

2015-09-15 Thread Kapil Arya
> On Sept. 14, 2015, 6:55 p.m., Niklas Nielsen wrote: > > src/slave/containerizer/mesos/containerizer.cpp, lines 832-843 > > > > > > How about moving this up as well? Fixed. - Kapil -

Re: Review Request 38365: Updated Isolator::prepare to return list of required namespaces.

2015-09-15 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38365/ --- (Updated Sept. 15, 2015, 5:05 p.m.) Review request for mesos, Connor Doyle, Jie

Re: Review Request 38365: Updated Isolator::prepare to return list of required namespaces.

2015-09-15 Thread Kapil Arya
> On Sept. 14, 2015, 6:50 p.m., Niklas Nielsen wrote: > > include/mesos/slave/isolator.proto, line 75 > > > > > > Taken we still wrap comments at 70, this needs to be wrapped :) We don't :-). See https://github.com/

Re: Review Request 38366: Added helper to model Labels message for state.json.

2015-09-15 Thread Kapil Arya
> On Sept. 15, 2015, 1:56 p.m., Niklas Nielsen wrote: > > src/common/protobuf_utils.cpp, line 140 > > > > > > This could break existing 3rd party parsing; why not leave it set? I am not so sure. This is fairly incon

Re: Review Request 38367: Added NetworkInfo message to ContainerInfo and TaskStatus.

2015-09-15 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38367/ --- (Updated Sept. 15, 2015, 5:06 p.m.) Review request for mesos, Connor Doyle, Jie

Re: Review Request 38367: Added NetworkInfo message to ContainerInfo and TaskStatus.

2015-09-15 Thread Kapil Arya
> On Sept. 14, 2015, 6:19 p.m., Vinod Kone wrote: > > src/common/http.cpp, lines 153-194 > > > > > > Why custom models? Can we not use the protobuf to json converters? This is mostly due to Labels. The way they are

Re: Review Request 38367: Added NetworkInfo message to ContainerInfo and TaskStatus.

2015-09-15 Thread Kapil Arya
> On Sept. 14, 2015, 6:24 p.m., Niklas Nielsen wrote: > > include/mesos/mesos.proto, line 1163 > > > > > > We still go by 70 col comments, right? If so, let's wrap the comment > > blocks. It's been updated :-) ht

Re: Review Request 38367: Added NetworkInfo message to ContainerInfo and TaskStatus.

2015-09-15 Thread Kapil Arya
> On Sept. 14, 2015, 6:43 p.m., Niklas Nielsen wrote: > > src/slave/slave.cpp, line 2760 > > > > > > s/the container IP address if/the container IP address with the IP from > > the agent PID, if/ or something that

Re: Review Request 38368: Replaced slaveTaskStatusLabelDecorator hook with slaveTaskStatusDecorator hook.

2015-09-15 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38368/ --- (Updated Sept. 15, 2015, 5:10 p.m.) Review request for mesos, Connor Doyle, Jie

Re: Review Request 37903: stout: Fix bug in IPNetwork::create() with zero prefix.

2015-09-15 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37903/ --- (Updated Sept. 15, 2015, 9:10 p.m.) Review request for mesos. Bugs: MESOS-332

Re: Review Request 38368: Replaced slaveTaskStatusLabelDecorator hook with slaveTaskStatusDecorator hook.

2015-09-15 Thread Kapil Arya
> On Sept. 14, 2015, 6:11 p.m., Niklas Nielsen wrote: > > src/examples/test_hook_module.cpp, line 198 > > > > > > How about setting the protocol (for testing)? So maybe have two network > > infos (one with each)? A

Re: Review Request 38370: Updated docker executor to set container IP in TaskStatus::NetworkInfo.

2015-09-15 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38370/ --- (Updated Sept. 15, 2015, 5:11 p.m.) Review request for mesos, Connor Doyle, Jie

Re: Review Request 38278: Updating unversioned Executor protobuf

2015-09-15 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38278/#review99102 --- include/mesos/executor/executor.proto (line 131)

Re: Review Request 38278: Updating unversioned Executor protobuf

2015-09-15 Thread Isabel Jimenez
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38278/ --- (Updated Sept. 15, 2015, 9:32 p.m.) Review request for mesos, Adam B, Anand Maz

Re: Review Request 38011: Maintenance Primitives: Use the parse> helper instead of a plural MachineID protobuf.

2015-09-15 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38011/#review99105 --- Patch looks great! Reviews applied: [38011] All tests passed. - M

Re: Review Request 38278: Updating unversioned Executor protobuf

2015-09-15 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38278/#review99110 --- Ship it! Ship It! - Vinod Kone On Sept. 15, 2015, 9:32 p.m., Isa

Re: Review Request 38278: Updating unversioned Executor protobuf

2015-09-15 Thread Anand Mazumdar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38278/#review99109 --- Looks good, just a few more minor things. Also to save effort don't

Re: Review Request 38278: Updating unversioned Executor protobuf

2015-09-15 Thread Anand Mazumdar
> On Sept. 15, 2015, 10:10 p.m., Anand Mazumdar wrote: > > include/mesos/executor/executor.proto, line 163 > > > > > > Minor: Can we also have a comment before the `framework_id` field ? > > > > // Identifie

Re: Review Request 38278: Updating unversioned Executor protobuf

2015-09-15 Thread Isabel Jimenez
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38278/ --- (Updated Sept. 15, 2015, 10:24 p.m.) Review request for mesos, Adam B, Anand Ma

Re: Review Request 38278: Updating unversioned Executor protobuf

2015-09-15 Thread Anand Mazumdar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38278/#review99114 --- Ship it! Ship It! - Anand Mazumdar On Sept. 15, 2015, 10:24 p.m.

Re: Review Request 38003: MESOS-3351 (duplicated slave id in master after master failover)

2015-09-15 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38003/#review99112 --- src/tests/master_tests.cpp (line 3596)

Re: Review Request 37275: [2/2]Generate make batch file to build project in windows.

2015-09-15 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37275/#review99116 --- Patch looks great! Reviews applied: [37273, 37275] All tests passe

Re: Review Request 37540: Add perf event API

2015-09-15 Thread Cong Wang
> On Sept. 15, 2015, 12:31 a.m., Vinod Kone wrote: > > my main question is regarding division of responsibilities between the > > wrapper class and the process class. Looks like opening the fd and mmaping > > is done by the wrapper and the rest by the process. Can everything be done > > by the

Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-15 Thread Isabel Jimenez
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38143/ --- (Updated Sept. 15, 2015, 10:35 p.m.) Review request for mesos, Adam B, Anand Ma

Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-15 Thread Anand Mazumdar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38143/#review99117 --- Ship it! LGTM minus Two Nits: 1. Can we update the description of

Re: Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-15 Thread Isabel Jimenez
> On Sept. 15, 2015, 10:40 p.m., Anand Mazumdar wrote: > > LGTM minus Two Nits: > > > > 1. Can we update the description of the review to just say that this change > > just copies the existing unversioned protobuf to the V1 namespace. > > 2. I am assuming that you would take care of Vinod's ear

  1   2   >