Re: Review Request 24569: Added missing 'defer' helpers.

2014-08-11 Thread Benjamin Hindman
--- On Aug. 11, 2014, 6:45 p.m., Benjamin Hindman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24569

Review Request 24588: Added Docker::pull.

2014-08-11 Thread Benjamin Hindman
Description --- See summary. Diffs - src/docker/docker.hpp 98b2d6099988f51f12e7b108e73dcfd0143adc48 src/docker/docker.cpp 1cba381118c6bd2ac7fcf5a8a229602e2c65c571 Diff: https://reviews.apache.org/r/24588/diff/ Testing --- make check Thanks, Benjamin Hindman

Review Request 24593: Updated Docker::run to look for entrypoint.

2014-08-12 Thread Benjamin Hindman
--- See summary. Diffs - src/docker/docker.hpp 98b2d6099988f51f12e7b108e73dcfd0143adc48 src/docker/docker.cpp 1cba381118c6bd2ac7fcf5a8a229602e2c65c571 Diff: https://reviews.apache.org/r/24593/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 24475: Add new Docker configurations

2014-08-12 Thread Benjamin Hindman
A CHECK_FAILED(status) would be nice here, but I don't think it's necessary if this function just takes a string. - Benjamin Hindman On Aug. 13, 2014, 2:59 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail

Re: Review Request 24631: Refactored the protobuf message comparison logic.

2014-08-12 Thread Benjamin Hindman
this, but let's profile and optimize later if necessary! ;-) - Benjamin Hindman On Aug. 13, 2014, 5:17 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24631

Re: Review Request 24632: Updated health check to use the new CommandInfo.

2014-08-12 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24632/#review50410 --- Ship it! Ship It! - Benjamin Hindman On Aug. 13, 2014, 5:21 a.m

Re: Review Request 24633: Updated mesos containerizer launcher to use the new CommandInfo.

2014-08-12 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24633/#review50411 --- Ship it! Ship It! - Benjamin Hindman On Aug. 13, 2014, 5:25 a.m

Re: Review Request 24634: Updated slave http to use the new CommmandInfo.

2014-08-12 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24634/#review50412 --- Ship it! Ship It! - Benjamin Hindman On Aug. 13, 2014, 5:25 a.m

Re: Review Request 24635: Updated command executor to use the new CommandInfo.

2014-08-12 Thread Benjamin Hindman
that explains this reasoning as much, and maybe even a TODO to do some validation in the master? - Benjamin Hindman On Aug. 13, 2014, 5:26 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 24194: Fixed bug in io::poll.

2014-08-13 Thread Benjamin Hindman
/#review49473 --- On Aug. 1, 2014, 8:05 p.m., Benjamin Hindman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24194

Review Request 24673: Used new CommandInfo with Docker::run.

2014-08-13 Thread Benjamin Hindman
Description --- See summary. Diffs - src/docker/docker.cpp 1cba381118c6bd2ac7fcf5a8a229602e2c65c571 Diff: https://reviews.apache.org/r/24673/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 24672: Renamed argv in CommandInfo to arguments.

2014-08-13 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24672/#review50516 --- Ship it! Ship It! - Benjamin Hindman On Aug. 13, 2014, 9:46 p.m

Re: Review Request 24673: Used new CommandInfo with Docker::run.

2014-08-14 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24673/#review50519 --- On Aug. 13, 2014, 9:53 p.m., Benjamin Hindman wrote

Re: Review Request 24673: Used new CommandInfo with Docker::run.

2014-08-14 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24673/#review50520 --- On Aug. 13, 2014, 9:53 p.m., Benjamin Hindman wrote: --- This is an automatically generated

Review Request 24709: Redirect Docker logs.

2014-08-14 Thread Benjamin Hindman
/mesos.cpp 5bd8ba0bb56c9be9d0a3c49c27b6ebc03cfbdf7a Diff: https://reviews.apache.org/r/24709/diff/ Testing --- make check Thanks, Benjamin Hindman

Review Request 24722: Make sure the mesos-fetcher exits if the slave terminates.

2014-08-14 Thread Benjamin Hindman
Thanks, Benjamin Hindman

Re: Review Request 24722: Make sure the mesos-fetcher exits if the slave terminates.

