Re: Review Request 53791: Use the stout ELF parser to collect Linux rootfs files.

2016-12-13 Thread Benjamin Bannier
> On Dec. 13, 2016, 11:13 a.m., Benjamin Bannier wrote: > > src/linux/ldd.cpp, lines 82-83 > > <https://reviews.apache.org/r/53791/diff/6/?file=1571593#file1571593line82> > > > > Let's move this up right after the check `needed.contains(path)`. >

Re: Review Request 54083: Made headers in stout standalone.

2016-12-14 Thread Benjamin Bannier
56f7308677dabebb6a84058bc210e48f10adfbc7 Diff: https://reviews.apache.org/r/54083/diff/ Testing --- make check (OS X, clang trunk w/o optimizations, SSL) Thanks, Benjamin Bannier

Re: Review Request 54085: Made internal Mesos headers more standalone.

2016-12-14 Thread Benjamin Bannier
6576b9001f93cb33c8c4ae83b32ee22ea0354fc7 Diff: https://reviews.apache.org/r/54085/diff/ Testing --- make check (OS X, clang trunk w/o optimizations, SSL) Thanks, Benjamin Bannier

Review Request 54739: Used correct shell string comparison operator in test.

2016-12-14 Thread Benjamin Bannier
various Linux flavors in internal CI. Thanks, Benjamin Bannier

Re: Review Request 54739: Used correct shell string comparison operator in test.

2016-12-14 Thread Benjamin Bannier
0380f2ca122c7b8ab1dac351b14f546526580e72 Diff: https://reviews.apache.org/r/54739/diff/ Testing --- Tested on various Linux flavors in internal CI. Thanks, Benjamin Bannier

Review Request 54753: Leaked module libraries to avoid inconsitencies in library unloading.

2016-12-14 Thread Benjamin Bannier
Testing --- * `make check` in on various Linux flavors in internal CI * `make check` Mac OS 10.12, clang-trunk, SSL-enabled, failures due to unfixed MESOS-6780 Thanks, Benjamin Bannier

Re: Review Request 53582: Updated mesos-tidy image to upstream 3.9 release.

2016-12-15 Thread Benjamin Bannier
. See for https://github.com/bbannier/clang-tools-extra/tree/mesos_39 for one possible form of that branch. Thanks, Benjamin Bannier

Review Request 54796: Avoided capturing `this` in `process::loop`.

2016-12-15 Thread Benjamin Bannier
nk, w/ optimizations, SSL) * `make check` (Fedora 25, clang-3.8, w/optimizations, SSL) Thanks, Benjamin Bannier

Review Request 54825: Made sure process::Loop instances can only be created as shared_ptrs.

