Re: Review Request 33296: Added a flag which controls libprocess firewall initialzation.

2015-04-23 Thread Till Toenshoff
://reviews.apache.org/r/33296/#comment131638 s/FirewallRule/FirewallRules/ src/master/main.cpp https://reviews.apache.org/r/33296/#comment131639 - Till Toenshoff On April 22, 2015, 2:36 p.m., Alexander Rojas wrote

Re: Review Request 32001: Required a period in trailing comments in the style guide.

2015-04-26 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32001/#review81650 --- Ship it! Ship It! - Till Toenshoff On April 24, 2015, 1:53 p.m

Re: Review Request 33263: Extended SlaveTest.ShutdownUnregisteredExecutor test with a reason check.

2015-04-26 Thread Till Toenshoff
On April 23, 2015, 4:28 p.m., Till Toenshoff wrote: Ship It! When testing your patch, I noticed that it fails for me. ``` ../../src/tests/slave_tests.cpp:184: Failure Value of: status.get().reason() Actual: 8 Expected: TaskStatus::REASON_COMMAND_EXECUTOR_FAILED Which is: 0 ``` - Till

Re: Review Request 33271: Update style guide to disallow capturing temporaries by reference.

2015-04-29 Thread Till Toenshoff
/r/33271/#comment132649 Why not using the ``` code-marks instead of the pre tag? The former triggers nice syntax highlighting when viewed with a proper markdown-viewer. - Till Toenshoff On April 22, 2015, 6:36 p.m., Joris Van Remoortere wrote

Re: Review Request 32198: Added a not equal operator for json objects.

2015-04-29 Thread Till Toenshoff
, and Till Toenshoff. Bugs: MESOS-2510 https://issues.apache.org/jira/browse/MESOS-2510 Repository: mesos Description --- For consistency, adds a non equal operator to the json objects. It also adds tests to the equality operators. Diffs - 3rdparty

Re: Review Request 33718: Extended documentation on Mesos hooks.

2015-05-08 Thread Till Toenshoff
: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33718/ --- (Updated May 7, 2015, 8:40 p.m.) Review request for mesos, Adam B, Kapil Arya, Niklas Nielsen, and Till

Re: Review Request 33718: Extended documentation on Mesos hooks.

2015-05-08 Thread Till Toenshoff
://reviews.apache.org/r/33718/#comment133942 s/5050/PORT/ s/--work_dir=s\/work// - Till Toenshoff On May 7, 2015, 8:40 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 33733: Fixed style error with template definitions withing common/parse.hpp

2015-05-05 Thread Till Toenshoff
the styleguide for this? Thanks a bunch :) - Till Toenshoff On April 30, 2015, 10:45 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33733

Re: Review Request 30612: Added /master/frameworks/{framework}/tasks/{task} endpoint.

2015-05-18 Thread Till Toenshoff
/#comment133799 `const std::string`? - Till Toenshoff On May 18, 2015, 12:56 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30612

Re: Review Request 34295: Added maintainers documentation.

2015-05-17 Thread Till Toenshoff
On May 17, 2015, 11:13 p.m., Till Toenshoff wrote: Shit It - after addressing Adam's great comments. Thanks Ben! hmpft - sry for that non freudian typo. - Till --- This is an automatically generated e-mail. To reply, visit

Re: Review Request 34295: Added maintainers documentation.

2015-05-17 Thread Till Toenshoff
Ben! docs/committers.md https://reviews.apache.org/r/34295/#comment135196 s/Libraires/Libraries/ - Till Toenshoff On May 15, 2015, 10:25 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit

Re: Review Request 34260: Replaced os::dirname and os::basename with Path::dirname and Path::basename.

2015-05-17 Thread Till Toenshoff
Thanks, Till Toenshoff

Re: Review Request 34260: Replaced os::dirname and os::basename with Path::dirname and Path::basename.

2015-05-17 Thread Till Toenshoff
Thanks, Till Toenshoff

Re: Review Request 30032: Added support for cache control in libprocess when dealing with static files.