2014-08-14 Thread Benjamin Hindman
/mesos/containerizer.cpp d0676c55e3bb30bcc8e32b27c091080ec58b0c81 Diff: https://reviews.apache.org/r/24722/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 24731: Allow external isolator flag to be backward compatible

2014-08-14 Thread Benjamin Hindman
://reviews.apache.org/r/24731/#comment88570 ExternalContainerizer::create(flags, local) - Benjamin Hindman On Aug. 15, 2014, 4:04 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 24730: Allow override without CommandInfo value to run in command executor

2014-08-15 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24730/#review50788 --- Ship it! Ship It! - Benjamin Hindman On Aug. 15, 2014, 9:50 p.m

Re: Review Request 24722: Make sure the mesos-fetcher exits if the slave terminates.

2014-08-15 Thread Benjamin Hindman
/ Testing --- make check Thanks, Benjamin Hindman

Review Request 24755: Improved safety of higher-level io::read/write.

2014-08-15 Thread Benjamin Hindman
/subprocess_tests.cpp 98a4e44bd50cd70c908846b71272525e8b5ce406 Diff: https://reviews.apache.org/r/24755/diff/ Testing --- make check Thanks, Benjamin Hindman

Review Request 24757: Cleaned up usage of io::read for new semantics.

2014-08-15 Thread Benjamin Hindman
Description --- This is an incomplete cleanup. Diffs - src/docker/docker.cpp 8f04babb9cd06ff5b2a39033664326d7d44cd6c6 Diff: https://reviews.apache.org/r/24757/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 24754: Introduced states for Docker containers to transition between.

2014-08-15 Thread Benjamin Hindman
e0fd62f83387635f503817ced7a592cc3ae6e775 Diff: https://reviews.apache.org/r/24754/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 24722: Make sure the mesos-fetcher exits if the slave terminates.

2014-08-15 Thread Benjamin Hindman
/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 24730: Allow override without CommandInfo value to run in command executor

2014-08-15 Thread Benjamin Hindman
: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24730/ --- (Updated Aug. 15, 2014, 9:50 p.m.) Review request for mesos, Benjamin Hindman and Jie Yu. Repository: mesos-git Description

Re: Review Request 24730: Allow override without CommandInfo value to run in command executor

2014-08-15 Thread Benjamin Hindman
comments to call out this semantics explicitly. Timothy Chen wrote: We plan to add more documentation around DockerInfo once all our work are in there :) Benjamin Hindman wrote: Yes, definitely Jie! I added more comments to the test, will update the diff. Oh wait, this isn't my

Re: Review Request 24754: Introduced states for Docker containers to transition between.

2014-08-15 Thread Benjamin Hindman
e0fd62f83387635f503817ced7a592cc3ae6e775 Diff: https://reviews.apache.org/r/24754/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 24754: Introduced states for Docker containers to transition between.

2014-08-15 Thread Benjamin Hindman
/docker_containerizer_tests.cpp e0fd62f83387635f503817ced7a592cc3ae6e775 Diff: https://reviews.apache.org/r/24754/diff/ Testing --- make check Thanks, Benjamin Hindman

Review Request 24761: Made DockerContainerizer be the default.

2014-08-15 Thread Benjamin Hindman
. Repository: mesos-git Description --- See summary. Diffs - src/slave/flags.hpp a4ddeb124f155ecdcfbd7a5b16f6ac51d9d0be37 Diff: https://reviews.apache.org/r/24761/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 24722: Make sure the mesos-fetcher exits if the slave terminates.

2014-08-15 Thread Benjamin Hindman
with the environment variable LIBPROCESS_PORT will cause the mesos-fetcher to fail to start because it also tries to bind to that port. - Benjamin Hindman On Aug. 15, 2014, 11:43 p.m., Benjamin Hindman wrote: --- This is an automatically generated e

Review Request 24765: Added some slave recovery DockerContainerizer tests.

2014-08-15 Thread Benjamin Hindman
Description --- See summary. Diffs - src/tests/docker_containerizer_tests.cpp 0d7c3b188319768227797d680b0ee8d5f764de10 Diff: https://reviews.apache.org/r/24765/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 24722: Make sure the mesos-fetcher exits if the slave terminates.