2016-12-16 Thread Benjamin Bannier
` tests on various Linux configurations in internal CI Thanks, Benjamin Bannier

Re: Review Request 54035: Extended test coverage of posix/rlimits isolator.

2016-12-19 Thread Benjamin Bannier
/containerizer/posix_rlimits_isolator_tests.cpp 24b19d9eb1cc3e19f8325fbd76182837a5690581 Diff: https://reviews.apache.org/r/54035/diff/ Testing --- `make check` (OS X, clang trunk w/o optimizations) Thanks, Benjamin Bannier

Re: Review Request 54034: Introduced common fixture to PosixRLimitsIsolatorTest.

2016-12-19 Thread Benjamin Bannier
n assertion fails there, test cases relying on the fixture will not be executed. - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54034/#review159503 ------

Review Request 54859: Shortened socket paths used in test.

2016-12-19 Thread Benjamin Bannier
/containerizer/io_switchboard_tests.cpp 930f98f45acb03d6773e85a25481a9dcc4bec78d Diff: https://reviews.apache.org/r/54859/diff/ Testing --- Thanks, Benjamin Bannier

Review Request 54889: Handled all possible offers in test.

2016-12-20 Thread Benjamin Bannier
/54889/diff/ Testing --- `make check` (OS X, clang-trunk, w/ optimizations, SSL) Before this fix `FaultToleranceTest.FrameworkReregister` failed for me multiple times in ~10k iterations; after this patch it didn't fail in a single run I stopped after 170k iterations. Thanks, Benjamin Bannier

Review Request 54896: Fixed copy-template-and-create-symlink make target.

2016-12-20 Thread Benjamin Bannier
commands and fix some whitespace issues. Diffs - src/Makefile.am 0f62ec70816e8b48e19d35036285656a6e7cd02b Diff: https://reviews.apache.org/r/54896/diff/ Testing --- `make distcheck` (Fedora 25) Thanks, Benjamin Bannier

Re: Review Request 54613: Install a symlink rather than building mesos-slave twice.

2016-12-20 Thread Benjamin Bannier
tps://reviews.apache.org/r/54613/#comment230747> Not yours, but since you touch this it would be great if you could make these independent commands (i.e., remove the ` &&` here). I added more cleanups here, https://reviews.apache.org/r/54896/. - Benjamin Bannier On Dec. 20, 2016, 1:

Re: Review Request 54889: Handled all possible offers in test.

2016-12-20 Thread Benjamin Bannier
run I stopped after 170k iterations. Thanks, Benjamin Bannier

Re: Review Request 54889: Handled all possible offers in test.

2016-12-20 Thread Benjamin Bannier
- Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54889/#review159727 --- On Dec. 20, 2016, 11:15 p.m., Benjamin Bannier wrote

Re: Review Request 54889: Handled all possible offers in test.

2016-12-20 Thread Benjamin Bannier
es in order to avoid this gmock assertion. - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54889/#review159727 ------

Re: Review Request 54896: Fixed copy-template-and-create-symlink make target.

2016-12-21 Thread Benjamin Bannier
-- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54896/#review159743 ------- On Dec. 20, 2016, 2:53 p.m., Benjamin Bannier wrote: > > -

Re: Review Request 54896: Fixed copy-template-and-create-symlink make target.

2016-12-21 Thread Benjamin Bannier
--- `make distcheck` (Fedora 25) Thanks, Benjamin Bannier

Re: Review Request 54889: Handled all possible offers in test.

2016-12-21 Thread Benjamin Bannier
't like about the `WillRepeatedly(...)` pattern is that it > > obscures whether additional offers are actually expected or not. Lots of > > tests have copied this pattern and use it, even when another offer is not > > actually expected. > > Benjamin Bannier wrote: >

Review Request 54946: Made sure classes deriving from FlagBase use virtual inheritance.

2016-12-21 Thread Benjamin Bannier
ubuntu-16 build (both SSL, non-SSL). Thanks, Benjamin Bannier

Review Request 54974: Actually overloaded function from base class in LibeventSSLSocketImpl.

2016-12-22 Thread Benjamin Bannier
0e292a8b0d470f4e199db08f09a0c863d73c Diff: https://reviews.apache.org/r/54974/diff/ Testing --- `make check` (on various flavors of Linux in internal CI) Thanks, Benjamin Bannier

Re: Review Request 54999: Fixed test flakiness due to floating point conversions.

2016-12-23 Thread Benjamin Bannier
a paused clock,` since at this point the clock is actually resumed. - Benjamin Bannier On Dec. 22, 2016, 11:54 p.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: >

Re: Review Request 54952: Made `getpwnam_r` error handling more robust.

2016-12-23 Thread Benjamin Bannier
n as it would add robustness to this wrapper to deal with other non-conformant stdlibs. - Benjamin Bannier On Dec. 22, 2016, 9:50 p.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https

Re: Review Request 52877: Fixed wrong float serialization in JSON in locales different from C.

2017-01-05 Thread Benjamin Bannier
removed by accident. 3rdparty/stout/include/stout/jsonify.hpp (line 125) <https://reviews.apache.org/r/52877/#comment231721> I feel this should be named `guard`; it probably also needs a comment so it isn't removed by accident. - Benjamin Bannier

Re: Review Request 55021: Added a framework capabilities struct.

2017-01-05 Thread Benjamin Bannier
214) <https://reviews.apache.org/r/55021/#comment231726> Nit: `Framework::Capability::Type` is probably smaller than the size of a ref, so you could just copy the loop variable here. - Benjamin Bannier On Jan. 5, 2017, 1:19 p

Re: Review Request 55021: Added a framework capabilities struct.

2017-01-05 Thread Benjamin Bannier
also update `mesos::internal::protobuf::frameworkHasCapability` here? - Benjamin Bannier On Jan. 5, 2017, 1:19 p.m., Jay Guo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 55021: Added a framework capabilities struct.