2015-05-18 Thread Till Toenshoff
/ --- (Updated April 20, 2015, 11:58 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Joerg Schad, Michael Park, and Till Toenshoff. Bugs: mesos-708 https://issues.apache.org/jira/browse/mesos-708 Repository: mesos

Re: Review Request 34375: Removed use of namespace aliases.

2015-05-18 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34375/#review84238 --- Ship it! Ship It! - Till Toenshoff On May 18, 2015, 11:13 p.m

Re: Review Request 34378: Fixed the dependency between 'summarize' and 'model'.

2015-05-18 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34378/#review84241 --- Ship it! Ship It! - Till Toenshoff On May 18, 2015, 11:23 p.m

Re: Review Request 34268: stout library - adding support for Solaris

2015-05-18 Thread Till Toenshoff
, please. 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp https://reviews.apache.org/r/34268/#comment135374 We do not commonly comment the `#endif` of a `#if define()` ``` #endif ``` Looking forward to give your updated patch another review, thanks again. - Till

Re: Review Request 34260: Replaced os::dirname and os::basename with Path::dirname and Path::basename.

2015-05-17 Thread Till Toenshoff
--- make check Thanks, Till Toenshoff

Re: Review Request 34260: Replaced os::dirname and os::basename with Path::dirname and Path::basename.

2015-05-17 Thread Till Toenshoff
/ Testing --- make check Thanks, Till Toenshoff

Re: Review Request 34268: stout library - adding support for Solaris

2015-06-02 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34268/#review86172 --- Ship it! Ship It! - Till Toenshoff On May 22, 2015, 7:15 p.m

Re: Review Request 35000: Doxygen'ized Subprocess.

2015-06-05 Thread Till Toenshoff
the colon and hence the following enumerations could use capitalized sentence starts. I would replace the colon with a period to enable full sentences below. - Till Toenshoff On June 3, 2015, 1:45 p.m., Benjamin Hindman wrote

Re: Review Request 34256: Added Path::dirname() and Path::basename().

2015-06-05 Thread Till Toenshoff
check (including new tests). Result comparison to match ::dirname and ::basename on interesting cases. Thanks, Till Toenshoff

Re: Review Request 34259: Replaced os::dirname and os::basename with Path::dirname and Path::basename.

2015-06-05 Thread Till Toenshoff
--- make check on trailing RR. Thanks, Till Toenshoff

Re: Review Request 34256: Added Path::dirname() and Path::basename().

2015-06-05 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34256/#review84375 --- On May 17, 2015, 7:46 p.m., Till Toenshoff wrote

Review Request 35131: Replaced os::dirname and os::basename with Path::dirname and Path::basename.

2015-06-05 Thread Till Toenshoff
, Till Toenshoff

Re: Review Request 34260: Replaced os::dirname and os::basename with Path::dirname and Path::basename.

2015-06-05 Thread Till Toenshoff
/fetcher_tests.cpp 361d918 src/tests/mesos.cpp d3a8bb7 src/zookeeper/group.cpp 173caa8 Diff: https://reviews.apache.org/r/34260/diff/ Testing --- make check Thanks, Till Toenshoff

Re: Review Request 34258: Removed os::dirname and os::basename.

2015-06-08 Thread Till Toenshoff
) - 3rdparty/libprocess/3rdparty/stout/README.md 588f739 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp d2ca4be Diff: https://reviews.apache.org/r/34258/diff/ Testing --- make check on trailing RR. Thanks, Till Toenshoff

Re: Review Request 35234: libprocess: consistent handling of --enable options

2015-06-08 Thread Till Toenshoff
/configure.ac https://reviews.apache.org/r/35234/#comment139408 Please end comments with a punctuation. - Till Toenshoff On June 9, 2015, 12:04 a.m., James Peach wrote: --- This is an automatically generated e-mail. To reply, visit

Re: Review Request 34258: Removed os::dirname and os::basename.

2015-06-05 Thread Till Toenshoff
) - 3rdparty/libprocess/3rdparty/stout/README.md 588f739 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp d2ca4be Diff: https://reviews.apache.org/r/34258/diff/ Testing --- make check on trailing RR. Thanks, Till Toenshoff

Re: Review Request 33057: Added secret check to CRAM-MD5 authenticatee.