2014-08-15 Thread Benjamin Hindman
On Aug. 16, 2014, 2:52 a.m., Benjamin Hindman wrote: Re-opened for review as a slave that was launched with the environment variable LIBPROCESS_PORT will cause the mesos-fetcher to fail to start because it also tries to bind to that port. Jie Yu wrote: hum, does that even affect

Review Request 24766: Set ownership of stdout/stderr and container directory properly.

2014-08-15 Thread Benjamin Hindman
Description --- See summary. Diffs - src/slave/containerizer/docker.cpp ced0f9247ccb8d1419e464f5c0730b2796ece8e7 Diff: https://reviews.apache.org/r/24766/diff/ Testing --- make check Thanks, Benjamin Hindman

Review Request 24767: Validate Docker version since we require = 1.0.0.

2014-08-15 Thread Benjamin Hindman
Description --- See summary. Diffs - src/docker/docker.cpp a4bad5b41765fdbcc9a748d5a3153700fbc1fc80 Diff: https://reviews.apache.org/r/24767/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 24767: Validate Docker version since we require = 1.0.0.

2014-08-15 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24767/#review50816 --- On Aug. 16, 2014, 5:03 a.m., Benjamin Hindman wrote

Re: Review Request 24768: Save docker pid for subsequent containerizer updates

2014-08-16 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24768/#review50821 --- Ship it! Ship It! - Benjamin Hindman On Aug. 16, 2014, 6:17 a.m

Re: Review Request 24761: Made DockerContainerizer be the default.

2014-08-16 Thread Benjamin Hindman
/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 24765: Added some slave recovery DockerContainerizer tests.

2014-08-16 Thread Benjamin Hindman
and Timothy Chen. Repository: mesos-git Description --- See summary. Diffs (updated) - src/tests/docker_containerizer_tests.cpp 0d7c3b188319768227797d680b0ee8d5f764de10 Diff: https://reviews.apache.org/r/24765/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 24767: Validate Docker version since we require = 1.0.0.

2014-08-16 Thread Benjamin Hindman
and Timothy Chen. Repository: mesos-git Description --- See summary. Diffs (updated) - src/docker/docker.cpp a4bad5b41765fdbcc9a748d5a3153700fbc1fc80 Diff: https://reviews.apache.org/r/24767/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 24772: Fixed libprocess tests due to the new io::read semantics.

2014-08-16 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24772/#review50828 --- Ship it! Thanks Jie. :-/ - Benjamin Hindman On Aug. 16, 2014, 5

Re: Review Request 24766: Set ownership of stdout/stderr and container directory properly.

2014-08-16 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24766/#review50823 --- On Aug. 16, 2014, 4:36 a.m., Benjamin Hindman wrote

Re: Review Request 24773: Fix style issues around docker

2014-08-16 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24773/#review50830 --- Ship it! Ship It! - Benjamin Hindman On Aug. 16, 2014, 5:49 p.m

Re: Review Request 24774: Cleanup mesos due to the new io::read semantics.

2014-08-16 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24774/#review50831 --- Ship it! Ship It! - Benjamin Hindman On Aug. 16, 2014, 6:41 p.m

Re: Review Request 24765: Added some slave recovery DockerContainerizer tests.

2014-08-16 Thread Benjamin Hindman
. Thank you! - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24765/#review50822 --- On Aug. 16, 2014, 2:57 p.m., Benjamin Hindman

Re: Review Request 24781: Updated the upgrades.md for 0.20.0.

2014-08-16 Thread Benjamin Hindman
Ship It! On Saturday, August 16, 2014, Jie Yu yujie@gmail.com wrote: This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24781/ Review request for mesos, Benjamin Hindman and Vinod Kone. By Jie Yu. *Repository: * mesos-git Description See

Re: Review Request 25030: Replaced PKG_CHECK_MODULES with AC_CHECKs in configure.ac.

2014-08-25 Thread Benjamin Hindman
/#comment89738 Is there one that is? - Benjamin Hindman On Aug. 25, 2014, 6:32 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25030

Re: Review Request 25031: Replaced PKG_CHECK_MODULES with AC_CHECKs in configure.ac.

2014-08-25 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25031/#review51422 --- Ship it! Same comment re: '_init' symbol. - Benjamin Hindman

Re: Review Request 25105: Explore disk io isolation in cgroups

2014-08-27 Thread Benjamin Hindman
it. The network isolator is a bit more special case because it does a lot more. - Benjamin Hindman On Aug. 27, 2014, 5:59 p.m., Patrick Reilly wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r