2017-01-05 Thread Benjamin Bannier
> On Jan. 5, 2017, 4:52 p.m., Benjamin Bannier wrote: > > While looking at https://reviews.apache.org/r/55068/, should we also update > > `mesos::internal::protobuf::frameworkHasCapability` here? Please disregard this comment, I got confused about the use value type

Review Request 55269: Avoided use of CHECK macros.

2017-01-06 Thread Benjamin Bannier
/reservation_endpoints_tests.cpp d94fe29e5972e6ed0011ca00cc56fe1c20cda495 src/tests/slave_tests.cpp d633a74d6b342239fbca0b44eec281eb315df5ff Diff: https://reviews.apache.org/r/55269/diff/ Testing --- Tested on various Linux configurations in internal CI. Thanks, Benjamin Bannier

Review Request 55268: Avoided use of CHECK macros in libprocess.

2017-01-06 Thread Benjamin Bannier
various Linux configurations in internal CI. Thanks, Benjamin Bannier

Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-06 Thread Benjamin Bannier
Diff: https://reviews.apache.org/r/55271/diff/ Testing --- Tested on various Linux configurations in internal CI. Thanks, Benjamin Bannier

Re: Review Request 54825: Made sure process::Loop instances can only be created as shared_ptrs.

2017-01-09 Thread Benjamin Bannier
: * `make check` (OS X, clang-trunk, w/optimizations, SSL) * `make check` and `ROOT` tests on various Linux configurations in internal CI Thanks, Benjamin Bannier

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-10 Thread Benjamin Bannier
ernal CI. Thanks, Benjamin Bannier

Re: Review Request 55269: Avoided use of CHECK macros.

2017-01-10 Thread Benjamin Bannier
27;d say no. Would you feel this would warrant its own patch? - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55269/#review160788 ---------

Re: Review Request 55268: Avoided use of CHECK macros in libprocess.

2017-01-10 Thread Benjamin Bannier
Diff: https://reviews.apache.org/r/55268/diff/ Testing --- Tested on various Linux configurations in internal CI. Thanks, Benjamin Bannier

Re: Review Request 55269: Avoided use of CHECK macros.

2017-01-10 Thread Benjamin Bannier
configurations in internal CI. Thanks, Benjamin Bannier

Re: Review Request 55269: Avoided use of CHECK macros.

2017-01-10 Thread Benjamin Bannier
set it to > > `Error()` to validate the idea. > > Benjamin Bannier wrote: > I tested this in internal CI on an incompletly configured machine; after > this fix I was able to see failures in e.g., from `PortMappingIsolatorTest` > where previously the code would have just ab

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-10 Thread Benjamin Bannier
. Thanks, Benjamin Bannier

Review Request 55381: Added test for framework upgrading to multi-role capability.

2017-01-10 Thread Benjamin Bannier
) Thanks, Benjamin Bannier

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-11 Thread Benjamin Bannier
internal CI. Thanks, Benjamin Bannier

Re: Review Request 55381: Added test for framework upgrading to multi-role capability.

2017-01-11 Thread Benjamin Bannier
: https://reviews.apache.org/r/55381/diff/ Testing --- make check (OS X) Thanks, Benjamin Bannier

Review Request 55462: WIP: Validate resource reservation against allocated role.

2017-01-12 Thread Benjamin Bannier
yet. Thanks, Benjamin Bannier

Review Request 55461: Made resource reservation validation multi-role aware.

2017-01-12 Thread Benjamin Bannier
1779ff7444904c2ad7bad33aaf9167b98d05 src/master/validation.cpp 96aa36585ded4bd7cf98526f710ccbc4f23b1f0f src/tests/master_validation_tests.cpp e5d55e03648cb218d42adc594d6fa7d40ea9bcbb Diff: https://reviews.apache.org/r/55461/diff/ Testing --- make check Thanks, Benjamin Bannier

Re: Review Request 55381: Added test for framework upgrading to multi-role capability.

