Re: Review Request 36663: Added ip_address field to MasterInfo

> On July 29, 2015, 1:01 p.m., Alexander Rukletsov wrote: > > include/mesos/mesos.proto, line 399 > > > > > > `ip` and `port` are required, while `address` is optional. Is it > > intentional / doesn't it introduce a

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

--- 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 36197: Documented "how to become a committer".

--- 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 M

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32587/#review93560 --- Ship it! LGTM, except for the scariness of the CHECK src/slave/sl

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

--- 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 remo

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

--- 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 Alexand

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

--- 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 Alexand

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

--- 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, @c

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36811/#review93563 --- Ship it! LGTM. Any build experts want to take a quick peek? @tstcla

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

--- 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 Malon

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

--- 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. - M

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

--- 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 passe

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

--- 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 (

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

--- 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 Niklas

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

> On July 30, 2015, 5:03 a.m., Adam B wrote: > > src/slave/slave.cpp, line 1240 > > > > > > Maybe not a CHECK, since that would kill the slave. How about just > > logging an error and, if you're feeling generous, m

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32587/#review93591 --- src/slave/slave.cpp (line 1242)

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

> 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 t

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

--- 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 htt

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

> On July 30, 2015, 12:24 p.m., Adam B wrote: > > src/slave/slave.cpp, lines 1243-1245 > > > > > > Maybe just warn and leave the CopyFrom there? > > Next release, we'll remove the field entirely, and consequentl

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

--- 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.

> On July 30, 2015, 9:24 a.m., Adam B wrote: > > src/slave/slave.cpp, lines 1243-1245 > > > > > > Maybe just warn and leave the CopyFrom there? > > Next release, we'll remove the field entirely, and consequently

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

--- 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 Niklas

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

--- 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.

> On July 30, 2015, 4:45 p.m., Vinod Kone wrote: > > src/tests/containerizer/launcher.hpp, lines 19-37 > > > > > > why did you remove these headers? > > > > i think we decided to explicitly include all the he

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36946/#review93607 --- Patch looks great! Reviews applied: [36946] All tests passed. - M

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

> On July 30, 2015, 4:45 p.m., Vinod Kone wrote: > > src/tests/containerizer/launcher.hpp, lines 19-37 > > > > > > why did you remove these headers? > > > > i think we decided to explicitly include all the he

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

> On July 30, 2015, 4:45 p.m., Vinod Kone wrote: > > src/tests/containerizer/launcher.hpp, lines 19-37 > > > > > > why did you remove these headers? > > > > i think we decided to explicitly include all the he

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

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 do

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

--- 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 36947: Fix new/delete mismatch in stout test.

--- 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.

--- 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. - M

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

> On July 30, 2015, 4:45 p.m., Vinod Kone wrote: > > src/tests/containerizer/launcher.hpp, lines 19-37 > > > > > > why did you remove these headers? > > > > i think we decided to explicitly include all the he

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

> On July 30, 2015, 12:49 a.m., Vinod Kone wrote: > > src/master/master.cpp, lines 1757-1779 > > > > > > checking validationError.isNone() in each if statement looks a bit > > weird. how about doing these in an els

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

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 wrote: > > >

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

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 36951: MESOS-3052: optimize slave ID comparisons

--- 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 https://issues.ap

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

--- 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. B

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36951/#review93641 --- Saw from your comment [here](https://issues.apache.org/jira/browse/

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

--- 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. - M

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

> On July 30, 2015, 9:45 a.m., Vinod Kone wrote: > > src/tests/containerizer/launcher.hpp, lines 19-37 > > > > > > why did you remove these headers? > > > > i think we decided to explicitly include all the he

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

--- 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! 3rdparty/libprocess/3rdp

Review Request 36954: Performed a self bind mount of rootfs itself in fs::chroot::enter().

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36954/ --- Review request for mesos, Ian Downes, Timothy Chen, Vinod Kone, and Jiang Yan Xu

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

--- 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 Xu

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

> 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. > O

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

--- 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.

Review Request 36958: Fixed a bug in authentication validation.

--- 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.

--- 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., Ben

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

--- 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 36844: Libprocess: Used THREAD_LOCAL to replace ThreadLocal.

--- 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 p.m.

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

--- 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.

--- 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 Lehene.

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36959/ --- Review request for mesos, Benjamin Hindman and Joris Van Remoortere. Repository

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

--- 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, 10:46

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36864/#review93667 --- Ship it! Ship It! 3rdparty/libprocess/3rdparty/stout/include/stou

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

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

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

--- 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, 11:51

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

--- 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 p.m

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

--- 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 p.m

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36958/ --- (Updated July 30, 2015, 11:54 p.m.) Review request for mesos and Vinod Kone.

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

--- 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 passe

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

--- 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 mo

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

--- 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 36958: Fixed a bug in authentication validation.

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

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

> On July 31, 2015, 12:05 a.m., Vinod Kone wrote: > > src/tests/authentication_tests.cpp, line 199 > > > > > > s/,/ than Credential::principal/ ? > > > > s/when/even when/ ? > > > > do we already ha

Re: Review Request 36321: Maintenance primitives: Add Unavailability and InverseOffer protobufs.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36321/ --- (Updated July 30, 2015, 6 p.m.) Review request for mesos, Benjamin Hindman, Ben

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

> On July 30, 2015, 5:14 p.m., haosdent huang wrote: > > src/tests/fetcher_tests.cpp, line 332 > > > > > > ``` > > // Tests whether fetcher can process URIs that contain leading > > whitespace > > ``` > >