Re: Review Request 25116: Fix External Containerizer creation

2014-08-27 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25116/#review51722 --- Ship it! Ship It! - Benjamin Hindman On Aug. 27, 2014, 8:46 p.m

Review Request 25218: Updated a comment and some syntax formatting.

2014-08-31 Thread Benjamin Hindman
2d13f2ac4184bf07285814ca5809406985da8027 configure.ac c4b43911f5f8f651ddf8f2e12c263849e07e8089 Diff: https://reviews.apache.org/r/25218/diff/ Testing --- Thanks, Benjamin Hindman

Re: Mesos 0.20.0 blog post

2014-09-02 Thread Benjamin Hindman
LGTM! On Tuesday, September 2, 2014, Jie Yu yujie@gmail.com wrote: Hi, I've drafted the blog post for the 0.20.0 release (the link below). Please let me know if you have any suggestion or comments. The plan is to publish it tomorrow.

Re: Docker Containerization

2014-09-05 Thread Benjamin Hindman
I appreciate the thoughtful email Tom! There's a bunch there so I've just made some inline comments addressing your final questions. ;-) - Can the docker containerizer support more friendly defaults? If I only want my mesos cluster to containerizer things with Docker, but don't wish to require

Re: Review Request 25434: Propagate slave shutdown grace period to Executor and CommandExecutor.

2014-09-09 Thread Benjamin Hindman
/25434/#comment91755 Why not call it 'value' like we do in the code above? src/slave/constants.hpp https://reviews.apache.org/r/25434/#comment91771 What is the 'base executor' versus the 'command executor'? - Benjamin Hindman On Sept. 9, 2014, 12:54 p.m., Alexander Rukletsov wrote

Re: Review Request 25434: Propagate slave shutdown grace period to Executor and CommandExecutor.

2014-09-09 Thread Benjamin Hindman
On Sept. 9, 2014, 4:21 p.m., Timothy Chen wrote: src/launcher/executor.cpp, line 682 https://reviews.apache.org/r/25434/diff/2/?file=683946#file683946line682 I'm not sure I'm really following the logic, I know the levels but I don't know why there is the divide by 2/3 on each

Re: Review Request 25434: Propagate slave shutdown grace period to Executor and CommandExecutor.

2014-09-09 Thread Benjamin Hindman
On Sept. 9, 2014, 4:39 p.m., Niklas Nielsen wrote: src/tests/containerizer.cpp, line 113 https://reviews.apache.org/r/25434/diff/2/?file=683955#file683955line113 Why 3 seconds? Alexander Rukletsov wrote: Though default is 5, I decide to set it to 3 (which is the lower

Re: Review Request 24776: Add docker containerizer destroy tests

2014-09-09 Thread Benjamin Hindman
/#comment91798 Looks like wierd indentation here. - Benjamin Hindman On Aug. 16, 2014, 10:23 p.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24776

Re: Review Request 25434: Propagate slave shutdown grace period to Executor and CommandExecutor.

2014-09-09 Thread Benjamin Hindman
On Sept. 9, 2014, 5:50 p.m., Benjamin Hindman wrote: src/slave/constants.hpp, line 53 https://reviews.apache.org/r/25434/diff/2/?file=683947#file683947line53 What is the 'base executor' versus the 'command executor'? Alexander Rukletsov wrote: We have Executor (lives in src

Re: Review Request 25237: Avoid Docker pull on each run

2014-09-09 Thread Benjamin Hindman
, 2014, 7:16 p.m.) Review request for mesos, Benjamin Hindman and Jie Yu. Repository: mesos-git Description --- Avoid Docker pull on each run. Currently each Docker run will run a docker pull which calls the docker registry each time. To avoid this this patch adds

Re: Review Request 25270: Enable bridge network in Mesos

2014-09-09 Thread Benjamin Hindman
to send to a mapped port and have it show up on a command listening on the internal port? Is there 'nc' or 'telnet' in busybox that you can assume to get the output of any data sent to the internal port? - Benjamin Hindman On Sept. 5, 2014, 4:55 p.m., Timothy Chen wrote

Re: Review Request 25403: Override entrypoint when shell enabled in Docker