2017-01-12 Thread Benjamin Bannier
s.cpp e5d55e03648cb218d42adc594d6fa7d40ea9bcbb Diff: https://reviews.apache.org/r/55381/diff/ Testing --- make check (OS X) Thanks, Benjamin Bannier

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-12 Thread Benjamin Bannier
) - src/master/master.cpp 1746a88953dbdc148d98881bcf7027b62ad6b040 src/tests/master_validation_tests.cpp e5d55e03648cb218d42adc594d6fa7d40ea9bcbb Diff: https://reviews.apache.org/r/55271/diff/ Testing --- Tested on various Linux configurations in internal CI. Thanks, Benjamin

Re: Review Request 55488: Slimmed down the size of the docker image from 2GB to roughly 650MB.

2017-01-13 Thread Benjamin Bannier
- > > (Updated Jan. 13, 2017, 10:54 a.m.) > > > Review request for mesos and Benjamin Bannier. > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > support/mesos-

Re: Review Request 55489: Used CMake to generate the compilation database instead.

2017-01-13 Thread Benjamin Bannier
.org/r/55489/#comment232806> Dito. - Benjamin Bannier On Jan. 13, 2017, 10:59 a.m., Michael Park wrote: > > --- > This is an automatically generated e-mail. To

Re: Review Request 55491: Added a `jenkins/tidybot.sh`.

2017-01-13 Thread Benjamin Bannier
configuration into version control? Right now this seems as if Jenkins could just be configured to point to the script directly; we'd then be able to tweak the scripts. - Benjamin Bannier On Jan. 13, 2017, 11:06 a.m., Michael Park

Re: Review Request 55496: Fixed parsing of proxy CONNECT responses.

2017-01-13 Thread Benjamin Bannier
tps://reviews.apache.org/r/55496/#comment232837> If you made `encoding` an `Option` and dropped the `getOrElse` unwrapping, you could directly compare it against `Some("chunked")` below. - Benjamin Bannier On Jan. 13, 2017, 2:48 p.m., Ja

Re: Review Request 55461: Made resource reservation validation multi-role aware.

2017-01-13 Thread Benjamin Bannier
tests.cpp e5d55e03648cb218d42adc594d6fa7d40ea9bcbb Diff: https://reviews.apache.org/r/55461/diff/ Testing --- make check Thanks, Benjamin Bannier

Re: Review Request 55461: Made resource reservation validation multi-role aware.

2017-01-13 Thread Benjamin Bannier
13, 2017, 4:50 p.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55461/ > --- > &

Re: Review Request 55461: Made resource reservation validation multi-role aware.

2017-01-13 Thread Benjamin Bannier
ment? - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55461/#review161488 ------- On Jan. 13, 2017, 4:50 p.m., Benjamin Bann

Re: Review Request 55488: Slimmed down the size of the docker image from 2GB to roughly 650MB.

2017-01-14 Thread Benjamin Bannier
> On Jan. 13, 2017, 12:53 p.m., Benjamin Bannier wrote: > > support/mesos-tidy/Dockerfile, lines 69-77 > > <https://reviews.apache.org/r/55488/diff/1/?file=1604404#file1604404line69> > > > > These packages are from the Mesos getting started guide, and I

Re: Review Request 55489: Used CMake to generate the compilation database instead.

2017-01-14 Thread Benjamin Bannier
> On Jan. 13, 2017, 1 p.m., Benjamin Bannier wrote: > > support/mesos-tidy/entrypoint.sh, line 27 > > <https://reviews.apache.org/r/55489/diff/2/?file=1604531#file1604531line27> > > > > Left over debug tooling? > > Michael Park wrote: > I us

Re: Review Request 55489: Used CMake to generate the compilation database instead.

2017-01-15 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55489/#review161662 --- Ship it! - Benjamin Bannier On Jan. 14, 2017, 9:01 p.m

Re: Review Request 55490: Used the `mesos/mesos-tidy` image from DockerHub.

2017-01-15 Thread Benjamin Bannier
the `docker pull` to ensure a recent enough image, but let's remove the `docker rmi`. - Benjamin Bannier On Jan. 14, 2017, 9:02 p.m., Michael Park wrote: > > --- > This is an automatically generated e-mail. To reply, visit: >

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-16 Thread Benjamin Bannier
ed information, however it currently returns just `void` where instead we probably need a `Try`. Let me try to improve `updateFrameworkInfo` so we can check there. - Benjamin --- This is an automatically generated e-mail. T

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-16 Thread Benjamin Bannier
with a multi-role scenario. Was that too subtle? I would like to avoid excessive redundancy in the test name. - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/#r