2015-06-08 Thread Till Toenshoff
when secret is missing. Diffs (updated) - src/authentication/cram_md5/authenticatee.cpp PRE-CREATION src/tests/cram_md5_authentication_tests.cpp 9923023 Diff: https://reviews.apache.org/r/33057/diff/ Testing --- make check Thanks, Till Toenshoff

Re: Review Request 34703: Added stream manipulators for the Time object.

2015-06-08 Thread Till Toenshoff
to temporals. 3rdparty/libprocess/include/process/time.hpp https://reviews.apache.org/r/34703/#comment139288 `const Time time;` - Till Toenshoff On May 29, 2015, 12:59 p.m., Alexander Rojas wrote: --- This is an automatically generated

Re: Review Request 34703: Added stream manipulators for the Time object.

2015-06-08 Thread Till Toenshoff
for that class name to make it intuitive - first thing that came to mind is ```UnixTimestamp```. Lets also add an example right here in this comment. 3rdparty/libprocess/include/process/time.hpp https://reviews.apache.org/r/34703/#comment139276 2 chars intention please. - Till Toenshoff

Re: Review Request 33331: Added file headers section to the C++ style guide.

2015-06-04 Thread Till Toenshoff
--- rebased Repository: mesos-incubating Description --- see summary. Diffs (updated) - docs/mesos-c++-style-guide.md 5dcbdad Diff: https://reviews.apache.org/r/1/diff/ Testing --- N/A Thanks, Till Toenshoff

Re: Review Request 35075: Fixed style issues in the File Header section in the C++ style-guide.

2015-06-04 Thread Till Toenshoff
On June 4, 2015, 3:10 p.m., Till Toenshoff wrote: Ship It! Thanks Joerg! - Till --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35075/#review86626

Re: Review Request 34260: Replaced os::dirname and os::basename with Path::dirname and Path::basename.

2015-06-04 Thread Till Toenshoff
--- On May 17, 2015, 10:42 p.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34260

Re: Review Request 35075: Fixed style issues in the File Header section in the C++ style-guide.

2015-06-04 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35075/#review86626 --- Ship it! Ship It! - Till Toenshoff On June 4, 2015, 3:08 p.m

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-04 Thread Till Toenshoff
/#comment138629 +1! src/tests/hook_tests.cpp https://reviews.apache.org/r/30339/#comment138628 Could you please explain quickly on why this was ever working / is needed now? - Till Toenshoff On June 1, 2015, 9:46 p.m., Kapil Arya wrote

Re: Review Request 33295: Added firewall mechanism to control access on libprocess http endpoints.

2015-06-05 Thread Till Toenshoff
please adjust the style of this comment towards those above? 3rdparty/libprocess/src/tests/process_tests.cpp https://reviews.apache.org/r/33295/#comment138893 s/a null pointer/an empty vector/ - Till Toenshoff On June 2, 2015, 9:57 a.m., Alexander Rojas wrote

Re: Review Request 34719: Added QOS_KILLED as status reason

2015-06-06 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34719/#review86929 --- Ship it! Ship It! - Till Toenshoff On June 4, 2015, 5:43 p.m

Re: Review Request 35695: stout: Fixed test names.

2015-06-22 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35695/#review88810 --- Ship it! Ship It! - Till Toenshoff On June 20, 2015, 7:14 p.m

Re: Review Request 35697: mesos: Fixed test names.

2015-06-22 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35697/#review88808 --- Ship it! Thanks a bunch for these. - Till Toenshoff On June 20

Re: Review Request 35562: Removed unnecessary use of os::ExecEnv.

2015-06-22 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35562/#review88788 --- Ship it! Ship It! - Till Toenshoff On June 17, 2015, 2:28 p.m

Re: Review Request 35701: Minor formatting cleanup in the Master.

2015-06-22 Thread Till Toenshoff
the jaggedness problem? Would you mind adding a comment to the ticket with your recent findings and experience from those other RRs? - Till Toenshoff On June 20, 2015, 8 p.m., Michael Park wrote: --- This is an automatically

Re: Review Request 35701: Minor formatting cleanup in the Master.

2015-06-22 Thread Till Toenshoff
On June 22, 2015, 6:26 p.m., Till Toenshoff wrote: src/master/master.cpp, lines 972-975 https://reviews.apache.org/r/35701/diff/1/?file=989004#file989004line972 I understand this was updated due to some discussions on related RRs. I would however love to see the definitive answer