2014-09-09 Thread Benjamin Hindman
/#comment91836 Why not move this up above as well? - Benjamin Hindman On Sept. 5, 2014, 10:13 p.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25403

Re: Review Request 25434: Propagate slave shutdown grace period to Executor and CommandExecutor.

2014-09-10 Thread Benjamin Hindman
On Sept. 9, 2014, 5:50 p.m., Benjamin Hindman wrote: src/slave/constants.hpp, line 53 https://reviews.apache.org/r/25434/diff/2/?file=683947#file683947line53 What is the 'base executor' versus the 'command executor'? Alexander Rukletsov wrote: We have Executor (lives in src

Re: Review Request 25523: Add Docker pull to docker abstraction

2014-09-16 Thread Benjamin Hindman
version that I had done before 0.20.0. - Benjamin Hindman On Sept. 11, 2014, 6:44 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25523

Re: Review Request 25270: Enable bridge network in Mesos

2014-09-16 Thread Benjamin Hindman
this test is going to start non-deterministically failing. - Benjamin Hindman On Sept. 11, 2014, 5:48 p.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25270

Re: Review Request 25403: Override entrypoint when shell enabled in Docker

2014-09-16 Thread Benjamin Hindman
On Sept. 9, 2014, 6:50 p.m., Benjamin Hindman wrote: src/docker/docker.cpp, line 337 https://reviews.apache.org/r/25403/diff/1/?file=680701#file680701line337 Why not move this up above as well? Timothy Chen wrote: The Docker cli --entrypoint only allows you to put

Re: Review Request 25403: Override entrypoint when shell enabled in Docker

2014-09-16 Thread Benjamin Hindman
a comment above argv.push_back(-c) so that it's clear. Thanks Tim! - Benjamin Hindman On Sept. 11, 2014, 12:40 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25403

Re: Review Request 24776: Add docker containerizer destroy tests

2014-09-16 Thread Benjamin Hindman
On Sept. 9, 2014, 6:15 p.m., Benjamin Hindman wrote: Why did you need to mock DockerContainerizerProcess in order to write these tests? Couldn't you have just used the existing MockDockerContainerizer? Timothy Chen wrote: I wanted to simulate having destroy called in a pull

Re: Review Request 25523: Add Docker pull to docker abstraction

2014-09-17 Thread Benjamin Hindman
/docker/docker.cpp https://reviews.apache.org/r/25523/#comment93430 Swap these two please. src/slave/containerizer/docker.cpp https://reviews.apache.org/r/25523/#comment93414 Comment is out of date now. lose - Benjamin Hindman On Sept. 17, 2014, 1:07 a.m., Timothy Chen wrote

Re: Review Request 25523: Add Docker pull to docker abstraction

2014-09-17 Thread Benjamin Hindman
. Perhaps the important thing for me to say is that just because we're calling 'future.discard()' does not mean that the future will get discarded, hence AWAIT_DISCARDED(future) is sufficient. Minor cleanup/simplification in the test then lets get this committed! - Benjamin Hindman On Sept

Re: Review Request 25758: Modify docker pull to call inspect after pull

2014-09-18 Thread Benjamin Hindman
a comment is definitely in order. - Benjamin Hindman On Sept. 18, 2014, 6:04 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25758

Re: [VOTE] Release Apache Mesos 0.20.1 (rc2)

2014-09-18 Thread Benjamin Hindman
I've committed Tim's fix, we can cut another release candidate and restart the vote. On Wed, Sep 17, 2014 at 11:07 PM, Tim Chen t...@mesosphere.io wrote: -1 The docker test failed when I removed the image, and found a problem from the docker pull implementation. I've created a reviewboard

Re: Review Request 25789: Variadic strings join for c++11 and above

2014-09-19 Thread Benjamin Hindman
on their own line, thanks! - Benjamin Hindman On Sept. 19, 2014, 12:36 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25789

Re: Review Request 25798: Refactor Libprocess Help to reduce compile time.

2014-09-19 Thread Benjamin Hindman
://reviews.apache.org/r/25798/#comment93832 Do we need the stringstream at all? Could we simplify these with: return strings::join(\n, std::forwardT(args)..., \n); - Benjamin Hindman On Sept. 19, 2014, 12:26 a.m., Joris Van Remoortere wrote

Re: Mesos Modules Design

