Re: Review Request 36167: Updated FirewallRule interface so is consistent with http::Response usage in the project.
--- 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., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36167/ --- (Updated July 3, 2015, 2:23 p.m.) Review request for mesos, Adam B, Ben Mahler, and Till Toenshoff. Repository: mesos Description --- While returning an `Optionhttp::Response` may lead to object slicing. Currently it is not a problem since descendants of http::Response do not alter the memory mapping of the object, i.e. they don't add new fields. At the same time, every where else the usage is return `Futurehttp::Response`. In order to keep it consistent the return value of `FirewallRule::apply()` is updated. Keep in mind that if object slicing ever happens, this change must be reverted and usage of `Futurehttp::Response` should be changed to `Futurehttp::Response*`. For discussion see [r/35919/](https://reviews.apache.org/r/35919/) Diffs - 3rdparty/libprocess/include/process/firewall.hpp 692e065f2744f38035d81c0137760d996a295df6 3rdparty/libprocess/src/process.cpp 883776a6d87f3f14d04e2d574b0e0baa469af579 Diff: https://reviews.apache.org/r/36167/diff/ Testing --- make check make distcheck Thanks, Alexander Rojas
Re: Review Request 36167: Updated FirewallRule interface so is consistent with http::Response usage in the project.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36167/#review90449 --- Ship it! Ship It! - Adam B On July 3, 2015, 7:23 a.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36167/ --- (Updated July 3, 2015, 7:23 a.m.) Review request for mesos, Adam B, Ben Mahler, and Till Toenshoff. Repository: mesos Description --- While returning an `Optionhttp::Response` may lead to object slicing. Currently it is not a problem since descendants of http::Response do not alter the memory mapping of the object, i.e. they don't add new fields. At the same time, every where else the usage is return `Futurehttp::Response`. In order to keep it consistent the return value of `FirewallRule::apply()` is updated. Keep in mind that if object slicing ever happens, this change must be reverted and usage of `Futurehttp::Response` should be changed to `Futurehttp::Response*`. For discussion see [r/35919/](https://reviews.apache.org/r/35919/) Diffs - 3rdparty/libprocess/include/process/firewall.hpp 692e065f2744f38035d81c0137760d996a295df6 3rdparty/libprocess/src/process.cpp 883776a6d87f3f14d04e2d574b0e0baa469af579 Diff: https://reviews.apache.org/r/36167/diff/ Testing --- make check make distcheck Thanks, Alexander Rojas
Review Request 36193: Improved Doxygen-Styleguide.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36193/ --- Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Joseph Wu. Repository: mesos Description --- Improved Doxygen-Styleguide for clarifying discussions arising from https://reviews.apache.org/r/36141/. Diffs - docs/mesos-doxygen-style-guide.md 72156c72fc40f72740e57d47d0436c60f6d05bd7 Diff: https://reviews.apache.org/r/36193/diff/ Testing --- Checked rendered markdown. Thanks, Joerg Schad
Re: Review Request 36048: Updated authorizer to allow for modularized implementations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36048/ --- (Updated July 6, 2015, 2:38 p.m.) Review request for mesos, Adam B and Till Toenshoff. Changes --- Fixed a bug when some operations and a deallocation would be applied to a `nullptr`. Bugs: MESOS-2945 https://issues.apache.org/jira/browse/MESOS-2945 Repository: mesos Description --- Splits and updates the original declaration of the `Authorizer` into its interface and a default implementation, the `LocalAuthorizer`. Following the pattern of the modularized `Authenticator`, it generates a default constructor which is required when writing a `TYPED_TEST` in a follow up patch. Additionally, an initialize method has been added, needed for passing in the current ACL definitions as provided via flags. Other changes are just updates to allow for compilation. Diffs (updated) - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/authorizer/authorizer.proto PRE-CREATION include/mesos/mesos.proto 3dd4a5b7a4b3bc56bdc690d6adf05f88c0d28273 include/mesos/type_utils.hpp e7bfe8ca60af945e76b5d85ab37cc97b17ff1b4a src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 src/authorizer/authorizer.hpp c039d9412780aa199db169b31991bf9f45b07d0f src/authorizer/authorizer.cpp 21e97e315478a4ca9442af83732665f85eb2f8fc src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 src/examples/persistent_volume_framework.cpp c6d6ed337bfca91dc146cb31298cabebdbb13509 src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/http.cpp 2be613b2e4c913b74c12d0d8f2d0e25da3cd3656 src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 src/master/master.hpp fb4d6fac85e284987ec8fbf6949b5023875573fb src/master/master.cpp c5a4875f0d43c5091ae9a52c6b1d04105dfa3914 src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26 src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 src/tests/mesos.cpp 5eab6dea6058865847425ab8d31708c92c6f098a Diff: https://reviews.apache.org/r/36048/diff/ Testing --- make check Thanks, Alexander Rojas
Review Request 36197: Documented how to become a committer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36197/ --- Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. Bugs: MESOS-1825 https://issues.apache.org/jira/browse/MESOS-1825 Repository: mesos Description --- Added new document committer-candidate-checklist.md and wrote a paragraph about the path to committership in committers.md. Diffs - docs/committer-candidate-checklist.md PRE-CREATION docs/committers.md ca8a6995c5272f3534ab63f95332565dfcaaf5b9 Diff: https://reviews.apache.org/r/36197/diff/ Testing --- The rendered files can be viewed here: https://gist.github.com/bernd-mesos/00de63ae13efec4331be Thanks, Bernd Mathiske
Re: Review Request 36197: Documented how to become a committer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36197/#review90473 --- docs/committers.md (line 7) https://reviews.apache.org/r/36197/#comment143559 They are also member of the PMC docs/committers.md (line 13) https://reviews.apache.org/r/36197/#comment143562 Sometimes you use Mesos PMC and sometimes Current Committers, there is a difference but currently the sets are identically (with the exception of you actually :-)). - Joerg Schad On July 6, 2015, 12:32 p.m., Bernd Mathiske wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36197/ --- (Updated July 6, 2015, 12:32 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. Bugs: MESOS-1825 https://issues.apache.org/jira/browse/MESOS-1825 Repository: mesos Description --- Added new document committer-candidate-checklist.md and wrote a paragraph about the path to committership in committers.md. Diffs - docs/committer-candidate-checklist.md PRE-CREATION docs/committers.md ca8a6995c5272f3534ab63f95332565dfcaaf5b9 Diff: https://reviews.apache.org/r/36197/diff/ Testing --- The rendered files can be viewed here: https://gist.github.com/bernd-mesos/00de63ae13efec4331be Thanks, Bernd Mathiske
Re: Review Request 36197: Documented how to become a committer.
On July 6, 2015, 5:39 a.m., Joerg Schad wrote: docs/committers.md, line 13 https://reviews.apache.org/r/36197/diff/1/?file=999718#file999718line13 Sometimes you use Mesos PMC and sometimes Current Committers, there is a difference but currently the sets are identically (with the exception of you actually :-)). I tried to use exactly the term (PMC member vs. committer) that is pertinent to the context in question. That they coincide does not mean that they are functionally equivalent. On July 6, 2015, 5:39 a.m., Joerg Schad wrote: docs/committers.md, line 7 https://reviews.apache.org/r/36197/diff/1/?file=999718#file999718line7 They are also member of the PMC I will ad a sentence saying that in the Mesos project committers eventually also join the PMC. - Bernd --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36197/#review90473 --- On July 6, 2015, 5:32 a.m., Bernd Mathiske wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36197/ --- (Updated July 6, 2015, 5:32 a.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. Bugs: MESOS-1825 https://issues.apache.org/jira/browse/MESOS-1825 Repository: mesos Description --- Added new document committer-candidate-checklist.md and wrote a paragraph about the path to committership in committers.md. Diffs - docs/committer-candidate-checklist.md PRE-CREATION docs/committers.md ca8a6995c5272f3534ab63f95332565dfcaaf5b9 Diff: https://reviews.apache.org/r/36197/diff/ Testing --- The rendered files can be viewed here: https://gist.github.com/bernd-mesos/00de63ae13efec4331be Thanks, Bernd Mathiske
Review Request 36200: Change framework for framwork
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36200/ --- Review request for mesos. Bugs: MESOS-2989 https://issues.apache.org/jira/browse/MESOS-2989 Repository: mesos Description --- Hi there, I've found some typos in Mesos. I think that it should be changed to framework from framwork. ``` $ git grep framwork docs/app-framework-development-guide.md: * message, failing over framework, etc.), a framwork that attempts frameworks/torque/torquesched.py:driverlog.info(Mesos torque framwork registered with frameworkID %s % fid) include/mesos/scheduler.hpp: // message, failing over framework, etc.), a framwork that attempts src/java/src/org/apache/mesos/Scheduler.java: * message, failing over framework, etc.), a framwork that attempts src/python/interface/src/mesos/interface/__init__.py: framework, etc.), a framwork that attempts to launch tasks using an src/scaling/nested_exec.py: def registered(self, driver, executorInfo, framworkInfo, slaveInfo): ``` Diffs - docs/app-framework-development-guide.md e340a6c663ca26ea3b00647c46397a794d88a9be frameworks/torque/torquesched.py c22ef9180c3aaf447c0288ee4948d0c8067791b7 include/mesos/scheduler.hpp 0b54ffe0275e58a138957367716f1216cbf02b65 src/java/src/org/apache/mesos/Scheduler.java 0e02f89997d46794d81deef928a1b9485d617b53 src/python/interface/src/mesos/interface/__init__.py e7b208a5a0d46eb90b4b8071bfec97673f707721 src/scaling/nested_exec.py 17e61706c75e6e849b0c40ae5232d8dc60804c55 Diff: https://reviews.apache.org/r/36200/diff/ Testing --- ```sh $ make check ``` Thanks, Ryuichi Okumura
Re: Review Request 36197: Documented how to become a committer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36197/ --- (Updated July 6, 2015, 6:43 a.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. Changes --- Addressed Joerg's review: Added sentence saying that in the Mesos project it is customary to also appoint every committer to the PMC. Bugs: MESOS-1825 https://issues.apache.org/jira/browse/MESOS-1825 Repository: mesos Description --- Added new document committer-candidate-checklist.md and wrote a paragraph about the path to committership in committers.md. Diffs (updated) - docs/committer-candidate-checklist.md PRE-CREATION docs/committers.md ca8a6995c5272f3534ab63f95332565dfcaaf5b9 Diff: https://reviews.apache.org/r/36197/diff/ Testing --- The rendered files can be viewed here: https://gist.github.com/bernd-mesos/00de63ae13efec4331be Thanks, Bernd Mathiske
Review Request 36202: Fixed style problem in TODO
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36202/ --- Review request for mesos, Isabel Jimenez, Joerg Schad, and Till Toenshoff. Repository: mesos Description --- Commit [920930f](https://github.com/apache/mesos/commit/920930fe89f55ac08493829aa5797ba85d104367) introduced a TODO comment which breaks the style checker. This patch just fixes it. Diffs - src/tests/environment.cpp 6a881cad76b63b18a7000ad889fd0e071ca00d47 Diff: https://reviews.apache.org/r/36202/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated July 6, 2015, 5:42 p.m.) Review request for mesos, Adam B and Till Toenshoff. Changes --- Proper use of factory. Includes cleanup. Bugs: MESOS-2945 https://issues.apache.org/jira/browse/MESOS-2945 Repository: mesos Description --- 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. Diffs (updated) - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26 Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36068: MESOS-2966: Fix 'peer()' call for ssl socket.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36068/#review90491 --- Joris, this is discarded now and can be removed right? - Benjamin Hindman On July 1, 2015, 12:33 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36068/ --- (Updated July 1, 2015, 12:33 a.m.) Review request for mesos, Adam B, Benjamin Hindman, and Artem Harutyunyan. Bugs: MESOS-2966 https://issues.apache.org/jira/browse/MESOS-2966 Repository: mesos Description --- This virtualizes the 'get()' call on Socket for LibeventSSLSocketImpl to deal with the extra FD case. Diffs - 3rdparty/libprocess/include/process/socket.hpp f53d2e1dbb31e135c8951145d379cbbff308 3rdparty/libprocess/src/libevent_ssl_socket.hpp 4f2cd357bfdb5268d2bae2df5d7138ff14064bf6 3rdparty/libprocess/src/libevent_ssl_socket.cpp 2920e0e1a5643118b14911d77fb682e60958b4e6 3rdparty/libprocess/src/tests/ssl_tests.cpp c077aaeaecbe2cdcdad2b042741eeb8906699a22 Diff: https://reviews.apache.org/r/36068/diff/ Testing --- Added a test. Failed before fix is applied. Passes after. Thanks, Joris Van Remoortere
Re: Review Request 35287: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout)
Hey Mark, First off, thanks for these patches! I appreciate you taking the time to fix this. Second, I think the review chain is reversed currently. The *stout* changes should be the start of the chain, as it has the introduction of *getOrElse* function. So if you could flip it around to be *stout - libprocess - mesos*, I think ReviewBot will be happier :) Thanks, MPark. On Mon, Jul 6, 2015 at 1:08 PM Mesos ReviewBot reviews@mesos.apache.org wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35287/#review90499 --- Bad patch! Reviews applied: [35285, 35286] Failed command: ./support/apply-review.sh -n -r 35286 Error: 2015-07-06 17:08:20 URL:https://reviews.apache.org/r/35286/diff/raw/ [1598/1598] - 35286.patch [1] error: patch failed: 3rdparty/libprocess/src/subprocess.cpp:327 error: 3rdparty/libprocess/src/subprocess.cpp: patch does not apply Failed to apply patch - Mesos ReviewBot On July 6, 2015, 3:04 p.m., Mark Wang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35287/ --- (Updated July 6, 2015, 3:04 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout) Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 8d5217a699a302cc9ebb2aa10d74cced7eb2b3d9 3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp c8d30d8c193eb14f7accfde4fe02ce0710cd1817 3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp f1ae80ab881ccfcefda69d5bfee2d969d171f1b9 Diff: https://reviews.apache.org/r/35287/diff/ Testing --- make check Thanks, Mark Wang
Re: Review Request 36207: Unified line wrapping for defines in gtest.hpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36207/ --- (Updated July 6, 2015, 5:19 p.m.) Review request for mesos and Alexander Rojas. Changes --- Fixed ooopsi. Repository: mesos Description --- This updates a bunch of defines within libprocess' gtest.hpp to unify their wrapping towards a consistent style. Diffs (updated) - 3rdparty/libprocess/include/process/gtest.hpp 8518c38c9cbf62a098b43b4d0b6d80e561b17ad4 Diff: https://reviews.apache.org/r/36207/diff/ Testing --- make check Thanks, Till Toenshoff
Re: Review Request 31444: Support chrooting in MesosContainerizer launch helper.
On June 29, 2015, 4:29 p.m., Jiang Yan Xu wrote:
src/slave/containerizer/mesos/launch.cpp, lines 64-65
https://reviews.apache.org/r/31444/diff/7/?file=989735#file989735line64
must be relative to is really is interpreted as relative to right?
Just wanted be sure clarify:
1) Should the user specify an absolute path with a preceding /?
2) The directory path as observed by processes outside the choot jail
is `path::join(rootfs, directory)` right?
1) Yes, absolute path. Added this to the description.
2) Yes.
On June 29, 2015, 4:29 p.m., Jiang Yan Xu wrote:
src/slave/containerizer/mesos/launch.cpp, lines 259-260
https://reviews.apache.org/r/31444/diff/7/?file=989735#file989735line259
This must be an absolute path
As in, if the flags specifies a path without a preceding slash this
throws an error?
This is not enforced is it?
Actually, it's just interpreted relative to the new root since we chdir() after
chroot() which will change to /. I clarified the comment.
- Ian
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31444/#review89425
---
On June 22, 2015, 9:38 a.m., Ian Downes wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31444/
---
(Updated June 22, 2015, 9:38 a.m.)
Review request for mesos, Chi Zhang, Dominic Hamon, Jay Buffington, Jie Yu,
and James Peach.
Bugs: MESOS-2350
https://issues.apache.org/jira/browse/MESOS-2350
Repository: mesos
Description
---
Optionally take a path that the launch helper should chroot to before
exec'ing the executor. It is assumed that the work directory is mounted to
the appropriate location under the chroot. In particular, the path to the
executor must be relative to the chroot.
Configuration that should be private to the chroot is done during the launch,
e.g. mounting proc and statically configuring basic devices. It is assumed
that other configuration, e.g., preparing the image, mounting in volumes or
persistent resources, is done by the caller.
Mounts can be made to the chroot (e.g., updating the volumes or persistent
resources) and they will propagate in to the container but mounts made inside
the container will not propagate out to the host.
It currently assumes that at least {{chroot}}/tmp is writeable and that mount
points {{chroot}}/{tmp,dev,proc,sys} exist in the chroot.
This is specific to Linux.
Diffs
-
src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573
src/linux/fs.cpp 568565f878b34708170a886dc4d62849aa01f263
src/slave/containerizer/mesos/launch.hpp
7c8b535746b5ce9add00afef86fdb6faefb5620e
src/slave/containerizer/mesos/launch.cpp
2f2d60e2011f60ec711d3b29fd2c157e30c83c34
src/tests/launch_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/31444/diff/
Testing
---
Manual testing only so far. This is harder to automate because we need a
self-contained chroot to execute something in... Suggestions welcome.
Thanks,
Ian Downes
Re: Review Request 36204: Pass slave's total resources in ResourceUsage.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36204/#review90510 --- Thanks Bartek! See my detailed comments. include/mesos/mesos.proto (lines 671 - 672) https://reviews.apache.org/r/36204/#comment143629 I think the 'total' here should be slaveInfo's resources (with no meta data, from command line --resourecs flag) applying checkpointed resources (for dynamic reservation/persistent volumes). The rationale is that the resource estimator might want to know about the reservation/persisent volume information to make a more informed decision. For example, some resource estimator might not want to oversubscribe dynamically reserved resources. So, could you please call out that the 'total' here is after applying the checkpointed resources to account for dynamic reservation and persistent volumes. ALso, could you please add a new line above the comment? Thanks! src/slave/slave.cpp (line 4379) https://reviews.apache.org/r/36204/#comment143630 Per my comments above, the logic here needs to be adjusted. We need to apply checkpointed resource to info.resources() here. Please refer to the following code: https://github.com/apache/mesos/blob/master/src/slave/slave.cpp#L3883 - Jie Yu On July 6, 2015, 3:25 p.m., Bartek Plotka wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36204/ --- (Updated July 6, 2015, 3:25 p.m.) Review request for mesos, Ben Mahler, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone. Bugs: MESOS-2933 https://issues.apache.org/jira/browse/MESOS-2933 Repository: mesos Description --- Pass slave's total resources to the ResourceEstimator and QoSController via Slave::usage(). Diffs - include/mesos/mesos.proto 3dd4a5b7a4b3bc56bdc690d6adf05f88c0d28273 src/slave/slave.cpp 008170f4ce7b97eda43b50b48083b32ae65925c0 Diff: https://reviews.apache.org/r/36204/diff/ Testing --- make check. Thanks, Bartek Plotka
Re: Review Request 36189: Add strings::Mode to strings::trim.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36189/#review90512 --- Ship it! Ship It! - Vinod Kone On July 6, 2015, 3:29 p.m., Benjamin Hindman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36189/ --- (Updated July 6, 2015, 3:29 p.m.) Review request for mesos, Artem Harutyunyan and Vinod Kone. Bugs: MESOS-2862 https://issues.apache.org/jira/browse/MESOS-2862 Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 963029bea989a68a484f7b8b47d29ea5fffeb955 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 9733b2ec5d68605b694210c66144b8d9f8c36467 Diff: https://reviews.apache.org/r/36189/diff/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 35287: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35287/#review90499 --- Bad patch! Reviews applied: [35285, 35286] Failed command: ./support/apply-review.sh -n -r 35286 Error: 2015-07-06 17:08:20 URL:https://reviews.apache.org/r/35286/diff/raw/ [1598/1598] - 35286.patch [1] error: patch failed: 3rdparty/libprocess/src/subprocess.cpp:327 error: 3rdparty/libprocess/src/subprocess.cpp: patch does not apply Failed to apply patch - Mesos ReviewBot On July 6, 2015, 3:04 p.m., Mark Wang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35287/ --- (Updated July 6, 2015, 3:04 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout) Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 8d5217a699a302cc9ebb2aa10d74cced7eb2b3d9 3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp c8d30d8c193eb14f7accfde4fe02ce0710cd1817 3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp f1ae80ab881ccfcefda69d5bfee2d969d171f1b9 Diff: https://reviews.apache.org/r/35287/diff/ Testing --- make check Thanks, Mark Wang
Re: Review Request 36068: MESOS-2966: Fix 'peer()' call for ssl socket.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36068/ --- (Updated July 6, 2015, 5:08 p.m.) Review request for mesos, Adam B, Benjamin Hindman, and Artem Harutyunyan. Bugs: MESOS-2966 https://issues.apache.org/jira/browse/MESOS-2966 Repository: mesos Description --- This virtualizes the 'get()' call on Socket for LibeventSSLSocketImpl to deal with the extra FD case. Diffs - 3rdparty/libprocess/include/process/socket.hpp f53d2e1dbb31e135c8951145d379cbbff308 3rdparty/libprocess/src/libevent_ssl_socket.hpp 4f2cd357bfdb5268d2bae2df5d7138ff14064bf6 3rdparty/libprocess/src/libevent_ssl_socket.cpp 2920e0e1a5643118b14911d77fb682e60958b4e6 3rdparty/libprocess/src/tests/ssl_tests.cpp c077aaeaecbe2cdcdad2b042741eeb8906699a22 Diff: https://reviews.apache.org/r/36068/diff/ Testing --- Added a test. Failed before fix is applied. Passes after. Thanks, Joris Van Remoortere
Re: Review Request 35287: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35287/#review90504 --- Hi Mark, Can you please make sure to rebase and post-review all of the patches in the dependency chain? The build bot will keep reporting this conflict until it can apply the patches in the order it reports in its review. - Joris Van Remoortere On July 6, 2015, 3:04 p.m., Mark Wang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35287/ --- (Updated July 6, 2015, 3:04 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout) Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 8d5217a699a302cc9ebb2aa10d74cced7eb2b3d9 3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp c8d30d8c193eb14f7accfde4fe02ce0710cd1817 3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp f1ae80ab881ccfcefda69d5bfee2d969d171f1b9 Diff: https://reviews.apache.org/r/35287/diff/ Testing --- make check Thanks, Mark Wang
Re: Review Request 35287: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout)
On July 6, 2015, 5:23 p.m., Joris Van Remoortere wrote: Hi Mark, Can you please make sure to rebase and post-review all of the patches in the dependency chain? The build bot will keep reporting this conflict until it can apply the patches in the order it reports in its review. Yea, I resolved the conflict. - Mark --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35287/#review90504 --- On July 6, 2015, 6 p.m., Mark Wang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35287/ --- (Updated July 6, 2015, 6 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout) Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 3d9b7a7568de6734097733f4e6d59acba629b849 3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 784104f26ceee2ef90709056a5f4428d48390c36 3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp d87cf22dbb98e1ac99f129668ad984a1542e4ec9 3rdparty/libprocess/src/subprocess.cpp 5b41f0d88fcab93d51d5c503f69faccedc210868 Diff: https://reviews.apache.org/r/35287/diff/ Testing --- make check Thanks, Mark Wang
Re: Review Request 35285: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (mesos)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35285/ --- (Updated July 6, 2015, 6:02 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (mesos) Diffs (updated) - src/log/catchup.cpp f7afc38916d0a7e57cdecb0da7ccb3901e726b90 src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 src/master/master.hpp fb4d6fac85e284987ec8fbf6949b5023875573fb src/slave/containerizer/containerizer.cpp b9ac94c59fdc229516d1ae193992ce0cf8ff96be src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26 Diff: https://reviews.apache.org/r/35285/diff/ Testing --- make check Thanks, Mark Wang
Re: Review Request 36204: Pass slave's total resources in ResourceUsage.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36204/#review90516 --- Patch looks great! Reviews applied: [36204] All tests passed. - Mesos ReviewBot On July 6, 2015, 3:25 p.m., Bartek Plotka wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36204/ --- (Updated July 6, 2015, 3:25 p.m.) Review request for mesos, Ben Mahler, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone. Bugs: MESOS-2933 https://issues.apache.org/jira/browse/MESOS-2933 Repository: mesos Description --- Pass slave's total resources to the ResourceEstimator and QoSController via Slave::usage(). Diffs - include/mesos/mesos.proto 3dd4a5b7a4b3bc56bdc690d6adf05f88c0d28273 src/slave/slave.cpp 008170f4ce7b97eda43b50b48083b32ae65925c0 Diff: https://reviews.apache.org/r/36204/diff/ Testing --- make check. Thanks, Bartek Plotka
Re: Review Request 36200: Change framework for framwork
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36200/#review90490 --- Ship it! Ship It! - Kapil Arya On July 6, 2015, 9:38 a.m., Ryuichi Okumura wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36200/ --- (Updated July 6, 2015, 9:38 a.m.) Review request for mesos. Bugs: MESOS-2989 https://issues.apache.org/jira/browse/MESOS-2989 Repository: mesos Description --- Hi there, I've found some typos in Mesos. I think that it should be changed to framework from framwork. ``` $ git grep framwork docs/app-framework-development-guide.md: * message, failing over framework, etc.), a framwork that attempts frameworks/torque/torquesched.py:driverlog.info(Mesos torque framwork registered with frameworkID %s % fid) include/mesos/scheduler.hpp: // message, failing over framework, etc.), a framwork that attempts src/java/src/org/apache/mesos/Scheduler.java: * message, failing over framework, etc.), a framwork that attempts src/python/interface/src/mesos/interface/__init__.py: framework, etc.), a framwork that attempts to launch tasks using an src/scaling/nested_exec.py: def registered(self, driver, executorInfo, framworkInfo, slaveInfo): ``` Diffs - docs/app-framework-development-guide.md e340a6c663ca26ea3b00647c46397a794d88a9be frameworks/torque/torquesched.py c22ef9180c3aaf447c0288ee4948d0c8067791b7 include/mesos/scheduler.hpp 0b54ffe0275e58a138957367716f1216cbf02b65 src/java/src/org/apache/mesos/Scheduler.java 0e02f89997d46794d81deef928a1b9485d617b53 src/python/interface/src/mesos/interface/__init__.py e7b208a5a0d46eb90b4b8071bfec97673f707721 src/scaling/nested_exec.py 17e61706c75e6e849b0c40ae5232d8dc60804c55 Diff: https://reviews.apache.org/r/36200/diff/ Testing --- ```sh $ make check ``` Thanks, Ryuichi Okumura
Review Request 36207: Unified line wrapping for defines in gtest.hpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36207/ --- Review request for mesos and Alexander Rojas. Repository: mesos Description --- This updates a bunch of defines within libprocess' gtest.hpp to unify their wrapping towards a consistent style. Diffs - 3rdparty/libprocess/include/process/gtest.hpp 8518c38c9cbf62a098b43b4d0b6d80e561b17ad4 Diff: https://reviews.apache.org/r/36207/diff/ Testing --- make check Thanks, Till Toenshoff
Re: Review Request 36106: cgroups: added cpuacct subsystem
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/ --- (Updated July 6, 2015, 6:20 p.m.) Review request for mesos, Ian Downes, Jie Yu, Joris Van Remoortere, and Timothy Chen. Bugs: MESOS-2961 https://issues.apache.org/jira/browse/MESOS-2961 Repository: mesos Description --- cgroups implementation does not have a cpuacct subsystem implementation as of today. Adding the implementation for stat function. Changes: - added Stats class to encapsulate cpuacct.stat data - added implementation for cpuacct::stats - added unit tests Jira: MESOS-2961 Diffs - src/linux/cgroups.hpp 73b98317880eea3d6a2ba37ac56d1f7e3600ba94 src/linux/cgroups.cpp 4c006d0c7382b940a83359d636c0d48952cdbb00 src/tests/cgroups_tests.cpp 475f48a474eea708f98d8c0300862351a2d4379a Diff: https://reviews.apache.org/r/36106/diff/ Testing --- make check Thanks, Jojy Varghese
Review Request 36216: Only run netcat tests when nc is available.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36216/ --- Review request for mesos, Bernd Mathiske, Joerg Schad, and Till Toenshoff. Repository: mesos Description --- nc is not available on CentOS 7.1 and all the tests using nc will fail. Added a new NetcatFilter so we detect nc before running these tests. Diffs - src/tests/docker_containerizer_tests.cpp d4ccb0b30fe27980846d913e292d2e18fd3d1055 src/tests/environment.cpp 3726e5d95b9388f0881184e1b4874e1eb2e7f531 src/tests/port_mapping_tests.cpp ac49cdfdcf6baf00ac2907e193c683c8b6c83ffb Diff: https://reviews.apache.org/r/36216/diff/ Testing --- make check Thanks, Timothy Chen
Review Request 36214: Fix running docker executor tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36214/ --- Review request for mesos, Bernd Mathiske, Joerg Schad, and Till Toenshoff. Repository: mesos Description --- When running on CentOS 7.1 it didn't have /bin in it's PATH and therefore it failed to find test-executor that is in /bin/ in the container. This works on other systems since the tests inherits the environment variables from the system and /bin usually is in the PATH. Diffs - src/tests/docker_containerizer_tests.cpp d4ccb0b30fe27980846d913e292d2e18fd3d1055 Diff: https://reviews.apache.org/r/36214/diff/ Testing --- make check Thanks, Timothy Chen
Review Request 36215: Dummy /call endpoint for master
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36215/ --- Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and Vinod Kone. Bugs: MESOS-2860 https://issues.apache.org/jira/browse/MESOS-2860 Repository: mesos-incubating Description --- Adding a dummy enpoint for Call requests. Diffs - src/Makefile.am addb63f src/master/http.cpp 2be613b src/master/master.hpp fb4d6fa src/master/master.cpp c5a4875 src/tests/call_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36215/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 36215: Dummy /call endpoint for master
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36215/#review90602 --- Ship it! Thanks isabel, I left some comments and cleaned them up for you, since they are pretty trivial. One follow-up request: We should be rejecting GET requests against the /call endpoint, yes? I've added a TODO for you. src/Makefile.am (line 1468) https://reviews.apache.org/r/36215/#comment143703 Let's call this http_api_tests? At least, that will give us a place to collect tests specific to the http api, as I imagine that we will want to test http events within the same file. src/master/http.cpp (lines 295 - 301) https://reviews.apache.org/r/36215/#comment143699 How about a TODO here to provide richer help information? As it is, hard to tell how this relates to the http api, what the request bodies should contain, etc. src/master/master.hpp (lines 1064 - 1066) https://reviews.apache.org/r/36215/#comment143702 Hm.. not sure this can remain 'const' given it will have to perform non-const operations on the master, but sounds ok for now. src/tests/call_tests.cpp (line 19) https://reviews.apache.org/r/36215/#comment143708 Need this? src/tests/call_tests.cpp (line 22) https://reviews.apache.org/r/36215/#comment143709 Need this? I think you need gtest.hpp instead. src/tests/call_tests.cpp (line 26) https://reviews.apache.org/r/36215/#comment143700 Need this? src/tests/call_tests.cpp (line 31) https://reviews.apache.org/r/36215/#comment143701 Need this? src/tests/call_tests.cpp (line 56) https://reviews.apache.org/r/36215/#comment143706 I'm guilty of this too, we don't need the extra space between anymore :) src/tests/call_tests.cpp (line 65) https://reviews.apache.org/r/36215/#comment143698 You don't need this line, the next one captures the expectation that it will become ready. :) - Ben Mahler On July 6, 2015, 6:31 p.m., Isabel Jimenez wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36215/ --- (Updated July 6, 2015, 6:31 p.m.) Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and Vinod Kone. Bugs: MESOS-2860 https://issues.apache.org/jira/browse/MESOS-2860 Repository: mesos-incubating Description --- Adding a dummy enpoint for Call requests. Diffs - src/Makefile.am addb63f src/master/http.cpp 2be613b src/master/master.hpp fb4d6fa src/master/master.cpp c5a4875 src/tests/call_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36215/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 23784: Missing Apache headers for stout
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23784/#review90609 --- Patch looks great! Reviews applied: [23784] All tests passed. - Mesos ReviewBot On July 6, 2015, 10:02 p.m., Isabel Jimenez wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23784/ --- (Updated July 6, 2015, 10:02 p.m.) Review request for mesos, Benjamin Hindman and Dominic Hamon. Repository: mesos Description --- Adding missing Apache licence headers on .cpp, .hpp and Makefiles Diffs - 3rdparty/libprocess/3rdparty/stout/Makefile.am 9df5de4 3rdparty/libprocess/3rdparty/stout/include/Makefile.am ff6fb28 3rdparty/libprocess/3rdparty/stout/tests/bits_tests.cpp c47bd13 3rdparty/libprocess/3rdparty/stout/tests/bytes_tests.cpp dc05284 3rdparty/libprocess/3rdparty/stout/tests/cache_tests.cpp f8a0ba5 3rdparty/libprocess/3rdparty/stout/tests/duration_tests.cpp 724c5fe 3rdparty/libprocess/3rdparty/stout/tests/error_tests.cpp 35a62b1 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 2a6f67b 3rdparty/libprocess/3rdparty/stout/tests/gzip_tests.cpp 2211f31 3rdparty/libprocess/3rdparty/stout/tests/hashmap_tests.cpp 4a8176b 3rdparty/libprocess/3rdparty/stout/tests/hashset_tests.cpp 97a7167 3rdparty/libprocess/3rdparty/stout/tests/interval_tests.cpp 1fa8fc1 3rdparty/libprocess/3rdparty/stout/tests/ip_tests.cpp c50480b 3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 0011f08 3rdparty/libprocess/3rdparty/stout/tests/linkedhashmap_tests.cpp 0644d99 3rdparty/libprocess/3rdparty/stout/tests/mac_tests.cpp b80fff9 3rdparty/libprocess/3rdparty/stout/tests/main.cpp f5f20ee 3rdparty/libprocess/3rdparty/stout/tests/multimap_tests.cpp 11f3bf7 3rdparty/libprocess/3rdparty/stout/tests/none_tests.cpp 1c1f8be 3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp 4a0f60b 3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp fee942c 3rdparty/libprocess/3rdparty/stout/tests/os/signals_tests.cpp 6d2d3d5 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 4a01d94 3rdparty/libprocess/3rdparty/stout/tests/proc_tests.cpp dd3700d 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp b3ce131 3rdparty/libprocess/3rdparty/stout/tests/set_tests.cpp 3ddba94 3rdparty/libprocess/3rdparty/stout/tests/some_tests.cpp 0689446 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 9733b2e 3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp 93339eb 3rdparty/libprocess/3rdparty/stout/tests/uuid_tests.cpp 7ac8fc0 Diff: https://reviews.apache.org/r/23784/diff/ Testing --- Thanks, Isabel Jimenez
Re: Review Request 36185: Create pre-launch hook before a docker container launches in slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/#review90610 --- Thanks for adding the hook! There is one more code path that a docker container can be launched, which is in launchExecutorProcess. Can you add the hook call in that path as well? - Timothy Chen On July 5, 2015, 9:14 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/ --- (Updated July 5, 2015, 9:14 a.m.) Review request for mesos and Timothy Chen. Bugs: MESOS-2588 https://issues.apache.org/jira/browse/MESOS-2588 Repository: mesos Description --- Create pre-launch hook before a docker container launches in slave. Diffs - include/mesos/hook.hpp 0995c249e9f07c6c4a26d1c5c369d48bb8056f1f src/hook/manager.hpp 47e8eb7d54d55049d054cf9b1225e67333f22adc src/hook/manager.cpp 0108534c1fc527a0c66d201d7a5232e80b9928bf src/slave/containerizer/docker.cpp 6eb1c84b1a6ef17c3e2ea2028e2f2d75110176ba Diff: https://reviews.apache.org/r/36185/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 36205: Added AWAIT_EXPECT_TRUE and AWAIT_EXPECT_FALSE macros.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36205/#review90616 --- Ship it! Ship It! - Isabel Jimenez On July 6, 2015, 3:44 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36205/ --- (Updated July 6, 2015, 3:44 p.m.) Review request for mesos, Bernd Mathiske, Joerg Schad, and Till Toenshoff. Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/include/process/gtest.hpp 8518c38c9cbf62a098b43b4d0b6d80e561b17ad4 Diff: https://reviews.apache.org/r/36205/diff/ Testing --- Used in tests in follow up patch. Thanks, Alexander Rojas
Re: Review Request 36216: Only run netcat tests when nc is available.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36216/#review90597 --- Patch looks great! Reviews applied: [36216] All tests passed. - Mesos ReviewBot On July 6, 2015, 8:40 p.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36216/ --- (Updated July 6, 2015, 8:40 p.m.) Review request for mesos, Bernd Mathiske, Joerg Schad, and Till Toenshoff. Bugs: MESOS-2996 https://issues.apache.org/jira/browse/MESOS-2996 Repository: mesos Description --- nc is not available on CentOS 7.1 and all the tests using nc will fail. Added a new NetcatFilter so we detect nc before running these tests. Diffs - src/tests/docker_containerizer_tests.cpp d4ccb0b30fe27980846d913e292d2e18fd3d1055 src/tests/environment.cpp 3726e5d95b9388f0881184e1b4874e1eb2e7f531 src/tests/port_mapping_tests.cpp ac49cdfdcf6baf00ac2907e193c683c8b6c83ffb Diff: https://reviews.apache.org/r/36216/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 36217: Adding http validations to master call request validations
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36217/ --- (Updated July 6, 2015, 11:31 p.m.) Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and Vinod Kone. Bugs: MESOS-2860 https://issues.apache.org/jira/browse/MESOS-2860 Repository: mesos-incubating Description --- Adding helper validations for http requests Diffs (updated) - src/Makefile.am addb63f src/common/http_validation.hpp PRE-CREATION src/common/http_validation.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36217/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 36106: cgroups: added cpuacct subsystem
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/ --- (Updated July 7, 2015, 12:26 a.m.) Review request for mesos, Ian Downes, Jie Yu, Joris Van Remoortere, and Timothy Chen. Changes --- post review comments from joris. Bugs: MESOS-2961 https://issues.apache.org/jira/browse/MESOS-2961 Repository: mesos Description --- cgroups implementation does not have a cpuacct subsystem implementation as of today. Adding the implementation for stat function. Changes: - added Stats class to encapsulate cpuacct.stat data - added implementation for cpuacct::stats - added unit tests Jira: MESOS-2961 Diffs (updated) - src/linux/cgroups.hpp 73b98317880eea3d6a2ba37ac56d1f7e3600ba94 src/linux/cgroups.cpp 4c006d0c7382b940a83359d636c0d48952cdbb00 src/tests/cgroups_tests.cpp 475f48a474eea708f98d8c0300862351a2d4379a Diff: https://reviews.apache.org/r/36106/diff/ Testing --- make check Thanks, Jojy Varghese
Re: Review Request 36189: Add strings::Mode to strings::trim.
On July 6, 2015, 1:53 p.m., Artem Harutyunyan wrote: Ship It! Thanks for taking care of this! - Artem --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36189/#review90558 --- On July 6, 2015, 8:29 a.m., Benjamin Hindman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36189/ --- (Updated July 6, 2015, 8:29 a.m.) Review request for mesos, Artem Harutyunyan and Vinod Kone. Bugs: MESOS-2862 https://issues.apache.org/jira/browse/MESOS-2862 Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 963029bea989a68a484f7b8b47d29ea5fffeb955 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 9733b2ec5d68605b694210c66144b8d9f8c36467 Diff: https://reviews.apache.org/r/36189/diff/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 36189: Add strings::Mode to strings::trim.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36189/#review90558 --- Ship it! Ship It! - Artem Harutyunyan On July 6, 2015, 8:29 a.m., Benjamin Hindman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36189/ --- (Updated July 6, 2015, 8:29 a.m.) Review request for mesos, Artem Harutyunyan and Vinod Kone. Bugs: MESOS-2862 https://issues.apache.org/jira/browse/MESOS-2862 Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 963029bea989a68a484f7b8b47d29ea5fffeb955 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 9733b2ec5d68605b694210c66144b8d9f8c36467 Diff: https://reviews.apache.org/r/36189/diff/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 35287: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35287/ --- (Updated July 6, 2015, 6:32 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout) Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 3d9b7a7568de6734097733f4e6d59acba629b849 3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 784104f26ceee2ef90709056a5f4428d48390c36 3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp d87cf22dbb98e1ac99f129668ad984a1542e4ec9 3rdparty/libprocess/src/subprocess.cpp 5b41f0d88fcab93d51d5c503f69faccedc210868 Diff: https://reviews.apache.org/r/35287/diff/ Testing --- make check Thanks, Mark Wang
Re: Review Request 35287: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35287/#review90536 --- Ship it! Ship It! - Joris Van Remoortere On July 6, 2015, 6:46 p.m., Mark Wang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35287/ --- (Updated July 6, 2015, 6:46 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout) Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 3d9b7a7568de6734097733f4e6d59acba629b849 3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 784104f26ceee2ef90709056a5f4428d48390c36 3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp d87cf22dbb98e1ac99f129668ad984a1542e4ec9 Diff: https://reviews.apache.org/r/35287/diff/ Testing --- make check Thanks, Mark Wang
Re: Review Request 36217: Adding http validations to master call request validations
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36217/ --- (Updated July 6, 2015, 9:24 p.m.) Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and Vinod Kone. Bugs: MESOS-2860 https://issues.apache.org/jira/browse/MESOS-2860 Repository: mesos-incubating Description --- Adding helper validations for http requests Diffs (updated) - src/Makefile.am addb63f src/common/http_validation.hpp PRE-CREATION src/common/http_validation.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36217/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 36215: Dummy /call endpoint for master
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36215/#review90585 --- Patch looks great! Reviews applied: [36215] All tests passed. - Mesos ReviewBot On July 6, 2015, 6:31 p.m., Isabel Jimenez wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36215/ --- (Updated July 6, 2015, 6:31 p.m.) Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and Vinod Kone. Bugs: MESOS-2860 https://issues.apache.org/jira/browse/MESOS-2860 Repository: mesos-incubating Description --- Adding a dummy enpoint for Call requests. Diffs - src/Makefile.am addb63f src/master/http.cpp 2be613b src/master/master.hpp fb4d6fa src/master/master.cpp c5a4875 src/tests/call_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36215/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 36193: Improved Doxygen-Styleguide.
On July 6, 2015, 11:39 a.m., Joseph Wu wrote: docs/mesos-doxygen-style-guide.md, line 102 https://reviews.apache.org/r/36193/diff/1/?file=999588#file999588line102 This isn't part of the diff, but should this `@note` tag be replaced with a `**NOTE:**`? Joerg Schad wrote: Good catch! But not part of this review, do you want to create another patch for this? Patch here: https://reviews.apache.org/r/36218/ - Joseph --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36193/#review90525 --- On July 6, 2015, 2:01 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36193/ --- (Updated July 6, 2015, 2:01 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Joseph Wu. Repository: mesos Description --- Improved Doxygen-Styleguide for clarifying discussions arising from https://reviews.apache.org/r/36141/. Diffs - docs/mesos-doxygen-style-guide.md 72156c72fc40f72740e57d47d0436c60f6d05bd7 Diff: https://reviews.apache.org/r/36193/diff/ Testing --- Checked rendered markdown. Thanks, Joerg Schad
Re: Review Request 36024: Refactored OSNetUri tests for fetcher to avoid code copy/pasting.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36024/ --- (Updated July 6, 2015, 1:59 p.m.) Review request for Joris Van Remoortere. Bugs: MESOS-2862 https://issues.apache.org/jira/browse/MESOS-2862 Repository: mesos Description --- Refactored OSNetUri tests for fetcher to avoid code copy/pasting. Diffs (updated) - src/tests/fetcher_tests.cpp ae10c420f7dddb8650377c91b5343591e8560392 Diff: https://reviews.apache.org/r/36024/diff/ Testing --- # This is a test case refactoring. $ make check Thanks, Artem Harutyunyan
Re: Review Request 23784: Missing Apache headers for stout
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23784/ --- (Updated July 6, 2015, 10:02 p.m.) Review request for mesos, Benjamin Hindman and Dominic Hamon. Repository: mesos Description --- Adding missing Apache licence headers on .cpp, .hpp and Makefiles Diffs (updated) - 3rdparty/libprocess/3rdparty/stout/Makefile.am 9df5de4 3rdparty/libprocess/3rdparty/stout/include/Makefile.am ff6fb28 3rdparty/libprocess/3rdparty/stout/tests/bits_tests.cpp c47bd13 3rdparty/libprocess/3rdparty/stout/tests/bytes_tests.cpp dc05284 3rdparty/libprocess/3rdparty/stout/tests/cache_tests.cpp f8a0ba5 3rdparty/libprocess/3rdparty/stout/tests/duration_tests.cpp 724c5fe 3rdparty/libprocess/3rdparty/stout/tests/error_tests.cpp 35a62b1 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 2a6f67b 3rdparty/libprocess/3rdparty/stout/tests/gzip_tests.cpp 2211f31 3rdparty/libprocess/3rdparty/stout/tests/hashmap_tests.cpp 4a8176b 3rdparty/libprocess/3rdparty/stout/tests/hashset_tests.cpp 97a7167 3rdparty/libprocess/3rdparty/stout/tests/interval_tests.cpp 1fa8fc1 3rdparty/libprocess/3rdparty/stout/tests/ip_tests.cpp c50480b 3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 0011f08 3rdparty/libprocess/3rdparty/stout/tests/linkedhashmap_tests.cpp 0644d99 3rdparty/libprocess/3rdparty/stout/tests/mac_tests.cpp b80fff9 3rdparty/libprocess/3rdparty/stout/tests/main.cpp f5f20ee 3rdparty/libprocess/3rdparty/stout/tests/multimap_tests.cpp 11f3bf7 3rdparty/libprocess/3rdparty/stout/tests/none_tests.cpp 1c1f8be 3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp 4a0f60b 3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp fee942c 3rdparty/libprocess/3rdparty/stout/tests/os/signals_tests.cpp 6d2d3d5 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 4a01d94 3rdparty/libprocess/3rdparty/stout/tests/proc_tests.cpp dd3700d 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp b3ce131 3rdparty/libprocess/3rdparty/stout/tests/set_tests.cpp 3ddba94 3rdparty/libprocess/3rdparty/stout/tests/some_tests.cpp 0689446 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 9733b2e 3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp 93339eb 3rdparty/libprocess/3rdparty/stout/tests/uuid_tests.cpp 7ac8fc0 Diff: https://reviews.apache.org/r/23784/diff/ Testing --- Thanks, Isabel Jimenez
Re: Review Request 35286: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (libprocess)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35286/ --- (Updated July 6, 2015, 6:46 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (libprocess) Diffs - 3rdparty/libprocess/src/process.cpp 883776a6d87f3f14d04e2d574b0e0baa469af579 Diff: https://reviews.apache.org/r/35286/diff/ Testing --- make check Thanks, Mark Wang
Re: Review Request 35285: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (mesos)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35285/ --- (Updated July 6, 2015, 6:45 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (mesos) Diffs (updated) - src/log/catchup.cpp f7afc38916d0a7e57cdecb0da7ccb3901e726b90 src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 src/master/master.hpp fb4d6fac85e284987ec8fbf6949b5023875573fb src/slave/containerizer/containerizer.cpp b9ac94c59fdc229516d1ae193992ce0cf8ff96be src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26 Diff: https://reviews.apache.org/r/35285/diff/ Testing --- make check Thanks, Mark Wang
Re: Review Request 35287: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35287/ --- (Updated July 6, 2015, 6:46 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout) Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 3d9b7a7568de6734097733f4e6d59acba629b849 3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 784104f26ceee2ef90709056a5f4428d48390c36 3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp d87cf22dbb98e1ac99f129668ad984a1542e4ec9 Diff: https://reviews.apache.org/r/35287/diff/ Testing --- make check Thanks, Mark Wang
Re: Review Request 35286: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (libprocess)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35286/#review90537 --- Ship it! Ship It! - Joris Van Remoortere On July 6, 2015, 6:46 p.m., Mark Wang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35286/ --- (Updated July 6, 2015, 6:46 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (libprocess) Diffs - 3rdparty/libprocess/src/process.cpp 883776a6d87f3f14d04e2d574b0e0baa469af579 Diff: https://reviews.apache.org/r/35286/diff/ Testing --- make check Thanks, Mark Wang
Re: Review Request 36214: Fix running docker executor tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36214/#review90557 --- Patch looks great! Reviews applied: [36214] All tests passed. - Mesos ReviewBot On July 6, 2015, 8:40 p.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36214/ --- (Updated July 6, 2015, 8:40 p.m.) Review request for mesos, Bernd Mathiske, Joerg Schad, and Till Toenshoff. Bugs: MESOS-2996 https://issues.apache.org/jira/browse/MESOS-2996 Repository: mesos Description --- When running on CentOS 7.1 it didn't have /bin in it's PATH and therefore it failed to find test-executor that is in /bin/ in the container. This works on other systems since the tests inherits the environment variables from the system and /bin usually is in the PATH. Diffs - src/tests/docker_containerizer_tests.cpp d4ccb0b30fe27980846d913e292d2e18fd3d1055 Diff: https://reviews.apache.org/r/36214/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 35285: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (mesos)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35285/#review90538 --- Ship it! Ship It! - Joris Van Remoortere On July 6, 2015, 6:47 p.m., Mark Wang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35285/ --- (Updated July 6, 2015, 6:47 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (mesos) Diffs - src/log/catchup.cpp f7afc38916d0a7e57cdecb0da7ccb3901e726b90 src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 src/master/master.hpp fb4d6fac85e284987ec8fbf6949b5023875573fb src/slave/containerizer/containerizer.cpp b9ac94c59fdc229516d1ae193992ce0cf8ff96be src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26 Diff: https://reviews.apache.org/r/35285/diff/ Testing --- make check Thanks, Mark Wang
Re: Review Request 36024: Refactored OSNetUri tests for fetcher to avoid code copy/pasting.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36024/ --- (Updated July 6, 2015, 1:51 p.m.) Review request for mesos and Joris Van Remoortere. Changes --- Fixed compile error. Merged changes from parent review. Bugs: MESOS-2862 https://issues.apache.org/jira/browse/MESOS-2862 Repository: mesos Description --- Refactored OSNetUri tests for fetcher to avoid code copy/pasting. Diffs (updated) - src/tests/fetcher_tests.cpp ae10c420f7dddb8650377c91b5343591e8560392 Diff: https://reviews.apache.org/r/36024/diff/ Testing --- # This is a test case refactoring. $ make check Thanks, Artem Harutyunyan
Re: Review Request 36193: Improved Doxygen-Styleguide.
On July 6, 2015, 6:06 p.m., Joseph Wu wrote: Can you comment on the use of the `@copydoc` tag? I used it in the associated review (36141), but it isn't mentioned in the Doxygen style guide. I am somewhat hesitant about us trying to whitelist *everything* that is allowed. I much prefer Google's approach of if it's not forbidden, and is not obviously wrong, it should be allowed The risk is to having then to maintain huge (and expanding) list of allowed stuff, and people having to memorize them - ideally, one should have the general guidelines, as the ones here; a set of blacklisted features (brief and automated checks); and then a general freedom to use whatever is not forbidden. Trying to codify every little thing tends to stifle creativity and is not something we should strive for, IMHO. - Marco --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36193/#review90515 --- On July 6, 2015, 9:01 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36193/ --- (Updated July 6, 2015, 9:01 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Joseph Wu. Repository: mesos Description --- Improved Doxygen-Styleguide for clarifying discussions arising from https://reviews.apache.org/r/36141/. Diffs - docs/mesos-doxygen-style-guide.md 72156c72fc40f72740e57d47d0436c60f6d05bd7 Diff: https://reviews.apache.org/r/36193/diff/ Testing --- Checked rendered markdown. Thanks, Joerg Schad
Re: Review Request 36106: cgroups: added cpuacct subsystem
On July 1, 2015, 9:46 p.m., Timothy Chen wrote: src/linux/cgroups.cpp, line 2014 https://reviews.apache.org/r/36106/diff/1/?file=997647#file997647line2014 I don't think we usually define a inline function like this anywhere else, so curious to see what others think. Personally I don't think it provides any additional benefits here. Jojy Varghese wrote: The advantage is that otherwise we will end up copy-paste code of parsing at two places(line 0 and line 1) Timothy Chen wrote: What I meant to compare with is having a another named function defined. Jojy Varghese wrote: Having an external function is useful if its being used outside this function. Anonymous functions are meant to solve this. Timothy Chen wrote: No one else chimed in then I guess it's fine, I don't have any particular setback around this but this is most likely the first introduction of this. I won't be suprised if some other committer jumps on this. After adding lambda support, we can finally do things like this (YAY!) See the `Feel free to use auto when naming a lambda expression:` section of the [style guide](http://mesos.apache.org/documentation/latest/mesos-c++-style-guide/) Specifically, if the function is not mutating local state, but rather represents a functional transformation this is a nice use of a lambda. - Joris --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/#review90139 --- On July 6, 2015, 6:20 p.m., Jojy Varghese wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/ --- (Updated July 6, 2015, 6:20 p.m.) Review request for mesos, Ian Downes, Jie Yu, Joris Van Remoortere, and Timothy Chen. Bugs: MESOS-2961 https://issues.apache.org/jira/browse/MESOS-2961 Repository: mesos Description --- cgroups implementation does not have a cpuacct subsystem implementation as of today. Adding the implementation for stat function. Changes: - added Stats class to encapsulate cpuacct.stat data - added implementation for cpuacct::stats - added unit tests Jira: MESOS-2961 Diffs - src/linux/cgroups.hpp 73b98317880eea3d6a2ba37ac56d1f7e3600ba94 src/linux/cgroups.cpp 4c006d0c7382b940a83359d636c0d48952cdbb00 src/tests/cgroups_tests.cpp 475f48a474eea708f98d8c0300862351a2d4379a Diff: https://reviews.apache.org/r/36106/diff/ Testing --- make check Thanks, Jojy Varghese
Re: Review Request 23783: Missing Apache headers for libprocess
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23783/ --- (Updated July 6, 2015, 10:21 p.m.) Review request for mesos, Benjamin Hindman and Dominic Hamon. Repository: mesos Description --- Adding missing Apache licence headers on .cpp, .hpp and Makefiles Diffs (updated) - 3rdparty/libprocess/Makefile.am 358893c 3rdparty/libprocess/examples/example.cpp 3fb4ef5 3rdparty/libprocess/include/Makefile.am d01880c 3rdparty/libprocess/include/process/address.hpp 88946d5 3rdparty/libprocess/include/process/async.hpp 9af3cc0 3rdparty/libprocess/include/process/clock.hpp 8dc7be8 3rdparty/libprocess/include/process/collect.hpp c713c1b 3rdparty/libprocess/include/process/defer.hpp 7c04736 3rdparty/libprocess/include/process/deferred.hpp 3746d69 3rdparty/libprocess/include/process/delay.hpp 29e3532 3rdparty/libprocess/include/process/dispatch.hpp 617fd43 3rdparty/libprocess/include/process/event.hpp ad4a8f4 3rdparty/libprocess/include/process/executor.hpp 157a1d2 3rdparty/libprocess/include/process/filter.hpp aa0c91b 3rdparty/libprocess/include/process/future.hpp a9e765d 3rdparty/libprocess/include/process/gc.hpp e83c636 3rdparty/libprocess/include/process/gmock.hpp 0fd3f8c 3rdparty/libprocess/include/process/gtest.hpp 8518c38 3rdparty/libprocess/include/process/help.hpp 07e99f1 3rdparty/libprocess/include/process/http.hpp 8d9adc5 3rdparty/libprocess/include/process/id.hpp e586937 3rdparty/libprocess/include/process/io.hpp 6388770 3rdparty/libprocess/include/process/latch.hpp 9d010f0 3rdparty/libprocess/include/process/limiter.hpp 444aa1b 3rdparty/libprocess/include/process/logging.hpp 80b1e8f 3rdparty/libprocess/include/process/message.hpp c67c5e1 3rdparty/libprocess/include/process/metrics/counter.hpp f9cab39 3rdparty/libprocess/include/process/metrics/gauge.hpp 7d02cd5 3rdparty/libprocess/include/process/metrics/metric.hpp 616f060 3rdparty/libprocess/include/process/metrics/metrics.hpp aa44992 3rdparty/libprocess/include/process/metrics/timer.hpp 813115a 3rdparty/libprocess/include/process/mime.hpp 0abeac1 3rdparty/libprocess/include/process/mutex.hpp 37ea49a 3rdparty/libprocess/include/process/network.hpp f4192e0 3rdparty/libprocess/include/process/once.hpp 2b81df3 3rdparty/libprocess/include/process/owned.hpp 0541113 3rdparty/libprocess/include/process/pid.hpp e0a9fce 3rdparty/libprocess/include/process/process.hpp 59b50af 3rdparty/libprocess/include/process/profiler.hpp 86050b1 3rdparty/libprocess/include/process/protobuf.hpp 91493de 3rdparty/libprocess/include/process/queue.hpp 5be29db 3rdparty/libprocess/include/process/reap.hpp 5e5051a 3rdparty/libprocess/include/process/run.hpp a0d7286 3rdparty/libprocess/include/process/sequence.hpp d755b34 3rdparty/libprocess/include/process/shared.hpp d80fb7f 3rdparty/libprocess/include/process/socket.hpp 4d95183 3rdparty/libprocess/include/process/statistics.hpp 929fb8d 3rdparty/libprocess/include/process/subprocess.hpp 869e3d9 3rdparty/libprocess/include/process/system.hpp 4fb3c83 3rdparty/libprocess/include/process/time.hpp c5ab2a3 3rdparty/libprocess/include/process/timeout.hpp 5c46d70 3rdparty/libprocess/include/process/timer.hpp e5d71f6 3rdparty/libprocess/include/process/timeseries.hpp ec0ac67 3rdparty/libprocess/src/clock.cpp dd726c1 3rdparty/libprocess/src/config.hpp d5084bf 3rdparty/libprocess/src/decoder.hpp 85ce9e3 3rdparty/libprocess/src/encoder.hpp b898658 3rdparty/libprocess/src/event_loop.hpp af57fe2 3rdparty/libprocess/src/fatal.hpp 34314fd 3rdparty/libprocess/src/fatal.cpp b2934e0 3rdparty/libprocess/src/gate.hpp e5c9379 3rdparty/libprocess/src/http.cpp 0898335 3rdparty/libprocess/src/io.cpp 4944e28 3rdparty/libprocess/src/latch.cpp cba4dcd 3rdparty/libprocess/src/libev.hpp a0a2f49 3rdparty/libprocess/src/libev.cpp 2b8c68d 3rdparty/libprocess/src/libev_poll.cpp 6191be3 3rdparty/libprocess/src/libevent.hpp 47b93f1 3rdparty/libprocess/src/libevent.cpp 67e7501 3rdparty/libprocess/src/libevent_poll.cpp d0b8946 3rdparty/libprocess/src/libevent_ssl_socket.hpp 11c1b70 3rdparty/libprocess/src/libevent_ssl_socket.cpp 9424dd4 3rdparty/libprocess/src/logging.cpp 9134718 3rdparty/libprocess/src/metrics/metrics.cpp a97b613 3rdparty/libprocess/src/openssl.hpp 16f3d56 3rdparty/libprocess/src/openssl.cpp 118ce55 3rdparty/libprocess/src/openssl_util.hpp f855e27 3rdparty/libprocess/src/openssl_util.cpp cd38f17 3rdparty/libprocess/src/pid.cpp 979c370 3rdparty/libprocess/src/poll_socket.hpp a14d63f 3rdparty/libprocess/src/poll_socket.cpp 9cb4ef9 3rdparty/libprocess/src/process.cpp c3e1763 3rdparty/libprocess/src/process_reference.hpp eea9e4b
Re: Review Request 36005: Removed obsolete ec2 scripts.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36005/#review90552 --- Ship it! LGTM. Mind adding to CHANGELOG that these have been removed? - Vinod Kone On June 29, 2015, 7:20 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36005/ --- (Updated June 29, 2015, 7:20 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2640 https://issues.apache.org/jira/browse/MESOS-2640 Repository: mesos Description --- See summary. Diffs - Makefile.am dcd0cb474944ae9c882e6cbdb64a33b4be5b9083 configure.ac 92909580ea735a1ccffbe03a2eb44e3d07566488 docs/ec2-scripts.md 4866e2123eab0503bcc7fa130432bd41296d4b36 docs/tools.md f485abb9ff498c4faf51a4b84b23f438da3fc0d1 ec2/Makefile.am 8c64f485888df1599697eb181fc76aa83206da07 ec2/deploy.amazon64-old/root/ephemeral-hdfs/conf/core-site.xml 565f54dd933a89eb966fb8b01c589e1008d543bf ec2/deploy.amazon64-old/root/ephemeral-hdfs/conf/hadoop-env.sh 4e1e6991591e09f8860ab130948b0e787fce2b42 ec2/deploy.amazon64-old/root/ephemeral-hdfs/conf/hdfs-site.xml 43e68aa3e2ecbb53bd3b0a99d33b11aadda2cca7 ec2/deploy.amazon64-old/root/ephemeral-hdfs/conf/mapred-site.xml b1637dc8ce9cb49ebaa684a182cc8c94ac7c1b7b ec2/deploy.amazon64-old/root/ephemeral-hdfs/conf/masters d26a1943aeb47ceeff904e593793e08c07eee9cc ec2/deploy.amazon64-old/root/ephemeral-hdfs/conf/slaves 05f969e0c667629041099e97dd8d6a8168e211a1 ec2/deploy.amazon64-old/root/mesos-ec2/cluster-url fcf8b41aed4b47a7b78d0c5a0042678f40059a5f ec2/deploy.amazon64-old/root/mesos-ec2/copy-dir 02b6e642f6348b3b4cdc4ff5adb2b0070c1dc1cc ec2/deploy.amazon64-old/root/mesos-ec2/create-swap 9ab32f841411f1053c29f0fa48f0cf2bbf468e82 ec2/deploy.amazon64-old/root/mesos-ec2/hadoop-framework-conf/core-site.xml 818ed1030d3dd97467c4d4c83735cb7bda5afa80 ec2/deploy.amazon64-old/root/mesos-ec2/hadoop-framework-conf/hadoop-env.sh ca227b460e9d6b89a2fe3692a74eb1c267bc7b7f ec2/deploy.amazon64-old/root/mesos-ec2/hadoop-framework-conf/mapred-site.xml 0ffa92f115d66adc9e370f4030850216545ade38 ec2/deploy.amazon64-old/root/mesos-ec2/haproxy+apache/haproxy.config.template 957c3f6a6b2a2f658e076337d88e69447b2a3341 ec2/deploy.amazon64-old/root/mesos-ec2/hypertable/Capfile fd5921e57d7985e128622ed132c9a24c7cd21d1b ec2/deploy.amazon64-old/root/mesos-ec2/hypertable/hypertable.cfg b4d5b7475fab3d4a842e4b0f459abc5ca316996a ec2/deploy.amazon64-old/root/mesos-ec2/masters c531652dc1ef9e6d6ca73cec389b6433f35ae9cf ec2/deploy.amazon64-old/root/mesos-ec2/mesos-daemon 20ec099f521c97af569b98555ef67964cdcf97cc ec2/deploy.amazon64-old/root/mesos-ec2/redeploy-mesos 941d783d82f0708a6da0f4677c3364537dfded63 ec2/deploy.amazon64-old/root/mesos-ec2/setup 2165f90139d179d57b3d3b243d96fbea20a50b2c ec2/deploy.amazon64-old/root/mesos-ec2/setup-slave 436f417bc5a746ad74cc88c27e630a91d55b0b23 ec2/deploy.amazon64-old/root/mesos-ec2/setup-torque 2ac8fd3546063d3ba391147383de53b7824c7c8c ec2/deploy.amazon64-old/root/mesos-ec2/slaves 05f969e0c667629041099e97dd8d6a8168e211a1 ec2/deploy.amazon64-old/root/mesos-ec2/ssh-no-keychecking 3daf46fe988b4b7dac4879f4ea31b6ffca1dc02a ec2/deploy.amazon64-old/root/mesos-ec2/start-hypertable af16c2d7bd615cb0c98f6ba65ff5c69859678850 ec2/deploy.amazon64-old/root/mesos-ec2/start-mesos cc309cc44ecf5b67536f275083dca22948926f6b ec2/deploy.amazon64-old/root/mesos-ec2/stop-hypertable 7280dc11bfc53ae84b7ecaba34c84810461ed7f4 ec2/deploy.amazon64-old/root/mesos-ec2/stop-mesos 9fdb8753dffc5115f94582753e0860538be6232b ec2/deploy.amazon64-old/root/mesos-ec2/zoo efc961b502f30a6f187f7e50c2b92b302203d89e ec2/deploy.amazon64-old/root/persistent-hdfs/conf/core-site.xml b23aef258a13e4bc4cbf27099f7aa68419ab0825 ec2/deploy.amazon64-old/root/persistent-hdfs/conf/hadoop-env.sh b38ba01817e3b9c9715a476ecb692bac68983f50 ec2/deploy.amazon64-old/root/persistent-hdfs/conf/hdfs-site.xml ec000cb2f306232a29cf701b10f230685d2662c9 ec2/deploy.amazon64-old/root/persistent-hdfs/conf/mapred-site.xml b1637dc8ce9cb49ebaa684a182cc8c94ac7c1b7b ec2/deploy.amazon64-old/root/persistent-hdfs/conf/masters d26a1943aeb47ceeff904e593793e08c07eee9cc ec2/deploy.amazon64-old/root/persistent-hdfs/conf/slaves 05f969e0c667629041099e97dd8d6a8168e211a1 ec2/deploy.amazon64-old/root/spark/conf/spark-env.sh 86da7f832d03dfc6cc0be5e479fea7d8f92541e0 ec2/deploy.amazon64/root/ephemeral-hdfs/conf/core-site.xml 565f54dd933a89eb966fb8b01c589e1008d543bf ec2/deploy.amazon64/root/ephemeral-hdfs/conf/hadoop-env.sh
Re: Review Request 23784: Missing Apache headers for stout
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23784/ --- (Updated July 6, 2015, 9:49 p.m.) Review request for mesos, Benjamin Hindman and Dominic Hamon. Repository: mesos Description --- Adding missing Apache licence headers on .cpp, .hpp and Makefiles Diffs (updated) - 3rdparty/libprocess/3rdparty/stout/Makefile.am 9df5de4 3rdparty/libprocess/3rdparty/stout/include/Makefile.am ff6fb28 3rdparty/libprocess/3rdparty/stout/tests/bits_tests.cpp c47bd13 3rdparty/libprocess/3rdparty/stout/tests/bytes_tests.cpp dc05284 3rdparty/libprocess/3rdparty/stout/tests/cache_tests.cpp f8a0ba5 3rdparty/libprocess/3rdparty/stout/tests/duration_tests.cpp 724c5fe 3rdparty/libprocess/3rdparty/stout/tests/error_tests.cpp 35a62b1 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 2a6f67b 3rdparty/libprocess/3rdparty/stout/tests/gzip_tests.cpp 2211f31 3rdparty/libprocess/3rdparty/stout/tests/hashmap_tests.cpp 4a8176b 3rdparty/libprocess/3rdparty/stout/tests/hashset_tests.cpp 97a7167 3rdparty/libprocess/3rdparty/stout/tests/interval_tests.cpp 1fa8fc1 3rdparty/libprocess/3rdparty/stout/tests/ip_tests.cpp c50480b 3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 0011f08 3rdparty/libprocess/3rdparty/stout/tests/linkedhashmap_tests.cpp 0644d99 3rdparty/libprocess/3rdparty/stout/tests/mac_tests.cpp b80fff9 3rdparty/libprocess/3rdparty/stout/tests/main.cpp f5f20ee 3rdparty/libprocess/3rdparty/stout/tests/multimap_tests.cpp 11f3bf7 3rdparty/libprocess/3rdparty/stout/tests/none_tests.cpp 1c1f8be 3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp 4a0f60b 3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp fee942c 3rdparty/libprocess/3rdparty/stout/tests/os/signals_tests.cpp 6d2d3d5 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 4a01d94 3rdparty/libprocess/3rdparty/stout/tests/proc_tests.cpp dd3700d 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp b3ce131 3rdparty/libprocess/3rdparty/stout/tests/set_tests.cpp 3ddba94 3rdparty/libprocess/3rdparty/stout/tests/some_tests.cpp 0689446 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 9733b2e 3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp 93339eb 3rdparty/libprocess/3rdparty/stout/tests/uuid_tests.cpp 7ac8fc0 Diff: https://reviews.apache.org/r/23784/diff/ Testing --- Thanks, Isabel Jimenez
Re: Review Request 36205: Added AWAIT_EXPECT_TRUE and AWAIT_EXPECT_FALSE macros.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36205/#review90534 --- Patch looks great! Reviews applied: [36205] All tests passed. - Mesos ReviewBot On July 6, 2015, 3:44 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36205/ --- (Updated July 6, 2015, 3:44 p.m.) Review request for mesos, Bernd Mathiske, Joerg Schad, and Till Toenshoff. Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/include/process/gtest.hpp 8518c38c9cbf62a098b43b4d0b6d80e561b17ad4 Diff: https://reviews.apache.org/r/36205/diff/ Testing --- Used in tests in follow up patch. Thanks, Alexander Rojas
Re: Review Request 35287: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35287/ --- (Updated July 6, 2015, 6:45 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout) Diffs (updated) - 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 3d9b7a7568de6734097733f4e6d59acba629b849 3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 784104f26ceee2ef90709056a5f4428d48390c36 3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp d87cf22dbb98e1ac99f129668ad984a1542e4ec9 Diff: https://reviews.apache.org/r/35287/diff/ Testing --- make check Thanks, Mark Wang
Re: Review Request 35286: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (libprocess)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35286/#review90546 --- Ship it! Ship It! - Benjamin Hindman On July 6, 2015, 6:46 p.m., Mark Wang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35286/ --- (Updated July 6, 2015, 6:46 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (libprocess) Diffs - 3rdparty/libprocess/src/process.cpp 883776a6d87f3f14d04e2d574b0e0baa469af579 Diff: https://reviews.apache.org/r/35286/diff/ Testing --- make check Thanks, Mark Wang
Re: Review Request 35287: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35287/#review90545 --- Ship it! Ship It! - Benjamin Hindman On July 6, 2015, 6:46 p.m., Mark Wang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35287/ --- (Updated July 6, 2015, 6:46 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout) Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 3d9b7a7568de6734097733f4e6d59acba629b849 3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 784104f26ceee2ef90709056a5f4428d48390c36 3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp d87cf22dbb98e1ac99f129668ad984a1542e4ec9 Diff: https://reviews.apache.org/r/35287/diff/ Testing --- make check Thanks, Mark Wang
Re: Review Request 35755: Changed fetcher to handle leading whitespace in URLs.
On July 5, 2015, 8:02 p.m., Benjamin Hindman wrote: Joris Van Remoortere wrote: Hey Ben, Great questions in the latter half of your review. It turns out that the original test was malformed. It happened to pass because there is also a help process that responds to the `/help` endpoint. The original intent of the test, however, was to ensure that the newly added `/help` endpoint would work. That newly added enpoint was actually never being hit. The name is being passed through to the HTTPProcess constructor explicitly to make the url at which the test endpoint is being hit stable. If the default `Process` constructor is used, then we end up using the auto-incrementing ids which can be hard to deterministically hit. I agree that we should stick with a single endpoint as you suggested; however, we need to change the name to something unique to avoid this test from passing accidentally when the intended path is broken. We definitely need a comment as to why we're explicitly specifying the name of the process (as described above). I have seen a couple of people bang their heads against this, so let's add cleared documentation to the headers / doxygen as well! Ben suggested to use MOCK_METHOD/EXPECT_CALL to make sure that the right callback is being invoked. As for the named processes, Ben suggested to use process.self().id instead of having named processes. - Artem --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35755/#review90423 --- On July 6, 2015, 9:03 a.m., Artem Harutyunyan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35755/ --- (Updated July 6, 2015, 9:03 a.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2862 https://issues.apache.org/jira/browse/MESOS-2862 Repository: mesos Description --- Fixes MESOS-2862 Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd src/tests/fetcher_tests.cpp ae10c420f7dddb8650377c91b5343591e8560392 Diff: https://reviews.apache.org/r/35755/diff/ Testing --- - make check - added a test case for fetcher Thanks, Artem Harutyunyan
Re: Review Request 35286: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (libprocess)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35286/ --- (Updated July 6, 2015, 6:45 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (libprocess) Diffs (updated) - 3rdparty/libprocess/src/process.cpp 883776a6d87f3f14d04e2d574b0e0baa469af579 Diff: https://reviews.apache.org/r/35286/diff/ Testing --- make check Thanks, Mark Wang
Re: Review Request 35285: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (mesos)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35285/#review90547 --- Ship it! Ship It! - Benjamin Hindman On July 6, 2015, 6:47 p.m., Mark Wang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35285/ --- (Updated July 6, 2015, 6:47 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (mesos) Diffs - src/log/catchup.cpp f7afc38916d0a7e57cdecb0da7ccb3901e726b90 src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 src/master/master.hpp fb4d6fac85e284987ec8fbf6949b5023875573fb src/slave/containerizer/containerizer.cpp b9ac94c59fdc229516d1ae193992ce0cf8ff96be src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26 Diff: https://reviews.apache.org/r/35285/diff/ Testing --- make check Thanks, Mark Wang
Review Request 36226: Missing Apache headers for libprocess 3rdparty
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36226/ --- Review request for mesos and Benjamin Hindman. Repository: mesos-incubating Description --- Adding missing Apache licence header Diffs - 3rdparty/libprocess/3rdparty/Makefile.am 519e38c Diff: https://reviews.apache.org/r/36226/diff/ Testing --- Thanks, Isabel Jimenez
Re: Review Request 36106: cgroups: added cpuacct subsystem
On July 2, 2015, 11:32 p.m., Ben Mahler wrote: src/linux/cgroups.hpp, lines 443-472 https://reviews.apache.org/r/36106/diff/1/?file=997646#file997646line443 Thanks! (1) Do you mind updating my TODO on cgroups::stat() to reflect that cpuacct::stat is implemented? (2) Can we just make this a simple struct with two non-const fields and remove the parse method and getters? Keep it simple and small, if we want immutability we can just have a 'const Stat' rather than forcing it on the caller :) (3) Any reason not to use 'Duration' for these fields? Jojy Varghese wrote: 1) Absolutely I can. 2) I wanted to reflect the semantics of the stats call. When you make a stats call, the data you get is immutable. By forcing external const, it would imply that the value is immutable. 3) Duration is a period ( i think) and the values here are absolute values derived from ticks. Regarding 3): Duration indeed represents an amount of time, rather than a specific point in time. I believe this is what we intend to represent in this code as well, right? The comments for your functions suggest an amount of time rather than a point in time. - Joris --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/#review90288 --- On July 6, 2015, 6:20 p.m., Jojy Varghese wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/ --- (Updated July 6, 2015, 6:20 p.m.) Review request for mesos, Ian Downes, Jie Yu, Joris Van Remoortere, and Timothy Chen. Bugs: MESOS-2961 https://issues.apache.org/jira/browse/MESOS-2961 Repository: mesos Description --- cgroups implementation does not have a cpuacct subsystem implementation as of today. Adding the implementation for stat function. Changes: - added Stats class to encapsulate cpuacct.stat data - added implementation for cpuacct::stats - added unit tests Jira: MESOS-2961 Diffs - src/linux/cgroups.hpp 73b98317880eea3d6a2ba37ac56d1f7e3600ba94 src/linux/cgroups.cpp 4c006d0c7382b940a83359d636c0d48952cdbb00 src/tests/cgroups_tests.cpp 475f48a474eea708f98d8c0300862351a2d4379a Diff: https://reviews.apache.org/r/36106/diff/ Testing --- make check Thanks, Jojy Varghese
Re: Review Request 23749: Missing Apache headers for mesos
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23749/ --- (Updated July 6, 2015, 10:25 p.m.) Review request for mesos and Benjamin Hindman. Repository: mesos Description --- Adding missing Apache licence headers on .cpp, .hpp and Makefiles Diffs (updated) - src/cli/mesos.cpp 0c315f7 src/files/files.cpp 3a62ee7 src/hdfs/hdfs.hpp cba386a src/slave/state.cpp fab47a7 src/state/in_memory.hpp 2040618 src/state/in_memory.cpp ce04e47 src/state/leveldb.hpp 53447c6 src/state/leveldb.cpp 14a1807 src/state/log.hpp a0ca4f8 src/state/log.cpp 326f3a7 src/state/zookeeper.hpp 1a8483d src/state/zookeeper.cpp 9151d55 src/tests/active_user_test_helper.cpp c32d467 src/zookeeper/authentication.hpp 7b6b767 src/zookeeper/authentication.cpp ccd6bf7 src/zookeeper/contender.hpp 6529245 src/zookeeper/contender.cpp 3255ef0 src/zookeeper/detector.hpp e41158e src/zookeeper/detector.cpp 5305161 src/zookeeper/group.hpp 9246130 src/zookeeper/group.cpp 01066e3 src/zookeeper/watcher.hpp 1e347ed Diff: https://reviews.apache.org/r/23749/diff/ Testing --- Thanks, Isabel Jimenez
Re: Review Request 23749: Missing Apache headers for mesos
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23749/ --- (Updated July 6, 2015, 10:33 p.m.) Review request for mesos and Benjamin Hindman. Repository: mesos Description --- Adding missing Apache licence headers on .cpp, .hpp and Makefiles Diffs (updated) - src/cli/mesos.cpp 0c315f7 src/files/files.cpp 3a62ee7 src/hdfs/hdfs.hpp cba386a src/slave/state.cpp fab47a7 src/state/in_memory.hpp 2040618 src/state/in_memory.cpp ce04e47 src/state/leveldb.hpp 53447c6 src/state/leveldb.cpp 14a1807 src/state/log.hpp a0ca4f8 src/state/log.cpp 326f3a7 src/state/zookeeper.hpp 1a8483d src/state/zookeeper.cpp 9151d55 src/tests/active_user_test_helper.cpp c32d467 src/zookeeper/authentication.hpp 7b6b767 src/zookeeper/authentication.cpp ccd6bf7 src/zookeeper/contender.hpp 6529245 src/zookeeper/contender.cpp 3255ef0 src/zookeeper/detector.hpp e41158e src/zookeeper/detector.cpp 5305161 src/zookeeper/group.hpp 9246130 src/zookeeper/group.cpp 01066e3 src/zookeeper/watcher.hpp 1e347ed Diff: https://reviews.apache.org/r/23749/diff/ Testing --- Thanks, Isabel Jimenez
Review Request 36217: Adding http validations to master call request validations
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36217/ --- Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and Vinod Kone. Bugs: MESOS-2860 https://issues.apache.org/jira/browse/MESOS-2860 Repository: mesos-incubating Description --- Adding helper validations for http requests Diffs - src/Makefile.am addb63f src/common/http_validation.hpp PRE-CREATION src/common/http_validation.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36217/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 36207: Unified line wrapping for defines in gtest.hpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36207/#review90549 --- Patch looks great! Reviews applied: [36207] All tests passed. - Mesos ReviewBot On July 6, 2015, 5:19 p.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36207/ --- (Updated July 6, 2015, 5:19 p.m.) Review request for mesos and Alexander Rojas. Repository: mesos Description --- This updates a bunch of defines within libprocess' gtest.hpp to unify their wrapping towards a consistent style. Diffs - 3rdparty/libprocess/include/process/gtest.hpp 8518c38c9cbf62a098b43b4d0b6d80e561b17ad4 Diff: https://reviews.apache.org/r/36207/diff/ Testing --- make check Thanks, Till Toenshoff
Re: Review Request 36193: Improved Doxygen-Styleguide.
On July 6, 2015, 6:39 p.m., Joseph Wu wrote: docs/mesos-doxygen-style-guide.md, line 102 https://reviews.apache.org/r/36193/diff/1/?file=999588#file999588line102 This isn't part of the diff, but should this `@note` tag be replaced with a `**NOTE:**`? Good catch! But not part of this review, do you want to create another patch for this? - Joerg --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36193/#review90525 --- On July 6, 2015, 9:01 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36193/ --- (Updated July 6, 2015, 9:01 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Joseph Wu. Repository: mesos Description --- Improved Doxygen-Styleguide for clarifying discussions arising from https://reviews.apache.org/r/36141/. Diffs - docs/mesos-doxygen-style-guide.md 72156c72fc40f72740e57d47d0436c60f6d05bd7 Diff: https://reviews.apache.org/r/36193/diff/ Testing --- Checked rendered markdown. Thanks, Joerg Schad
Re: Review Request 35286: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (libprocess)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35286/ --- (Updated July 6, 2015, 6:33 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (libprocess) Diffs - 3rdparty/libprocess/src/process.cpp 883776a6d87f3f14d04e2d574b0e0baa469af579 3rdparty/libprocess/src/subprocess.cpp 5b41f0d88fcab93d51d5c503f69faccedc210868 Diff: https://reviews.apache.org/r/35286/diff/ Testing --- make check Thanks, Mark Wang
Re: Review Request 36214: Fix running docker executor tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36214/ --- (Updated July 6, 2015, 8:40 p.m.) Review request for mesos, Bernd Mathiske, Joerg Schad, and Till Toenshoff. Bugs: MESOS-2996 https://issues.apache.org/jira/browse/MESOS-2996 Repository: mesos Description --- When running on CentOS 7.1 it didn't have /bin in it's PATH and therefore it failed to find test-executor that is in /bin/ in the container. This works on other systems since the tests inherits the environment variables from the system and /bin usually is in the PATH. Diffs - src/tests/docker_containerizer_tests.cpp d4ccb0b30fe27980846d913e292d2e18fd3d1055 Diff: https://reviews.apache.org/r/36214/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 36193: Improved Doxygen-Styleguide.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36193/#review90529 --- docs/mesos-doxygen-style-guide.md (line 17) https://reviews.apache.org/r/36193/#comment143651 It might be worthwhile to emphasize that markdown syntax can/should be used in with the comment blocks. I think Javadocs generally use plain HTML for links, tables, etc. - Joseph Wu On July 6, 2015, 2:01 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36193/ --- (Updated July 6, 2015, 2:01 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Joseph Wu. Repository: mesos Description --- Improved Doxygen-Styleguide for clarifying discussions arising from https://reviews.apache.org/r/36141/. Diffs - docs/mesos-doxygen-style-guide.md 72156c72fc40f72740e57d47d0436c60f6d05bd7 Diff: https://reviews.apache.org/r/36193/diff/ Testing --- Checked rendered markdown. Thanks, Joerg Schad
Re: Review Request 36106: cgroups: added cpuacct subsystem
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36106/#review90581
---
Hey Jojy, great work so far! Thanks for writing up the JIRA first ;-)
Just doing a first review as I familiarize myself with the code.
I think the pattern and style feedback is valuable regardless! :-D
Regarding the debate around `Duration`: regardless of whether `Duration` is
'overkill', I think there is value in consistency for the sake of readability.
Many of our utility functions use / return `Duration`s and so a user of this
code will very likely be familiar with the types of things they can do with a
`Duration`.
src/linux/cgroups.cpp (lines 2006 - 2010)
https://reviews.apache.org/r/36106/#comment143679
Even though these thoughts are related, if the assignment statement doesn't
fit on a single line, we tend to leave a blank line between the assignment and
the if block.
```
Tryhashmapstring, uint64_t stats =
cgroups::stat(hierarchy, cgroup, cpuacct.stat);
if (!stats.isSome()) {
return Error(stats.error());
}
```
Same elsewhere in this patch.
Not yours: We can also kill the trailing space for the template
specialization :-)
src/linux/cgroups.cpp (line 2008)
https://reviews.apache.org/r/36106/#comment143686
`stats.isError()` instead of requiring the negation.
Here and elsewhere.
src/linux/cgroups.cpp (lines 2016 - 2019)
https://reviews.apache.org/r/36106/#comment143683
Can you add a comment here as to why it's ok to cache this value? (As in
this value is not dynamic)
What does it mean for this value to be `0`? Should we be aborting here?
If we're cache the value, should we only check it once? If so:
Maybe it's worth pulling this out into a static function within this module
so that we can use it elsewhere. This value seems generally usefull.
What do you think?
src/linux/cgroups.cpp (line 2029)
https://reviews.apache.org/r/36106/#comment143684
We leave a space after c-style casts
`(double) stats`
Here and elsehwere.
src/tests/cgroups_tests.cpp (line 1192)
https://reviews.apache.org/r/36106/#comment143688
We currently don't allow namespace aliasing. I understand this isn't well
documented, sorry about that!
src/tests/cgroups_tests.cpp (line 1194)
https://reviews.apache.org/r/36106/#comment143687
Can make this `const`.
Side note: This file has a significant amount of `std::string` and
`std::vector` and trailing spaces for template specializations.
Maybe we can do a clean-up pass after this patch lands :-)
- Joris Van Remoortere
On July 6, 2015, 6:20 p.m., Jojy Varghese wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36106/
---
(Updated July 6, 2015, 6:20 p.m.)
Review request for mesos, Ian Downes, Jie Yu, Joris Van Remoortere, and
Timothy Chen.
Bugs: MESOS-2961
https://issues.apache.org/jira/browse/MESOS-2961
Repository: mesos
Description
---
cgroups implementation does not have a cpuacct subsystem implementation as of
today. Adding the implementation for stat function.
Changes:
- added Stats class to encapsulate cpuacct.stat data
- added implementation for cpuacct::stats
- added unit tests
Jira: MESOS-2961
Diffs
-
src/linux/cgroups.hpp 73b98317880eea3d6a2ba37ac56d1f7e3600ba94
src/linux/cgroups.cpp 4c006d0c7382b940a83359d636c0d48952cdbb00
src/tests/cgroups_tests.cpp 475f48a474eea708f98d8c0300862351a2d4379a
Diff: https://reviews.apache.org/r/36106/diff/
Testing
---
make check
Thanks,
Jojy Varghese
Re: Review Request 35986: Allow slave attributes flag take a value with ':'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35986/#review90623 --- Very good, but there are some subtleties that we should not ignore regarding empty names/values and multiple attributes with colons. src/common/attributes.cpp (lines 150 - 152) https://reviews.apache.org/r/35986/#comment143744 There's a subtle difference in behavior between strings::tokenize and strings::split. For tokenize, Empty tokens will not be included in the result. whereas for split, Empty tokens are allowed in the result. so you need to test not only that `pairs.size() == 2`, but also that `!pairs[0].empty()` and `!pairs[1].empty()`. Let's create unit tests for handling `:foo` and `foo:`. src/tests/attributes_tests.cpp (line 55) https://reviews.apache.org/r/35986/#comment143738 Would love to see this test multiple attributes, so we can be sure that subsequent colons still work as expected in situations like `parse(attr1:foo:bar;attr2:baz:qux)`. - Adam B On June 28, 2015, 6:49 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35986/ --- (Updated June 28, 2015, 6:49 a.m.) Review request for mesos and Isabel Jimenez. Bugs: MESOS-2868 https://issues.apache.org/jira/browse/MESOS-2868 Repository: mesos Description --- Allow slave attributes flag take a value with ':'. Diffs - src/common/attributes.cpp aab114e1a5932e3f218b850e1afc7f2ef0f10e21 src/tests/attributes_tests.cpp 2e10eaf3c33e418603ea19090bb0bf9179af71f7 Diff: https://reviews.apache.org/r/35986/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 23783: Missing Apache headers for libprocess
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23783/#review90624 --- Patch looks great! Reviews applied: [23783] All tests passed. - Mesos ReviewBot On July 6, 2015, 10:21 p.m., Isabel Jimenez wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23783/ --- (Updated July 6, 2015, 10:21 p.m.) Review request for mesos, Benjamin Hindman and Dominic Hamon. Repository: mesos Description --- Adding missing Apache licence headers on .cpp, .hpp and Makefiles Diffs - 3rdparty/libprocess/Makefile.am 358893c 3rdparty/libprocess/examples/example.cpp 3fb4ef5 3rdparty/libprocess/include/Makefile.am d01880c 3rdparty/libprocess/include/process/address.hpp 88946d5 3rdparty/libprocess/include/process/async.hpp 9af3cc0 3rdparty/libprocess/include/process/clock.hpp 8dc7be8 3rdparty/libprocess/include/process/collect.hpp c713c1b 3rdparty/libprocess/include/process/defer.hpp 7c04736 3rdparty/libprocess/include/process/deferred.hpp 3746d69 3rdparty/libprocess/include/process/delay.hpp 29e3532 3rdparty/libprocess/include/process/dispatch.hpp 617fd43 3rdparty/libprocess/include/process/event.hpp ad4a8f4 3rdparty/libprocess/include/process/executor.hpp 157a1d2 3rdparty/libprocess/include/process/filter.hpp aa0c91b 3rdparty/libprocess/include/process/future.hpp a9e765d 3rdparty/libprocess/include/process/gc.hpp e83c636 3rdparty/libprocess/include/process/gmock.hpp 0fd3f8c 3rdparty/libprocess/include/process/gtest.hpp 8518c38 3rdparty/libprocess/include/process/help.hpp 07e99f1 3rdparty/libprocess/include/process/http.hpp 8d9adc5 3rdparty/libprocess/include/process/id.hpp e586937 3rdparty/libprocess/include/process/io.hpp 6388770 3rdparty/libprocess/include/process/latch.hpp 9d010f0 3rdparty/libprocess/include/process/limiter.hpp 444aa1b 3rdparty/libprocess/include/process/logging.hpp 80b1e8f 3rdparty/libprocess/include/process/message.hpp c67c5e1 3rdparty/libprocess/include/process/metrics/counter.hpp f9cab39 3rdparty/libprocess/include/process/metrics/gauge.hpp 7d02cd5 3rdparty/libprocess/include/process/metrics/metric.hpp 616f060 3rdparty/libprocess/include/process/metrics/metrics.hpp aa44992 3rdparty/libprocess/include/process/metrics/timer.hpp 813115a 3rdparty/libprocess/include/process/mime.hpp 0abeac1 3rdparty/libprocess/include/process/mutex.hpp 37ea49a 3rdparty/libprocess/include/process/network.hpp f4192e0 3rdparty/libprocess/include/process/once.hpp 2b81df3 3rdparty/libprocess/include/process/owned.hpp 0541113 3rdparty/libprocess/include/process/pid.hpp e0a9fce 3rdparty/libprocess/include/process/process.hpp 59b50af 3rdparty/libprocess/include/process/profiler.hpp 86050b1 3rdparty/libprocess/include/process/protobuf.hpp 91493de 3rdparty/libprocess/include/process/queue.hpp 5be29db 3rdparty/libprocess/include/process/reap.hpp 5e5051a 3rdparty/libprocess/include/process/run.hpp a0d7286 3rdparty/libprocess/include/process/sequence.hpp d755b34 3rdparty/libprocess/include/process/shared.hpp d80fb7f 3rdparty/libprocess/include/process/socket.hpp 4d95183 3rdparty/libprocess/include/process/statistics.hpp 929fb8d 3rdparty/libprocess/include/process/subprocess.hpp 869e3d9 3rdparty/libprocess/include/process/system.hpp 4fb3c83 3rdparty/libprocess/include/process/time.hpp c5ab2a3 3rdparty/libprocess/include/process/timeout.hpp 5c46d70 3rdparty/libprocess/include/process/timer.hpp e5d71f6 3rdparty/libprocess/include/process/timeseries.hpp ec0ac67 3rdparty/libprocess/src/clock.cpp dd726c1 3rdparty/libprocess/src/config.hpp d5084bf 3rdparty/libprocess/src/decoder.hpp 85ce9e3 3rdparty/libprocess/src/encoder.hpp b898658 3rdparty/libprocess/src/event_loop.hpp af57fe2 3rdparty/libprocess/src/fatal.hpp 34314fd 3rdparty/libprocess/src/fatal.cpp b2934e0 3rdparty/libprocess/src/gate.hpp e5c9379 3rdparty/libprocess/src/http.cpp 0898335 3rdparty/libprocess/src/io.cpp 4944e28 3rdparty/libprocess/src/latch.cpp cba4dcd 3rdparty/libprocess/src/libev.hpp a0a2f49 3rdparty/libprocess/src/libev.cpp 2b8c68d 3rdparty/libprocess/src/libev_poll.cpp 6191be3 3rdparty/libprocess/src/libevent.hpp 47b93f1 3rdparty/libprocess/src/libevent.cpp 67e7501 3rdparty/libprocess/src/libevent_poll.cpp d0b8946 3rdparty/libprocess/src/libevent_ssl_socket.hpp 11c1b70 3rdparty/libprocess/src/libevent_ssl_socket.cpp 9424dd4 3rdparty/libprocess/src/logging.cpp 9134718 3rdparty/libprocess/src/metrics/metrics.cpp a97b613
Re: Review Request 36217: Adding http validations to master call request validations
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36217/#review90625 --- Thanks Isabel! Left some comments per our chat. Also, can we inline this into the /call handler? After adjusting this, it's going to become less generally useful, and the 'Accept' logic could even become subsumed by a more intelligent route(). src/common/http_validation.cpp (lines 39 - 64) https://reviews.apache.org/r/36217/#comment143749 Couple things per our conversation: (1) If we don't have the 'Accept' header, the spec says to assume they accept everything (i.e. `*/*`), so let's reply with json in this case to make it easier for folks. Alternatively, if you want to be more cautious, we could reject these but I'm guessing this is going to surprise many people given that no `Accept` and `Accept = */*` are semantically the same. (2) The way you're checking 'Accept' is not complete (look at the RFC and Request::accepts, which implements the 'Accept-Encoding' logic, (we should rename this to Request::acceptsEncoding). At the very least, let's add a TODO for now that this is incomplete. (3) Why is the 'Connection' header required? For HTTP 1.1, there is only a 'close' value, and if ommitted, we can assume a persistent connection. So we don't need to reject requests without this. This is all the more reason to have some types around these common headers (e.g. Connection enum), but let's punt on that. :) - Ben Mahler On July 6, 2015, 11:31 p.m., Isabel Jimenez wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36217/ --- (Updated July 6, 2015, 11:31 p.m.) Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and Vinod Kone. Bugs: MESOS-2860 https://issues.apache.org/jira/browse/MESOS-2860 Repository: mesos-incubating Description --- Adding helper validations for http requests Diffs - src/Makefile.am addb63f src/common/http_validation.hpp PRE-CREATION src/common/http_validation.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36217/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 34943: Added validation to flags.
Wasn't sure if you guys saw this?
On Wed, Jun 24, 2015 at 3:22 PM, Ben Mahler benjamin.mah...@gmail.com
wrote:
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34943/
3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp
https://reviews.apache.org/r/34943/diff/5/?file=986809#file986809line790
(Diff
revision 5)
inline TryNothing FlagsBase::load(
748
// Validate the flags value.
749
//
750
// TODO(benh): Consider validating all flags at the same time in
751
// order to provide more feedback rather than requiring a user to
752
// fix one at a time.
753
foreachvalue (const Flag flag, flags_) {
754
OptionError error = flag.validate(*this);
755
if (error.isSome()) {
756
return error.get();
757
}
758
}
Seems nice to have this print the same kind of error strings as above, so
that the caller can just compose a specific error, rather than having to
explicitly include the flag name:
foreachvalue (const Flag flag, flags_) {
OptionError error = flag.validate(*this);
if (error.isSome()) {
return Invalid value for ' + flag.name + ': + error.get();
}
}
Then the caller can just do:
add(Flags::duration,
duration,
,
DEFAULT_DURATION,
[](const Duration value) - OptionError {
if (value Seconds(1)) {
return Error(Must be greater than + stringify(Seconds(1)));
}
return None();
});
vs.
add(Flags::duration,
duration,
,
DEFAULT_DURATION,
[](const Duration value) - OptionError {
if (value Seconds(1)) {
return Error(--duration must be greater than +
stringify(Seconds(1)));
}
return None();
});
Thoughts?
Thoughts?
- Ben Mahler
On June 18th, 2015, 3:34 p.m. UTC, Benjamin Hindman wrote:
Review request for mesos and Niklas Nielsen.
By Benjamin Hindman.
*Updated June 18, 2015, 3:34 p.m.*
*Repository: * mesos
Description
Also refactored existing 'lambda::bind' arguments to use C++11 lambdas,
enabling us to get rid of our loader and stringifier functors.
Also, followed up as requested of some reviewers and added initial
documentation for flags in https://reviews.apache.org/r/35611.
Testing
make check
Diffs
- 3rdparty/libprocess/3rdparty/stout/include/Makefile.am
(6ac2f04a6a1d8db1725972a3b4b46a0dd734566f)
- 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp
(87606d860dea3235ec70d127d3379065d42dc90b)
- 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp
(ee855da6128c2d671eb2fc7eaa2c0aab2341aebb)
- 3rdparty/libprocess/3rdparty/stout/include/stout/flags/loader.hpp
(51d3ab020bd2bae1f12b66cac0da5383c813d5d7)
- 3rdparty/libprocess/3rdparty/stout/include/stout/flags/stringifier.hpp
(fda5ae1328a23a4371475652f921341dce7448d5)
- 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp
(80450185f60c5b273face490e0bb9e695b0cb984)
View Diff https://reviews.apache.org/r/34943/diff/
Re: Review Request 36226: Missing Apache headers for libprocess 3rdparty
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36226/#review90621 --- Patch looks great! Reviews applied: [36226] All tests passed. - Mesos ReviewBot On July 6, 2015, 10:17 p.m., Isabel Jimenez wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36226/ --- (Updated July 6, 2015, 10:17 p.m.) Review request for mesos and Benjamin Hindman. Repository: mesos-incubating Description --- Adding missing Apache licence header Diffs - 3rdparty/libprocess/3rdparty/Makefile.am 519e38c Diff: https://reviews.apache.org/r/36226/diff/ Testing --- Thanks, Isabel Jimenez
Re: Review Request 23749: Missing Apache headers for mesos
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23749/#review90628 --- Patch looks great! Reviews applied: [23749] All tests passed. - Mesos ReviewBot On July 6, 2015, 10:33 p.m., Isabel Jimenez wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23749/ --- (Updated July 6, 2015, 10:33 p.m.) Review request for mesos and Benjamin Hindman. Repository: mesos Description --- Adding missing Apache licence headers on .cpp, .hpp and Makefiles Diffs - src/cli/mesos.cpp 0c315f7 src/files/files.cpp 3a62ee7 src/hdfs/hdfs.hpp cba386a src/slave/state.cpp fab47a7 src/state/in_memory.hpp 2040618 src/state/in_memory.cpp ce04e47 src/state/leveldb.hpp 53447c6 src/state/leveldb.cpp 14a1807 src/state/log.hpp a0ca4f8 src/state/log.cpp 326f3a7 src/state/zookeeper.hpp 1a8483d src/state/zookeeper.cpp 9151d55 src/tests/active_user_test_helper.cpp c32d467 src/zookeeper/authentication.hpp 7b6b767 src/zookeeper/authentication.cpp ccd6bf7 src/zookeeper/contender.hpp 6529245 src/zookeeper/contender.cpp 3255ef0 src/zookeeper/detector.hpp e41158e src/zookeeper/detector.cpp 5305161 src/zookeeper/group.hpp 9246130 src/zookeeper/group.cpp 01066e3 src/zookeeper/watcher.hpp 1e347ed Diff: https://reviews.apache.org/r/23749/diff/ Testing --- Thanks, Isabel Jimenez
Re: Review Request 32982: Added reservation user guide.
On June 29, 2015, 4:43 p.m., Adam B wrote: Looks great! I know this is already committed, but I had a few questions/clarifications. Maybe you've answered these elsewhere, but I've been out of the loop for a while. Thanks for answering my questions. I don't think there's anything that really warrants an edit/update. On June 29, 2015, 4:43 p.m., Adam B wrote: docs/reservation.md, lines 109-110 https://reviews.apache.org/r/32982/diff/12/?file=994059#file994059line109 Shouldn't the master be able to determine the principal that this frameworkId is currently authenticated as? Why even allow the framework to specify a different principal if you're just going to Error? Michael Park wrote: Hm, that's true. This was another situation where I wanted to keep `ReservationInfo.principal` as `required`. Fair enough, although it seems like you're leaking the internal data representation into the user-facing API. Shouldn't be necessary. On June 29, 2015, 4:43 p.m., Adam B wrote: docs/reservation.md, line 196 https://reviews.apache.org/r/32982/diff/12/?file=994059#file994059line196 Could also only unreserve a subset, right? Michael Park wrote: Yeah, that's true. I didn't really mention that part. Not terribly important. Anybody familiar with using resource offers should be able to figure that one out. - Adam --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32982/#review89824 --- On June 27, 2015, 8:33 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32982/ --- (Updated June 27, 2015, 8:33 p.m.) Review request for mesos, Alexander Rukletsov, Jie Yu, and Timothy Chen. Bugs: MESOS-2205 https://issues.apache.org/jira/browse/MESOS-2205 Repository: mesos Description --- The Github rendered version is available [here]( https://github.com/mesosphere/mesos/blob/user-docs/docs/reservation.md) Diffs - docs/reservation.md PRE-CREATION Diff: https://reviews.apache.org/r/32982/diff/ Testing --- Documentation. Thanks, Michael Park
Re: Review Request 36217: Adding http validations to master call request validations
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36217/#review90640 --- Patch looks great! Reviews applied: [36215, 36217] All tests passed. - Mesos ReviewBot On July 6, 2015, 11:31 p.m., Isabel Jimenez wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36217/ --- (Updated July 6, 2015, 11:31 p.m.) Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and Vinod Kone. Bugs: MESOS-2860 https://issues.apache.org/jira/browse/MESOS-2860 Repository: mesos-incubating Description --- Adding helper validations for http requests Diffs - src/Makefile.am addb63f src/common/http_validation.hpp PRE-CREATION src/common/http_validation.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36217/diff/ Testing --- make check Thanks, Isabel Jimenez
Review Request 36245: Fix compilation error for clang-3.5 type deduction error.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36245/ --- Review request for mesos, Benjamin Hindman and Michael Park. Bugs: MESOS-2943 https://issues.apache.org/jira/browse/MESOS-2943 Repository: mesos Description --- While we figure out how to avoid this bug in clang-3.5, we can allow people to compile by explicitly specifying the return type of the lambda. Diffs - 3rdparty/libprocess/src/libevent_ssl_socket.cpp 9424dd421b290bec25b4a7c4dc0071ffef6fdc5e Diff: https://reviews.apache.org/r/36245/diff/ Testing --- make check 3rdparty using clang-3.5 Thanks, Joris Van Remoortere
Re: Review Request 36245: Fix compilation error for clang-3.5 type deduction error.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36245/#review90648 --- Ship it! Ship It! - Adam B On July 6, 2015, 10:16 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36245/ --- (Updated July 6, 2015, 10:16 p.m.) Review request for mesos, Benjamin Hindman and Michael Park. Bugs: MESOS-2943 https://issues.apache.org/jira/browse/MESOS-2943 Repository: mesos Description --- While we figure out how to avoid this bug in clang-3.5, we can allow people to compile by explicitly specifying the return type of the lambda. Diffs - 3rdparty/libprocess/src/libevent_ssl_socket.cpp 9424dd421b290bec25b4a7c4dc0071ffef6fdc5e Diff: https://reviews.apache.org/r/36245/diff/ Testing --- make check 3rdparty using clang-3.5 Thanks, Joris Van Remoortere
Re: Review Request 36245: Fix compilation error for clang-3.5 type deduction error.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36245/#review90650 --- Ship it! Ship It! - Artem Harutyunyan On July 6, 2015, 10:16 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36245/ --- (Updated July 6, 2015, 10:16 p.m.) Review request for mesos, Benjamin Hindman and Michael Park. Bugs: MESOS-2943 https://issues.apache.org/jira/browse/MESOS-2943 Repository: mesos Description --- While we figure out how to avoid this bug in clang-3.5, we can allow people to compile by explicitly specifying the return type of the lambda. Diffs - 3rdparty/libprocess/src/libevent_ssl_socket.cpp 9424dd421b290bec25b4a7c4dc0071ffef6fdc5e Diff: https://reviews.apache.org/r/36245/diff/ Testing --- make check 3rdparty using clang-3.5 Thanks, Joris Van Remoortere
Re: Review Request 36173: Update attributes doc to reflect current supported attributes types.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36173/#review90445
---
docs/attributes-resources.md (line 35)
https://reviews.apache.org/r/36173/#comment143491
Doesn't `( scalar | range | text | , )+` look like a set that doesn't
wrap itself in `{}` braces?
It doesn't look like Attributes::parse handles the , case, so let's
remove that (and the `()+`) from the documentation.
- Adam B
On July 3, 2015, 2:30 p.m., Timothy Chen wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36173/
---
(Updated July 3, 2015, 2:30 p.m.)
Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till
Toenshoff.
Repository: mesos
Description
---
Update attributes doc to reflect current supported attributes types.
Diffs
-
docs/attributes-resources.md 0ae8b5908fe0b3a3499e6d813afbb328a13bdcde
Diff: https://reviews.apache.org/r/36173/diff/
Testing
---
make
Thanks,
Timothy Chen
Re: Review Request 35287: Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35287/ --- (Updated July 6, 2015, 3:04 p.m.) Review request for mesos and Joris Van Remoortere. Changes --- rebase Bugs: MESOS-2800 https://issues.apache.org/jira/browse/MESOS-2800 Repository: mesos Description --- Rename OptionT::get(const T _t) to getOrElse() and refactor original functions (stout) Diffs (updated) - 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 8d5217a699a302cc9ebb2aa10d74cced7eb2b3d9 3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp c8d30d8c193eb14f7accfde4fe02ce0710cd1817 3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp f1ae80ab881ccfcefda69d5bfee2d969d171f1b9 Diff: https://reviews.apache.org/r/35287/diff/ Testing --- make check Thanks, Mark Wang
Re: Review Request 36189: Add strings::Mode to strings::trim.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36189/ --- (Updated July 6, 2015, 3:29 p.m.) Review request for mesos, Artem Harutyunyan and Vinod Kone. Bugs: MESOS-2862 https://issues.apache.org/jira/browse/MESOS-2862 Repository: mesos Description --- See summary. Diffs (updated) - 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 963029bea989a68a484f7b8b47d29ea5fffeb955 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 9733b2ec5d68605b694210c66144b8d9f8c36467 Diff: https://reviews.apache.org/r/36189/diff/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 35755: Changed fetcher to handle leading whitespace in URLs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35755/ --- (Updated July 6, 2015, 9:03 a.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2862 https://issues.apache.org/jira/browse/MESOS-2862 Repository: mesos Description --- Fixes MESOS-2862 Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd src/tests/fetcher_tests.cpp ae10c420f7dddb8650377c91b5343591e8560392 Diff: https://reviews.apache.org/r/35755/diff/ Testing --- - make check - added a test case for fetcher Thanks, Artem Harutyunyan
Review Request 36204: Pass slave's total resources in ResourceUsage.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36204/ --- Review request for mesos, Ben Mahler, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone. Bugs: MESOS-2933 https://issues.apache.org/jira/browse/MESOS-2933 Repository: mesos Description --- Pass slave's total resources to the ResourceEstimator and QoSController via Slave::usage(). Diffs - include/mesos/mesos.proto 3dd4a5b7a4b3bc56bdc690d6adf05f88c0d28273 src/slave/slave.cpp 008170f4ce7b97eda43b50b48083b32ae65925c0 Diff: https://reviews.apache.org/r/36204/diff/ Testing --- make check. Thanks, Bartek Plotka
Re: Review Request 36048: Updated authorizer to allow for modularized implementations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36048/ --- (Updated July 6, 2015, 5:41 p.m.) Review request for mesos, Adam B and Till Toenshoff. Changes --- Properly use of includes. Bugs: MESOS-2945 https://issues.apache.org/jira/browse/MESOS-2945 Repository: mesos Description --- Splits and updates the original declaration of the `Authorizer` into its interface and a default implementation, the `LocalAuthorizer`. Following the pattern of the modularized `Authenticator`, it generates a default constructor which is required when writing a `TYPED_TEST` in a follow up patch. Additionally, an initialize method has been added, needed for passing in the current ACL definitions as provided via flags. Other changes are just updates to allow for compilation. Diffs (updated) - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/authorizer/authorizer.proto PRE-CREATION include/mesos/mesos.proto 3dd4a5b7a4b3bc56bdc690d6adf05f88c0d28273 include/mesos/type_utils.hpp e7bfe8ca60af945e76b5d85ab37cc97b17ff1b4a src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 src/authorizer/authorizer.hpp c039d9412780aa199db169b31991bf9f45b07d0f src/authorizer/authorizer.cpp 21e97e315478a4ca9442af83732665f85eb2f8fc src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 src/examples/persistent_volume_framework.cpp c6d6ed337bfca91dc146cb31298cabebdbb13509 src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/http.cpp 2be613b2e4c913b74c12d0d8f2d0e25da3cd3656 src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 src/master/master.hpp fb4d6fac85e284987ec8fbf6949b5023875573fb src/master/master.cpp c5a4875f0d43c5091ae9a52c6b1d04105dfa3914 src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26 src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 src/tests/mesos.cpp 5eab6dea6058865847425ab8d31708c92c6f098a Diff: https://reviews.apache.org/r/36048/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36048: Updated authorizer to allow for modularized implementations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36048/ --- (Updated July 6, 2015, 4:18 p.m.) Review request for mesos, Adam B and Till Toenshoff. Changes --- Fixes small style issue. Bugs: MESOS-2945 https://issues.apache.org/jira/browse/MESOS-2945 Repository: mesos Description --- Splits and updates the original declaration of the `Authorizer` into its interface and a default implementation, the `LocalAuthorizer`. Following the pattern of the modularized `Authenticator`, it generates a default constructor which is required when writing a `TYPED_TEST` in a follow up patch. Additionally, an initialize method has been added, needed for passing in the current ACL definitions as provided via flags. Other changes are just updates to allow for compilation. Diffs (updated) - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/authorizer/authorizer.proto PRE-CREATION include/mesos/mesos.proto 3dd4a5b7a4b3bc56bdc690d6adf05f88c0d28273 include/mesos/type_utils.hpp e7bfe8ca60af945e76b5d85ab37cc97b17ff1b4a src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 src/authorizer/authorizer.hpp c039d9412780aa199db169b31991bf9f45b07d0f src/authorizer/authorizer.cpp 21e97e315478a4ca9442af83732665f85eb2f8fc src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 src/examples/persistent_volume_framework.cpp c6d6ed337bfca91dc146cb31298cabebdbb13509 src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/http.cpp 2be613b2e4c913b74c12d0d8f2d0e25da3cd3656 src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 src/master/master.hpp fb4d6fac85e284987ec8fbf6949b5023875573fb src/master/master.cpp c5a4875f0d43c5091ae9a52c6b1d04105dfa3914 src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26 src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 src/tests/mesos.cpp 5eab6dea6058865847425ab8d31708c92c6f098a Diff: https://reviews.apache.org/r/36048/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36202: Fixed style problem in TODO
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36202/#review90485 --- Ship it! Ship It! - Till Toenshoff On July 6, 2015, 2:06 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36202/ --- (Updated July 6, 2015, 2:06 p.m.) Review request for mesos, Isabel Jimenez, Joerg Schad, and Till Toenshoff. Repository: mesos Description --- Commit [920930f](https://github.com/apache/mesos/commit/920930fe89f55ac08493829aa5797ba85d104367) introduced a TODO comment which breaks the style checker. This patch just fixes it. Diffs - src/tests/environment.cpp 6a881cad76b63b18a7000ad889fd0e071ca00d47 Diff: https://reviews.apache.org/r/36202/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36193: Improved Doxygen-Styleguide.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36193/#review90486 --- Patch looks great! Reviews applied: [36193] All tests passed. - Mesos ReviewBot On July 6, 2015, 9:01 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36193/ --- (Updated July 6, 2015, 9:01 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Joseph Wu. Repository: mesos Description --- Improved Doxygen-Styleguide for clarifying discussions arising from https://reviews.apache.org/r/36141/. Diffs - docs/mesos-doxygen-style-guide.md 72156c72fc40f72740e57d47d0436c60f6d05bd7 Diff: https://reviews.apache.org/r/36193/diff/ Testing --- Checked rendered markdown. Thanks, Joerg Schad
Review Request 36205: Added AWAIT_EXPECT_TRUE and AWAIT_EXPECT_FALSE macros.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36205/ --- Review request for mesos, Bernd Mathiske, Joerg Schad, and Till Toenshoff. Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/include/process/gtest.hpp 8518c38c9cbf62a098b43b4d0b6d80e561b17ad4 Diff: https://reviews.apache.org/r/36205/diff/ Testing --- Thanks, Alexander Rojas
Re: Review Request 36205: Added AWAIT_EXPECT_TRUE and AWAIT_EXPECT_FALSE macros.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36205/ --- (Updated July 6, 2015, 5:44 p.m.) Review request for mesos, Bernd Mathiske, Joerg Schad, and Till Toenshoff. Changes --- Added testing comment. Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/include/process/gtest.hpp 8518c38c9cbf62a098b43b4d0b6d80e561b17ad4 Diff: https://reviews.apache.org/r/36205/diff/ Testing (updated) --- Used in tests in follow up patch. Thanks, Alexander Rojas