Re: Review Request 35701: Minor formatting cleanup in the Master.

2015-06-22 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35701/#review7 --- Ship it! Ship It! - Till Toenshoff On June 20, 2015, 8 p.m

Re: Review Request 35701: Minor formatting cleanup in the Master.

2015-06-22 Thread Till Toenshoff
On June 22, 2015, 6:26 p.m., Till Toenshoff wrote: src/master/master.cpp, lines 972-975 https://reviews.apache.org/r/35701/diff/1/?file=989004#file989004line972 I understand this was updated due to some discussions on related RRs. I would however love to see the definitive answer

Re: Review Request 35567: Added 'executor_environment_variables' flag to slave.

2015-06-22 Thread Till Toenshoff
June 20, 2015, 7 p.m.) Review request for mesos and Till Toenshoff. Bugs: MESOS-2832 https://issues.apache.org/jira/browse/MESOS-2832 Repository: mesos Description --- See summary. Diffs - src/slave/containerizer/containerizer.cpp

Re: Review Request 35799: Support mounting relative paths with docker.

2015-06-23 Thread Till Toenshoff
/#comment141654 I would either get rid of this `containerName` as you only use it once, or I would make it const if you plan to reuse it in later updates. - Till Toenshoff On June 23, 2015, 7:18 p.m., Timothy Chen wrote

Re: Review Request 35863: Fix a typo in docs/docker-containerizer.md file

2015-06-25 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35863/#review89328 --- Ship it! Ship It! - Till Toenshoff On June 25, 2015, 5:47 a.m

Re: Review Request 35567: Added 'executor_environment_variables' flag to slave.

2015-06-24 Thread Till Toenshoff
://reviews.apache.org/r/35567/#comment141867 Test LGTM - glad you came up with this so quick. - Till Toenshoff On June 24, 2015, 10:01 p.m., Benjamin Hindman wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 35848: Fixed typo in getting started

2015-06-24 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35848/#review89274 --- Ship it! Ship It! - Till Toenshoff On June 24, 2015, 10:09 p.m

Re: Review Request 35561: Updated process::subprocess to replace environment.

2015-06-24 Thread Till Toenshoff
://reviews.apache.org/r/35561/#comment141861 Nice little concern break :) - Till Toenshoff On June 18, 2015, 6:48 p.m., Benjamin Hindman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r

Re: Review Request 35353: Smaller fixes on libprocess firewall

2015-06-24 Thread Till Toenshoff
/#comment141766 Bad rebase!? src/slave/containerizer/isolators/network/port_mapping.hpp (lines 68 - 76) https://reviews.apache.org/r/35353/#comment141759 Rebase?! - Till Toenshoff On June 18, 2015, 9:20 a.m., Alexander Rojas wrote

Re: Review Request 33295: Added firewall mechanism to control access on libprocess http endpoints.

2015-06-24 Thread Till Toenshoff
, 2015, 10:09 a.m.) Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff. Bugs: MESOS-2620 https://issues.apache.org/jira/browse/MESOS-2620 Repository: mesos Description --- Introduces the interface `FirewallRule` which

Re: Review Request 35354: Smaller fixes in libprocess firewall initialization

2015-06-24 Thread Till Toenshoff
/r/35354/#comment141767 Add a `using process::Owned` please. I really wonder which header is doing this so it compiles without the `using`. - Till Toenshoff On June 15, 2015, 9:24 a.m., Alexander Rojas wrote

Re: Review Request 35743: flags: fixed const'ness of load

2015-06-24 Thread Till Toenshoff
(lines 33 - 39) https://reviews.apache.org/r/35743/#comment141877 Should live somewhere else? ... how about stout/utils.hpp? - Till Toenshoff On June 23, 2015, 9:51 p.m., Jojy Varghese wrote: --- This is an automatically generated

Re: Review Request 35286: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (libprocess)

2015-06-24 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35286/#review89280 --- Ship it! Ship It! - Till Toenshoff On June 20, 2015, 4:47 p.m

Re: Review Request 35287: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout)

