Re: Review Request 36910: Patch configure.ac to include $LIBS in the CRAM-MD5 check

2015-07-30 Thread Chris Heller
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36910/#review93579 --- It's unclear what failed in that auto build. It appears unrelated

Review Request 36951: MESOS-3052: optimize slave ID comparisons

2015-07-30 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36951/ --- Review request for mesos and Ben Mahler. Bugs: MESOS-3052

Re: Review Request 36951: MESOS-3052: optimize slave ID comparisons

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

Re: Review Request 36929: Fixed a few issues in test launcher header.

2015-07-30 Thread Vinod Kone
On July 30, 2015, 4:45 p.m., Vinod Kone wrote: src/tests/containerizer/launcher.hpp, lines 19-37 https://reviews.apache.org/r/36929/diff/1/?file=1024924#file1024924line19 why did you remove these headers? i think we decided to explicitly include all the headers that are

Re: Review Request 36929: Fixed a few issues in test launcher header.

2015-07-30 Thread Benjamin Mahler
Jie, I thought that duplicate includes of headers don't have a significant impact on compile times given our include guards, why do you say it slows down the compilation? e.g. https://gcc.gnu.org/onlinedocs/cppinternals/Guard-Macros.html On Thu, Jul 30, 2015 at 12:57 PM, Vinod Kone

Re: Review Request 36951: MESOS-3052: optimize slave ID comparisons

2015-07-30 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36951/ --- (Updated July 30, 2015, 8:43 p.m.) Review request for mesos and Ben Mahler.

Re: Review Request 36929: Fixed a few issues in test launcher header.

2015-07-30 Thread Jie Yu
aha, I just thought it might slow down the compilation. But looks like it will not given the optimization. I guess clang should have the same optimization as well. The burden of updating the headers while doing refactor is real. It'll be really cool if we can automate this. BTW: the code base is

Review Request 36958: Fixed a bug in authentication validation.

2015-07-30 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36958/ --- Review request for mesos and Vinod Kone. Repository: mesos Description

Re: Review Request 36927: Pulled apart authorization and authentication validation for frameworks in the master.

2015-07-30 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36927/#review93657 --- Ship it! Ship It! - Vinod Kone On July 30, 2015, 10:16 p.m.,

Re: Review Request 36958: Fixed a bug in authentication validation.

2015-07-30 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36958/#review93679 --- Ship it! src/tests/authentication_tests.cpp (line 199)

Re: Review Request 34129: Add 2 optional args advertise_ip and advertise_port for libprocess to advertise.

2015-07-30 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34129/#review93678 --- Patch looks great! Reviews applied: [34128, 34129] All tests

Re: Review Request 36828: Used std::thread instead of pthread for stout proc tests.

2015-07-30 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36828/#review93654 --- Ship it! I'll fix up and commit, thanks!

Re: Review Request 34128: Enable different IP/Port for external access.

2015-07-30 Thread Anindya Sinha
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34128/ --- (Updated July 30, 2015, 10:36 p.m.) Review request for mesos. Changes

Re: Review Request 34129: Add 2 optional args advertise_ip and advertise_port for libprocess to advertise.

2015-07-30 Thread Anindya Sinha
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34129/ --- (Updated July 30, 2015, 10:36 p.m.) Review request for mesos and Cosmin

Re: Review Request 36844: Libprocess: Used THREAD_LOCAL to replace ThreadLocal.

2015-07-30 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36844/#review93659 --- Ship it! Ship It! - Benjamin Hindman On July 27, 2015, 9:15

Re: Review Request 36958: Fixed a bug in authentication validation.

2015-07-30 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36958/#review93658 --- Mind adding a test for this if one doesn't exist already? Should be

Re: Review Request 36865: Style change: Space after the ... in variadic templates.

2015-07-30 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36865/#review93669 --- Ship it! Ship It! - Joris Van Remoortere On July 29, 2015,

Re: Review Request 36929: Fixed a few issues in test launcher header.

2015-07-30 Thread Jiang Yan Xu
On July 30, 2015, 9:45 a.m., Vinod Kone wrote: src/tests/containerizer/launcher.hpp, lines 19-37 https://reviews.apache.org/r/36929/diff/1/?file=1024924#file1024924line19 why did you remove these headers? i think we decided to explicitly include all the headers that are

Re: Review Request 36864: Style change: Space after the ... in variadic templates.

2015-07-30 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36864/#review93667 --- Ship it! Ship It!

Re: Review Request 36958: Fixed a bug in authentication validation.

2015-07-30 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36958/#review93668 --- Bad patch! Reviews applied: [36927] Failed command:

Re: Review Request 36783: Windows: Header splitting continued (stout/os.hpp)

2015-07-30 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36783/#review93672 --- Ship it! Ship It! - Benjamin Hindman On July 29, 2015, 11:24

Re: Review Request 36958: Fixed a bug in authentication validation.

2015-07-30 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36958/#review93687 --- Bad patch! Reviews applied: [36927] Failed command:

Review Request 36956: Created a test abstraction for preparing test rootfs.

2015-07-30 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36956/ --- Review request for mesos, Ian Downes, Timothy Chen, Vinod Kone, and Jiang Yan

Re: Review Request 36959: Code safety: Remove capture by reference of a temporary.

2015-07-30 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36959/#review93663 --- Ship it! Ship It! - Joris Van Remoortere On July 30, 2015,

Re: Review Request 36930: Forced the network isolator to use the mount namespace.

2015-07-30 Thread Chi Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36930/#review93680 --- Ship it! Could you make a comment that port mapping doesn't need

Re: Review Request 36958: Fixed a bug in authentication validation.

2015-07-30 Thread Ben Mahler
On July 31, 2015, 12:05 a.m., Vinod Kone wrote: src/tests/authentication_tests.cpp, line 199 https://reviews.apache.org/r/36958/diff/2/?file=1025323#file1025323line199 s/,/ than Credential::principal/ ? s/when/even when/ ? do we already have a test for when

Re: Review Request 36946: Factored out the pattern for URL generation in a fetcher test.

2015-07-30 Thread Klaus Ma
On July 30, 2015, 5:14 p.m., haosdent huang wrote: src/tests/fetcher_tests.cpp, line 332 https://reviews.apache.org/r/36946/diff/1/?file=1025117#file1025117line332 ``` // Tests whether fetcher can process URIs that contain leading whitespace ``` Does this patch

Re: Review Request 36821: Fix disable endpoints rule fails to recognize HTTP path delegates.

2015-07-30 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36821/#review93557 --- Ship it! - Alexander Rojas On July 29, 2015, 7:40 p.m., haosdent

Re: Review Request 36663: Added ip_address field to MasterInfo

2015-07-30 Thread Alexander Rukletsov
On July 29, 2015, 1:01 p.m., Alexander Rukletsov wrote: include/mesos/mesos.proto, line 399 https://reviews.apache.org/r/36663/diff/5/?file=1021578#file1021578line399 `ip` and `port` are required, while `address` is optional. Is it intentional / doesn't it introduce a pitfall?

Re: Review Request 36197: Documented how to become a committer.

2015-07-30 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36197/#review93558 --- Ship it! Ship It! - Adam B On July 28, 2015, 11:02 a.m., Bernd

Review Request 36946: Factored out the pattern for URL generation in a fetcher test.

2015-07-30 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36946/ --- Review request for mesos, Bernd Mathiske and Klaus Ma. Bugs: MESOS-3023

Re: Review Request 36910: Patch configure.ac to include $LIBS in the CRAM-MD5 check

2015-07-30 Thread Vinod Kone
On July 30, 2015, 12:28 p.m., Chris Heller wrote: It's unclear what failed in that auto build. It appears unrelated (potentially). As a test I pulled my branch, and rebased from master, then configured a build and ran `make -j3 distcheck` and was successful in building. Can the auto

Re: Review Request 32587: Stopped using RunTaskMessage.framework_id.

2015-07-30 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32587/ --- (Updated July 30, 2015, 12:54 p.m.) Review request for mesos, Adam B and

Re: Review Request 36947: Fix new/delete mismatch in stout test.

2015-07-30 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36947/#review93619 --- Ship it! Note that subprocess is in libprocess rather than stout

Re: Review Request 36947: Fix new/delete mismatch in stout test.

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

Re: Review Request 36819: Use setup.py in python cli package.

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

Re: Review Request 36929: Fixed a few issues in test launcher header.

2015-07-30 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36929/#review93599 --- src/tests/containerizer/launcher.hpp (lines 19 - 24)

Re: Review Request 32587: Stopped using RunTaskMessage.framework_id.

2015-07-30 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32587/ --- (Updated July 30, 2015, 12:18 p.m.) Review request for mesos, Adam B and

Re: Review Request 32587: Stopped using RunTaskMessage.framework_id.

2015-07-30 Thread Kapil Arya
On July 30, 2015, 5:03 a.m., Adam B wrote: src/slave/slave.cpp, line 1240 https://reviews.apache.org/r/32587/diff/5/?file=1023551#file1023551line1240 Maybe not a CHECK, since that would kill the slave. How about just logging an error and, if you're feeling generous, maybe sending