2014-09-22 Thread Benjamin Hindman
Jumping on a tcon/hangout sounds healthy, but given I'm traveling right now in Europe and timing is difficult I'm going to comment inline here. - callsites need to be modules aware to use the right factory method to instantiate the modular object I don't know how else you'd accomplish making

Re: Mesos Modules Design

2014-09-23 Thread Benjamin Hindman
- create abstract classes to define interfaces to objects that should be modular We're all in agreement here! - build modules as static libraries that can be assembled at link time to create custom Mesos builds Okay, but unless I'm missing something here we'll still need a level of

Re: Auto usage in mesos

2014-09-25 Thread Benjamin Hindman
+1 to growing the list of use cases for 'auto' organically. I agree with Cody that using 'auto' for return type deduction should also be included, especially since we're already using it in libprocess. On Mon, Sep 22, 2014 at 10:50 AM, Cody Maloney c...@mesosphere.io wrote: There are some

Re: Review Request 25622: Update the Mesos Style Guide with C++11 and naming notes.

2014-09-25 Thread Benjamin Hindman
/ --- (Updated Sept. 22, 2014, 1:10 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Till Toenshoff. Bugs: MESOS-1793 https://issues.apache.org/jira/browse/MESOS-1793 Repository: mesos

Re: Review Request 25789: Variadic strings join

2014-09-25 Thread Benjamin Hindman
/#comment94721 Silly style thing, but in this case both arguments should be indented the same. - Benjamin Hindman On Sept. 25, 2014, 5:10 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 24535: Added functionality to create SVN based diffs of arbitrary strings.

2014-09-29 Thread Benjamin Hindman
bd1dc8df0259a318a9171a9c045a223800e64f47 3rdparty/libprocess/3rdparty/stout/include/stout/svn.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/tests/svn_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/24535/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 24341: Added Java replicated log implementation of State.

2014-09-29 Thread Benjamin Hindman
/LogState.java PRE-CREATION Diff: https://reviews.apache.org/r/24341/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 24537: Updated metrics::Timer::stop to return elapsed time.

2014-09-29 Thread Benjamin Hindman
, Benjamin Hindman

Re: Review Request 24341: Added Java replicated log implementation of State.

2014-09-29 Thread Benjamin Hindman
://reviews.apache.org/r/24341/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 24535: Added functionality to create SVN based diffs of arbitrary strings.

2014-09-29 Thread Benjamin Hindman
/3rdparty/stout/include/stout/svn.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/tests/svn_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/24535/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 24536: Added DIFF to the replicated log state storage implementation.

2014-09-29 Thread Benjamin Hindman
59276e55fcbebdb754c20d39b13b402fd11c3dad src/state/log.cpp fd8b28a0b5d14f5ba3e6fde4695f6d09acf9c56a Diff: https://reviews.apache.org/r/24536/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Auto usage in mesos

2014-09-29 Thread Benjamin Hindman
(Perhaps you're thinking about: auto f(...) - return_type { ... }) Yes, I was referring to the use of auto as the return type in the function declaration, not (yet) for automatic deduction. Thanks for the clarification.

Re: Review Request 25848: Introducing mesos modules.

2014-10-01 Thread Benjamin Hindman
a comment saying as much). - Benjamin Hindman On Sept. 30, 2014, 11:01 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848

Re: Review Request 26214: Add apr and svn to configure.ac

2014-10-06 Thread Benjamin Hindman
: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26214/ --- (Updated Oct. 2, 2014, 7:06 p.m.) Review request for mesos and Benjamin Hindman. Repository: mesos-git

Re: Review Request 26214: Add apr and svn to configure.ac

2014-10-06 Thread Benjamin Hindman
, visit: https://reviews.apache.org/r/26214/ --- (Updated Oct. 2, 2014, 7:06 p.m.) Review request for mesos and Benjamin Hindman. Repository: mesos-git Description --- Able to build with OSX and Ubuntu 14.04

Re: Review Request 26214: Add apr and svn to configure.ac

2014-10-06 Thread Benjamin Hindman
/#comment95907 Let's avoid the extra APR_CFLAGS variable and just set CPPFLAGS in both the branches. Same below with SVN_CFLAGS (and in the libprocess configure.ac too). - Benjamin Hindman On Oct. 2, 2014, 7:06 p.m., Timothy Chen wrote

Re: Review Request 26352: Add --with-curl to libprocess since stout/net.hpp needs it.

2014-10-06 Thread Benjamin Hindman
On Oct. 6, 2014, 1:30 p.m., Till Toenshoff wrote: 3rdparty/libprocess/configure.ac, line 99 https://reviews.apache.org/r/26352/diff/1/?file=714029#file714029line99 Is there a reason you are not adhering to that without_bundled_xxx scheme we are using for other bundled libraries?

Re: Review Request 26247: Remove usage of stout/preprocessor.hpp from mesos.

2014-10-06 Thread Benjamin Hindman
On Oct. 1, 2014, 9:30 p.m., Dominic Hamon wrote: src/slave/status_update_manager.cpp, line 570 https://reviews.apache.org/r/26247/diff/1/?file=710348#file710348line570 this is an unfortunate pattern. I assume your changes in dispatch have somehow changed the template deduction so

Re: Review Request 25848: Introducing mesos modules.

2014-10-06 Thread Benjamin Hindman
see Bernd commented that the use of these functions was hard to track down, so why not eliminate that function all together! ;-) - Benjamin Hindman On Oct. 3, 2014, 11:46 p.m., Kapil Arya wrote: --- This is an automatically generated e