2015-06-24 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35287/#review89279 --- Ship it! Ship It! - Till Toenshoff On June 20, 2015, 4:48 p.m

Re: Review Request 35765: Updated network-monitoring docs to reflect correct libnl version.

2015-06-22 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35765/#review88912 --- Ship it! Ship It! - Till Toenshoff On June 23, 2015, 3:09 a.m

Re: Review Request 35762: Updated libnl configure check to reflect correct requirements.

2015-06-22 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35762/#review88906 --- Ship it! Ship It! - Till Toenshoff On June 23, 2015, 2:41 a.m

Re: Review Request 35762: Updated libnl configure check to reflect correct requirements.

2015-06-22 Thread Till Toenshoff
On June 23, 2015, 2:47 a.m., Till Toenshoff wrote: Ship It! As discussed, another RR updating docs/network-monitoring.md would be great to keep things in sync. - Till --- This is an automatically generated e-mail. To reply, visit

Re: Review Request 35694: Added helper constructors to hashmap.

2015-06-22 Thread Till Toenshoff
/ --- (Updated June 20, 2015, 7:18 p.m.) Review request for mesos and Till Toenshoff. Bugs: MESOS-2832 https://issues.apache.org/jira/browse/MESOS-2832 Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess

Re: Review Request 35694: Added helper constructors to hashmap.

2015-06-22 Thread Till Toenshoff
/hashmap.hpp (line 59) https://reviews.apache.org/r/35694/#comment141358 s/from a/from an/ - Till Toenshoff On June 20, 2015, 7:18 p.m., Benjamin Hindman wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 35771: Fixed post-reviews.py hanging bug.

2015-06-23 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35771/#review88934 --- lgtm - thanks for the fix, much appreciated. - Till Toenshoff

Re: Review Request 35772: Made post-reviews.py print parent diff only if one exists.

2015-06-23 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35772/#review88935 --- Ship it! Ship It! - Till Toenshoff On June 23, 2015, 7:21 a.m

Re: Review Request 35773: Removed libgen.h include.

2015-06-23 Thread Till Toenshoff
: https://reviews.apache.org/r/35773/diff/ Testing --- make check on trailing RR. Thanks, Till Toenshoff

Re: Review Request 34260: Replaced os::dirname and os::basename with Path::dirname and Path::basename.

2015-06-23 Thread Till Toenshoff
361d918db183d31ef00e414e8b991846a9172be8 src/tests/mesos.cpp 2cd2435eba4f911867e7e09338aa28e65b2d1f14 src/zookeeper/group.cpp 33c56da4c83ed6cb272a69df36e6ed2f83583068 Diff: https://reviews.apache.org/r/34260/diff/ Testing --- make check Thanks, Till Toenshoff

Re: Review Request 34259: Replaced os::dirname and os::basename with Path::dirname and Path::basename.

2015-06-23 Thread Till Toenshoff
/ Testing --- make check on trailing RR. Thanks, Till Toenshoff

Re: Review Request 35131: Replaced os::dirname and os::basename with Path::dirname and Path::basename.

2015-06-23 Thread Till Toenshoff
://reviews.apache.org/r/35131/diff/ Testing --- make check on trailing RR. Thanks, Till Toenshoff

Review Request 35773: Removed libgen.h include.

2015-06-23 Thread Till Toenshoff
on trailing RR. Thanks, Till Toenshoff

Re: Review Request 34256: Added Path::dirname() and Path::basename().

2015-06-23 Thread Till Toenshoff
(including new tests). Result comparison to match ::dirname and ::basename on interesting cases. Thanks, Till Toenshoff

Re: Review Request 35438: Fixed fetcher cache test race for resource offers when starting tasks and changed corresponding CHECK to EXPECT.

2015-06-22 Thread Till Toenshoff
://reviews.apache.org/r/35438/#comment141376 s/ +/ +/ src/tests/fetcher_cache_tests.cpp (line 1044) https://reviews.apache.org/r/35438/#comment141375 s/Http/HTTP/g (in comments) - Till Toenshoff On June 22, 2015, 2:30 p.m., Bernd Mathiske wrote

Re: Review Request 35763: Document workaround for LIBNL_CFLAGS path