Re: Review Request 32587: Stopped using RunTaskMessage.framework_id.

2015-07-30 Thread Adam B
On July 30, 2015, 9:24 a.m., Adam B wrote: src/slave/slave.cpp, lines 1243-1245 https://reviews.apache.org/r/32587/diff/6/?file=1025122#file1025122line1243 Maybe just warn and leave the CopyFrom there? Next release, we'll remove the field entirely, and consequently this

Re: Review Request 36946: Factored out the pattern for URL generation in a fetcher test.

2015-07-30 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36946/#review93605 --- src/tests/fetcher_tests.cpp (line 332)

Re: Review Request 36929: Fixed a few issues in test launcher header.

2015-07-30 Thread Jie Yu
On July 30, 2015, 4:45 p.m., Vinod Kone wrote: src/tests/containerizer/launcher.hpp, lines 19-37 https://reviews.apache.org/r/36929/diff/1/?file=1024924#file1024924line19 why did you remove these headers? i think we decided to explicitly include all the headers that are

Re: Review Request 36927: Pulled apart authorization and authentication validation for frameworks in the master.

2015-07-30 Thread Ben Mahler
On July 30, 2015, 12:49 a.m., Vinod Kone wrote: Kept the validation error composition per our offline discussion, returning for each case individually led to really verbose code, and we looked at using a lambda to leverage 'return', but this seemed to be the simplest route for now. On

Re: Review Request 36927: Pulled apart authorization and authentication validation for frameworks in the master.

2015-07-30 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36927/ --- (Updated July 30, 2015, 10:16 p.m.) Review request for mesos and Vinod Kone.

Re: Review Request 36625: Windows: Split up platform specific functions into separate headers.

2015-07-30 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36625/#review93671 --- Ship it! Ship It! - Benjamin Hindman On July 29, 2015, 11:18

Re: Review Request 36929: Fixed a few issues in test launcher header.

2015-07-30 Thread Vinod Kone
On July 30, 2015, 4:45 p.m., Vinod Kone wrote: src/tests/containerizer/launcher.hpp, lines 19-37 https://reviews.apache.org/r/36929/diff/1/?file=1024924#file1024924line19 why did you remove these headers? i think we decided to explicitly include all the headers that are

Re: Review Request 36929: Fixed a few issues in test launcher header.

2015-07-30 Thread Jie Yu
On July 30, 2015, 4:45 p.m., Vinod Kone wrote: src/tests/containerizer/launcher.hpp, lines 19-37 https://reviews.apache.org/r/36929/diff/1/?file=1024924#file1024924line19 why did you remove these headers? i think we decided to explicitly include all the headers that are

Re: Review Request 36929: Fixed a few issues in test launcher header.

2015-07-30 Thread Benjamin Mahler
Well, it does seem easier to maintain includes if we rely on the parent header demonstrating intent to provide symbols (e.g. adding a vector to an interface does not require adding includes in all child files). If it provides significant speedup to build times, it would be very compelling! How

Review Request 36947: Fix new/delete mismatch in stout test.

2015-07-30 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36947/ --- Review request for mesos, Benjamin Hindman, Ben Mahler, Jie Yu, and Vinod Kone.

Re: Review Request 36811: Don't check protobuf jar when --disable-java flag.

2015-07-30 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36811/ --- (Updated July 30, 2015, 9:47 a.m.) Review request for mesos, Adam B, Cody

Re: Review Request 36821: Fix disable endpoints rule fails to recognize HTTP path delegates.

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

Re: Review Request 36821: Fix disable endpoints rule fails to recognize HTTP path delegates.

2015-07-30 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36821/ --- (Updated July 30, 2015, 9:29 a.m.) Review request for mesos, Adam B and

Re: Review Request 36810: Don't check protobuf jar in libprocess.

2015-07-30 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36810/#review93565 --- Ship it! LGTM. Any build experts want to take a look? @tstclair,

Re: Review Request 36821: Fix disable endpoints rule fails to recognize HTTP path delegates.

2015-07-30 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36821/#review93561 --- Ship it! Looks great! Unless anybody has any objections, I can

Re: Review Request 36821: Fix disable endpoints rule fails to recognize HTTP path delegates.

2015-07-30 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36821/ --- (Updated July 30, 2015, 9:29 a.m.) Review request for mesos, Adam B and

Re: Review Request 36811: Don't check protobuf jar when --disable-java flag.

2015-07-30 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36811/#review93570 --- Patch looks great! Reviews applied: [36810, 36811] All tests