Re: Review Request 25848: Introducing mesos modules.

2014-10-07 Thread Benjamin Hindman
/ --- (Updated Oct. 7, 2014, 11:07 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and Timothy St. Clair. Bugs: MESOS-1384 https://issues.apache.org/jira/browse/MESOS-1384 Repository: mesos-git Description --- Adding a first class

Re: Review Request 25848: Introducing mesos modules.

2014-10-08 Thread Benjamin Hindman
changes together and this is looking good to go! Will commit now! ;-) - Benjamin Hindman On Oct. 8, 2014, 8:52 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848

Re: Review Request 26533: Memory cleanup: libprocess finalize

2014-10-10 Thread Benjamin Hindman
://reviews.apache.org/r/26533/#comment96515 We've used s/ret/result/ in the code base, let's stay consistent please. - Benjamin Hindman On Oct. 10, 2014, 1:02 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail

Re: Review Request 26487: Perform read right after subprocess for docker ps

2014-10-11 Thread Benjamin Hindman
putting the .then on the next line to make it even easier to read? This is also the prevailing style (see the rest of the .then's in this file, for example). - Benjamin Hindman On Oct. 9, 2014, 9:38 p.m., Timothy Chen wrote

Review Request 26612: Introduced DockerContainerizerProcess::Container::create.

2014-10-11 Thread Benjamin Hindman
--- See summary. Diffs - src/slave/containerizer/docker.cpp 9a2948951f57f3ab16291df51cd9f33e5e96add4 Diff: https://reviews.apache.org/r/26612/diff/ Testing --- make check Thanks, Benjamin Hindman

Review Request 26613: Simplified launch continuations in DockerContainerizerProcess.

2014-10-11 Thread Benjamin Hindman
--- See summary. Diffs - src/slave/containerizer/docker.cpp 9a2948951f57f3ab16291df51cd9f33e5e96add4 Diff: https://reviews.apache.org/r/26613/diff/ Testing --- make check Thanks, Benjamin Hindman

Review Request 26614: More simplifications of Docker container launching.

2014-10-11 Thread Benjamin Hindman
--- See summary. Diffs - src/slave/containerizer/docker.cpp 9a2948951f57f3ab16291df51cd9f33e5e96add4 Diff: https://reviews.apache.org/r/26614/diff/ Testing --- make check Thanks, Benjamin Hindman

Review Request 26610: Consolidated launch code paths in Docker containerizer.

2014-10-11 Thread Benjamin Hindman
/26610/diff/ Testing --- make check Thanks, Benjamin Hindman

Re: Review Request 26436: Avoid docker inspect on each usage call

2014-10-11 Thread Benjamin Hindman
, 3:56 p.m.) Review request for mesos and Benjamin Hindman. Repository: mesos-git Description --- Review: https://reviews.apache.org/r/26436 Diffs - src/slave/containerizer/docker.cpp 9a2948951f57f3ab16291df51cd9f33e5e96add4 Diff: https://reviews.apache.org/r

  1   2   3   4   5   6   7   8   9   10   >