2015-06-23 Thread Till Toenshoff
On June 23, 2015, 5:39 a.m., Marco Massenzio wrote: Can you please add one of the committers as a reviewer? Did we document such requirement somewhere? - Till --- This is an automatically generated e-mail. To reply, visit:

Re: Review Request 34256: Added Path::dirname() and Path::basename().

2015-06-23 Thread Till Toenshoff
/diff/ Testing --- make check (including new tests). Result comparison to match ::dirname and ::basename on interesting cases. Thanks, Till Toenshoff

Re: Review Request 34256: Added Path::dirname() and Path::basename().

2015-06-23 Thread Till Toenshoff
--- On June 24, 2015, 2:44 a.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34256

Re: Review Request 35752: Added stub Event protobuf handler to scheduler driver.

2015-06-23 Thread Till Toenshoff
On June 23, 2015, 11:31 p.m., Isabel Jimenez wrote: src/sched/sched.cpp, line 438 https://reviews.apache.org/r/35752/diff/1/?file=990748#file990748line438 Aren't we using braces on switch cases syntax? like: ``` switch (type) { case ENUM: { break

Re: Review Request 35568: Remove html from libprocess Developer Guide.

2015-06-23 Thread Till Toenshoff
On June 23, 2015, 11:09 p.m., Till Toenshoff wrote: Ship It! Needs rebase though. - Till --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35568/#review89077

Re: Review Request 34256: Added Path::dirname() and Path::basename().

2015-06-23 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34256/#review88979 --- On June 23, 2015, 8:26 a.m., Till Toenshoff wrote

Re: Review Request 35455: Update 'Getting Started' documentation.

2015-06-15 Thread Till Toenshoff
we are fine with Python 2.7.x on all platforms. Could you please get rid of this part? docs/getting-started.md https://reviews.apache.org/r/35455/#comment140329 I know this isn't yours but it seems weird to mark C++, Java and Python as being code keywords, no? - Till Toenshoff On June

Re: Review Request 35561: Updated process::subprocess to replace environment.

2015-06-17 Thread Till Toenshoff
. It should be right before the preparation of any arguments for, and the invocation of the subprocess. In this particular test, I would put it after the preparation of the parent's environment. - Till Toenshoff On June 17, 2015, 2:28 p.m., Benjamin Hindman wrote

Re: Review Request 35571: Adding ability to decode JSON from ZK

2015-06-17 Thread Till Toenshoff
' (masterInfo.isError() ? : + masterInfo.error() : ); ``` - Till Toenshoff On June 17, 2015, 10:56 p.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 35353: Smaller fixes on libprocess firewall

2015-06-17 Thread Till Toenshoff
referenced by paths -- as such, we do not expect *disabled* endpoint paths as parameters. s/disabled// - Till Toenshoff On June 15, 2015, 3:51 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit

Re: Review Request 35562: Removed unnecessary use of os::ExecEnv.

2015-06-17 Thread Till Toenshoff
to have this TODO? - Till Toenshoff On June 17, 2015, 2:28 p.m., Benjamin Hindman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35562

Re: Review Request 35561: Updated process::subprocess to replace environment.

2015-06-17 Thread Till Toenshoff
On June 17, 2015, 8:23 p.m., Till Toenshoff wrote: 3rdparty/libprocess/src/subprocess.cpp, line 335 https://reviews.apache.org/r/35561/diff/1/?file=986458#file986458line335 Should we be using copies here instead? ``` foreachpair (const string key, const string

Re: Review Request 35563: Removed unused os::ExecEnv.

2015-06-17 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35563/#review88317 --- Ship it! Ship It! - Till Toenshoff On June 17, 2015, 2:28 p.m

Re: Review Request 35565: Refactored os::environment to return a std::map.

2015-06-17 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35565/#review88319 --- Ship it! Ship It! - Till Toenshoff On June 17, 2015, 2:28 p.m

Re: Review Request 35564: Update process::subprocess callees per new semantics.

2015-06-17 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35564/#review88318 --- Ship it! Ship It! - Till Toenshoff On June 17, 2015, 2:28 p.m

Re: Review Request 35566: Refactor executorEnvironment to take slave::Flags.