Re: Review Request 55381: Added test for framework upgrading to multi-role capability.

2017-01-16 Thread Benjamin Bannier
an make sure the next patch does not introduce regressions. What do you think. - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55381/#review161676 ---------

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-16 Thread Benjamin Bannier
in internal CI. Thanks, Benjamin Bannier

Review Request 55571: Changed Master::Framework::updateFrameworkInfo so it can return errors.

2017-01-16 Thread Benjamin Bannier
/ Testing --- Tested as part of https://reviews.apache.org/r/55271/. Thanks, Benjamin Bannier

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-16 Thread Benjamin Bannier
various Linux configurations in internal CI. Thanks, Benjamin Bannier

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-16 Thread Benjamin Bannier
o against "old" one if available. And this may be > > done in `updateFrameworkInfo`? > > Benjamin Bannier wrote: > Very good point. I agree that moving all these checks to > `updateFrameworkInfo` makes most sense as there we have all needed > info

Re: Review Request 55461: Made resource reservation validation multi-role aware.

2017-01-16 Thread Benjamin Bannier
com/apache/mesos/blob/master/src/common/roles.cpp#L67 > > Also, should the case `framework->info.has_role() == false && > > framework->info.role() != "*"` be invalid and caught upon subscription? > > Benjamin Bannier wrote: > Multi-role frameworks

Re: Review Request 55461: Made resource reservation validation multi-role aware.

2017-01-16 Thread Benjamin Bannier
a9bcbb Diff: https://reviews.apache.org/r/55461/diff/ Testing --- make check Thanks, Benjamin Bannier

Re: Review Request 55571: Changed Master::Framework::updateFrameworkInfo so it can return errors.

2017-01-17 Thread Benjamin Bannier
44f4fecb1fbe8bebf830990a59a5462338e6e004 src/master/master.cpp b863ff6e93931c3d1ee056248084c7f44caf2fd9 Diff: https://reviews.apache.org/r/55571/diff/ Testing --- Tested as part of https://reviews.apache.org/r/55271/. Thanks, Benjamin Bannier

Re: Review Request 55571: Changed Master::Framework::updateFrameworkInfo so it can return errors.

2017-01-17 Thread Benjamin Bannier
t()->pid, DEFAULT_CREDENTIAL); > > > > Future registered; > > EXPECT_CALL(sched, registered(&driver, _, _)) > > .WillOnce(FutureSatisfy(®istered)); > > > > driver.start(); > > > > Clock::pause();

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-17 Thread Benjamin Bannier
) - src/master/master.hpp 44f4fecb1fbe8bebf830990a59a5462338e6e004 src/tests/master_validation_tests.cpp e5d55e03648cb218d42adc594d6fa7d40ea9bcbb Diff: https://reviews.apache.org/r/55271/diff/ Testing --- Tested on various Linux configurations in internal CI. Thanks, Benjamin

Review Request 55625: Prevented certain kinds of gaming the quota system.

2017-01-17 Thread Benjamin Bannier
_allocator_tests.cpp 1edd0ecc8a93cd41532e1cf3641f67c780ab23a5 Diff: https://reviews.apache.org/r/55625/diff/ Testing --- Tested of various Linux configurations in internal CI. Thanks, Benjamin Bannier

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-17 Thread Benjamin Bannier
. Thanks, Benjamin Bannier

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-17 Thread Benjamin Bannier
------- On Jan. 17, 2017, 11:20 p.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55271/ >

Re: Review Request 55491: Added a `support/README.md` and the `support/jenkins` directory.

2017-01-18 Thread Benjamin Bannier
rom the outside, e.g., via an environment variable? We very likely also need to soon change this password now since it is known to everybody. support/jenkins/tidybot.sh (line 22) <https://reviews.apache.org/r/55491/#comment28> `MESOS_DIRECTORY`? - Benjamin Ba

Re: Review Request 55654: Renamed `CONFIGURE_FLAGS` to `CMAKE_ARGS`.

2017-01-18 Thread Benjamin Bannier
) <https://reviews.apache.org/r/55654/#comment233318> Not yours, but would you mind changing this to `This directory is intended for ...`? support/mesos-tidy/README.md (line 48) <https://reviews.apache.org/r/55654/#comment29> nit: Could you add an extra newline? - Benj

Re: Review Request 55490: Used the `mesos/mesos-tidy` image from DockerHub.

2017-01-18 Thread Benjamin Bannier
g/r/55490/#comment233317> Let's keep this further down so to not take away from `CHECKS` prominence. Any reason we'd want to deviate from `MESOS_DIRECTORY` used elsewhere? They seem to mean the same thing (extracted in slightly different ways). - Benjamin Bannier On Jan. 18

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-18 Thread Benjamin Bannier
> On Jan. 17, 2017, 10:10 a.m., Jay Guo wrote: > > Also, should we allow user to downgrade from a multi-role framework to > > single-role? I feel it would be very complicated and we should explicitly > > disallow that... > > Benjamin Bannier wrote: > I am

Re: Review Request 55667: Added `libssl-dev` to `support/mesos-tidy/Dockerfile`.

2017-01-18 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55667/#review162094 --- Ship it! Ship It! - Benjamin Bannier On Jan. 18, 2017, 9:57

Re: Review Request 55490: Used the `mesos/mesos-tidy` image from DockerHub.

2017-01-18 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55490/#review162095 --- Ship it! Modulo my comments. - Benjamin Bannier On Jan. 18

Re: Review Request 55490: Used the `mesos/mesos-tidy` image from DockerHub.

2017-01-18 Thread Benjamin Bannier
> On Jan. 18, 2017, 1:53 p.m., Benjamin Bannier wrote: > > support/mesos-tidy.sh, line 22 > > <https://reviews.apache.org/r/55490/diff/6/?file=1607372#file1607372line22> > > > > Let's keep this further down so to not take away from `CHECKS` > >

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-18 Thread Benjamin Bannier
55271/#review162021 --- On Jan. 17, 2017, 11:20 p.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-18 Thread Benjamin Bannier
internal CI. Thanks, Benjamin Bannier

Re: Review Request 55571: Changed Master::Framework::updateFrameworkInfo so it can return errors.

2017-01-18 Thread Benjamin Bannier
73159328ce3fd838e02eba0e6a30cf69efc319ba Diff: https://reviews.apache.org/r/55571/diff/ Testing --- Tested as part of https://reviews.apache.org/r/55271/. Thanks, Benjamin Bannier

Re: Review Request 55381: Added test for framework upgrading to multi-role capability.

2017-01-18 Thread Benjamin Bannier
: https://reviews.apache.org/r/55381/diff/ Testing --- make check (OS X) Thanks, Benjamin Bannier

Re: Review Request 55491: Added a `support/README.md` and the `support/jenkins` directory.

2017-01-18 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55491/#review162141 --- Ship it! Ship It! - Benjamin Bannier On Jan. 18, 2017, 5:38

Re: Review Request 55571: Changed Master::Framework::updateFrameworkInfo so it can return errors.