2015-06-17 Thread Till Toenshoff
://reviews.apache.org/r/35566/#comment140777 See above. - Till Toenshoff On June 17, 2015, 2:28 p.m., Benjamin Hindman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35566

Re: Review Request 35562: Removed unnecessary use of os::ExecEnv.

2015-06-17 Thread Till Toenshoff
On June 17, 2015, 8:32 p.m., Till Toenshoff wrote: Would it make sense to have this RR as being the last in this chain? That way we could be safely commit earlier RRs without breaking the build. Oops, I meant to say the above for the next RR (35563) - Till

Re: Review Request 34427: AppC provisioner backend using bind mounts.

2015-06-15 Thread Till Toenshoff
://reviews.apache.org/r/34427/#comment140424 Given that you dont declare any templates, would it make sense to split this into cpp+hpp? src/slave/containerizer/provisioners/appc/bind_backend.hpp https://reviews.apache.org/r/34427/#comment140425 - Till Toenshoff On May 19, 2015, 6:46

Re: Review Request 35467: Fix a comment in Slave.

2015-06-16 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35467/#review88033 --- Ship it! Ship It! - Till Toenshoff On June 15, 2015, 5:37 p.m

Re: Review Request 35494: Added format for template declaration in the styleguide.

2015-06-16 Thread Till Toenshoff
On June 16, 2015, 8:13 a.m., Joerg Schad wrote: Actually I would really like to understand why we do it this way. Most other code I know omits the space. Still, as this seems to be common practice... That question should get raised on the JIRA to make things transparent. - Till

Re: Review Request 30612: Added /master/frameworks/{framework}/tasks/{task} endpoint.

2015-05-29 Thread Till Toenshoff
: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30612/ --- (Updated May 19, 2015, 8:42 a.m.) Review request for mesos, Adam B, Joerg Schad, Marco Massenzio, Niklas Nielsen, and Till Toenshoff. Bugs: MESOS

Re: Review Request 34268: stout library - adding support for Solaris

2015-06-01 Thread Till Toenshoff
On June 1, 2015, 10:36 a.m., Till Toenshoff wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp, lines 71-82 https://reviews.apache.org/r/34268/diff/2/?file=969835#file969835line71 Seems we got some options here; A. use your separate, stream-based approach

Re: Review Request 35799: Support mounting relative paths with docker.

2015-07-01 Thread Till Toenshoff
Const? src/tests/docker_tests.cpp (line 391) https://reviews.apache.org/r/35799/#comment142979 Const? - Till Toenshoff On July 1, 2015, 12:14 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply

Re: Review Request 36071: Add flow diagram for docker containerizer.

2015-07-01 Thread Till Toenshoff
/docker_containerizer_flow.jpg' error: docs/images/docker_containerizer_flow.jpg: patch does not apply Failed to apply patch ``` - Till Toenshoff On July 1, 2015, 1:07 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit

Review Request 35998: Added doxygen styled comments to Path::basename and Path::dirname.

2015-06-29 Thread Till Toenshoff
Description --- see summary. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp a4afdad0b5f053186ace4d6a37b41cd02e7d415b Diff: https://reviews.apache.org/r/35998/diff/ Testing --- Thanks, Till Toenshoff

Re: Review Request 36167: Updated FirewallRule interface so is consistent with http::Response usage in the project.

2015-07-06 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36167/#review90450 --- Ship it! Ship It! - Till Toenshoff On July 3, 2015, 2:23 p.m

Re: Review Request 36049: Added support for modularized Authorizer

2015-07-03 Thread Till Toenshoff
On July 3, 2015, 8:29 a.m., Till Toenshoff wrote: Let's also update the description of this RR; ``` Adds and integrates helper classes needed to support an Authorizer module. Also adds a flag to the master, allowing the selection of an Authorizer module. ``` or maybe better `.. Authorizer

Re: Review Request 36050: Added test authorizer module

2015-07-03 Thread Till Toenshoff
://reviews.apache.org/r/36050/ --- (Updated July 2, 2015, 11:18 p.m.) Review request for mesos and Till Toenshoff. Bugs: MESOS-2945 https://issues.apache.org/jira/browse/MESOS-2945 Repository: mesos Description --- Adds

  1   2   3   4   5   6   7   8   9   10   >