2017-01-18 Thread Benjamin Bannier
frameworkInfo.clear_role(); > > frameworkInfo.add_capabilities()->set_type( > > FrameworkInfo::Capability::MULTI_ROLE); > > > > MockScheduler sched; > > MesosSchedulerDriver driver( > > &sched, frameworkInfo, master.get

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-18 Thread Benjamin Bannier
://reviews.apache.org/r/55271/diff/ Testing --- Tested on various Linux configurations in internal CI. Thanks, Benjamin Bannier

Re: Review Request 55462: WIP: Validate resource reservation against allocated role.

2017-01-18 Thread Benjamin Bannier
96aa36585ded4bd7cf98526f710ccbc4f23b1f0f Diff: https://reviews.apache.org/r/55462/diff/ Testing --- N/A yet. Thanks, Benjamin Bannier

Re: Review Request 55490: Used the `mesos/mesos-tidy` image from DockerHub.

2017-01-19 Thread Benjamin Bannier
> On Jan. 15, 2017, 10:55 a.m., Benjamin Bannier wrote: > > This is great. Could you make sure to follow up with setting > > `mesos/mesos-tidy` up for automated builds? Before building the image in > > the dockerhub time constraints was hard, but I am optimistic this wou

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-19 Thread Benjamin Bannier
/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b src/tests/master_validation_tests.cpp c092362152e1fe8a6b615c2eda171d852c1bbd86 Diff: https://reviews.apache.org/r/55271/diff/ Testing --- Tested on various Linux configurations in internal CI. Thanks, Benjamin Bannier

Re: Review Request 55701: Fixed unsafe usage of process pointer in async.hpp.

2017-01-19 Thread Benjamin Bannier
)`, should this one also be adjusted, https://github.com/apache/mesos/blob/745b3c7589e5252cf93f62e081b78fa420771d0c/3rdparty/libprocess/include/process/loop.hpp#L134-L144? - Benjamin Bannier On Jan. 19, 2017, 3:41 a.m., Joseph Wu wrote

Re: Review Request 55740: Fixed unsafe usage of process pointer in loop.hpp.

2017-01-20 Thread Benjamin Bannier
actual danger from this code today. This change future-proofs this code by raising the level of abstraction. - Benjamin Bannier On Jan. 20, 2017, 2:34 a.m., Joseph Wu wrote: > > --- > This is an automatically generated e-mail. To rep

Review Request 55773: Avoided optimizing if we are working with a libcxx with UB.

2017-01-20 Thread Benjamin Bannier
--- * tested on various Linux configurations internal CI * tested on OS X Sierra with the bundled compiler+stdlib and versions close to their respective HEADs Thanks, Benjamin Bannier

Review Request 55772: Avoided optimizing if we are working with a libcxx with UB.

2017-01-20 Thread Benjamin Bannier
enable-optimize CXX=/usr/bin/clang++ $ autoreconf -fi && ./configure --enable-optimize CXX=clang++ # versions from HEAD Thanks, Benjamin Bannier

Review Request 55771: Avoided optimizing if we are working with a libcxx with UB.

2017-01-20 Thread Benjamin Bannier
imize CXX=/usr/bin/clang++ $ autoreconf -fi && ./configure --enable-optimize CXX=clang++ # versions from HEAD Thanks, Benjamin Bannier

Re: Review Request 55381: Added test for framework upgrading to multi-role capability.

2017-01-20 Thread Benjamin Bannier
- Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55381/#review162351 --- On Jan. 18, 2017, 4:26 p.m., Benjamin Bannier w

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

2017-01-20 Thread Benjamin Bannier
/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b src/tests/master_validation_tests.cpp a63178139a5283d6a3fcbe60c271dab1914e5da9 Diff: https://reviews.apache.org/r/55271/diff/ Testing --- Tested on various Linux configurations in internal CI. Thanks, Benjamin Bannier

Re: Review Request 55773: Rejected optimizing if we are working with a libcxx with UB.

2017-01-20 Thread Benjamin Bannier
HEADs Thanks, Benjamin Bannier

Re: Review Request 55772: Rejected optimizing if we are working with a libcxx with UB.

2017-01-20 Thread Benjamin Bannier
/usr/bin/clang++ $ autoreconf -fi && ./configure CXX=clang++ # versions from HEAD $ autoreconf -fi && ./configure --enable-optimize CXX=/usr/bin/clang++ $ autoreconf -fi && ./configure --enable-optimize CXX=clang++ # versions from HEAD Thanks, Benjamin Bannier

Re: Review Request 55771: Rejected optimizing if we are working with a libcxx with UB.

2017-01-20 Thread Benjamin Bannier
in/clang++ $ autoreconf -fi && ./configure CXX=clang++ # versions from HEAD $ autoreconf -fi && ./configure --enable-optimize CXX=/usr/bin/clang++ $ autoreconf -fi && ./configure --enable-optimize CXX=clang++ # versions from HEAD Thanks, Benjamin Bannier

Re: Review Request 55810: Fixed bug allowing IOSwitchboard::connect() after container destruction.

2017-01-21 Thread Benjamin Bannier
I was seeing in 1000+ test iterations testing both parameterizations of this test. - Benjamin Bannier On Jan. 21, 2017, 2:03 a.m., Kevin Klues wrote: > > --- > This is an automatically generated e-mail. To reply,

<    3   4   5   6   7   8   9   10   11   12   >