Re: Review Request 33828: Fix for MESOS-2690. Issue with enable-optimize build.

2015-05-04 Thread Adam B
. configure.ac https://reviews.apache.org/r/33828/#comment133200 Style nit: Any reason you went with `+=` instead of `CXXFLAGS=${CXXFLAGS} -Wno-maybe-uninitialized` like the rest of the file? - Adam B On May 4, 2015, 4:48 p.m., Joris Van Remoortere wrote

Re: Review Request 33109: Allow setting environment variables in mesos-execute

2015-05-12 Thread Adam B
On May 11, 2015, 10:58 p.m., Adam B wrote: src/cli/execute.cpp, lines 203-204 https://reviews.apache.org/r/33109/diff/5-6/?file=939687#file939687line203 You can keep this as one statement, just wrap with the '=' at the start of the newline. Correction... wrap after

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

2015-05-12 Thread Adam B
be so short. docs/modules.md https://reviews.apache.org/r/33718/#comment134341 Does it get loaded too by --modules, even if it's not selected by --hooks? If so, I would say load it instead of introduce it. If not, introduce seems like a perfect term. - Adam B On May 8, 2015, 4:19 p.m

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-05-14 Thread Adam B
timeouts. `make check` with new unit tests: ShortPingTimeoutUnreachableMaster and ShortPingTimeoutUnreachableSlave Thanks, Adam B

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-05-14 Thread Adam B
and ShortPingTimeoutUnreachableSlave Thanks, Adam B

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

2015-06-02 Thread Adam B
it a consistent 2-space indent. - Adam B On June 1, 2015, 8:52 a.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33295

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

2015-06-02 Thread Adam B
On June 1, 2015, 1:06 a.m., Adam B wrote: 3rdparty/libprocess/include/process/firewall.hpp, lines 54-55 https://reviews.apache.org/r/33295/diff/6/?file=964946#file964946line54 Please capitalize The beginning of each @param description. Alexander Rojas wrote: I do completely

Re: Review Request 33058: Updated test-frameworks to support principal only credential.

2015-06-02 Thread Adam B
April 10, 2015, 12:25 a.m.) Review request for mesos, Adam B and Alexander Rojas. Bugs: MESOS-2608 https://issues.apache.org/jira/browse/MESOS-2608 Repository: mesos-incubating Description --- see summary. Diffs - src/examples/java/TestFramework.java

Re: Review Request 34655: Use relative url in /help generated links point

2015-06-02 Thread Adam B
On May 28, 2015, 10:40 a.m., Adam B wrote: 3rdparty/libprocess/src/help.cpp, line 140 https://reviews.apache.org/r/34655/diff/1/?file=971486#file971486line140 Why does this '/help/' need to be removed? Marco Massenzio wrote: because it's wrong - it shouldn't

Re: Review Request 34976: Added installation instructions for Ubuntu 14.04 and OSX

2015-06-04 Thread Adam B
/#comment138727 It was suggested that we should keep this document shortsweet and only reference the latest Ubuntu LTS release, and then we can archive the 12.04 instructions in a blog post (on mesos/mesosphere site?). - Adam B On June 2, 2015, 11:44 p.m., Marco Massenzio wrote

Re: Review Request 35714: Added a new HTTP response type: PreconditionFailed.

2015-06-21 Thread Adam B
! 3rdparty/libprocess/include/process/http.hpp (line 473) https://reviews.apache.org/r/35714/#comment141299 Let's keep these in numerical order, so put this after 404, before 500, please. - Adam B On June 21, 2015, 9:16 a.m., Michael Park wrote

Re: Review Request 33339: Add a Java example framework to test persistent volumes.

2015-06-20 Thread Adam B
On June 1, 2015, 4:32 p.m., Marco Massenzio wrote: This is great -sorry it took so long to get to do a review. Thanks for doing it, I'm quite looking forward to using it to learning more about the Persistent Framework :) it would be great if we could have a bit more comments in the

Re: Review Request 35506: Excluding MarkDown files from the style hook

2015-06-20 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35506/#review88638 --- Ship it! Ship It! - Adam B On June 16, 2015, 4:24 a.m

Re: Review Request 35583: Updated slave to exit on authentication refusal.

2015-06-20 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35583/#review88637 --- Ship it! Ship It! - Adam B On June 17, 2015, 3:42 p.m., Till

Re: Review Request 31838: Fixed authentication failure triggered slave crash.

2015-06-20 Thread Adam B
the if/else-if/else formatting. I could do it before committing, or you could. - Adam B On March 8, 2015, 5:54 p.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-06-22 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/#review86039 --- On May 28, 2015, 4:13 p.m., Adam B wrote

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-06-22 Thread Adam B
slave failover/shutdown with master using different --slave_ping_timeout and --max_slave_ping_timeouts. Ran unit tests with shorter non-default values for ping timeouts. `make check` with new unit tests: ShortPingTimeoutUnreachableMaster and ShortPingTimeoutUnreachableSlave Thanks, Adam B

Re: Review Request 35917: Disabled DISABLED_HttpCachedRecovery.

2015-06-26 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35917/#review89475 --- Ship it! Ship It! - Adam B On June 26, 2015, 12:47 a.m., Bernd

Re: Review Request 35717: Add reservations support to master's state.json

2015-06-25 Thread Adam B
On June 24, 2015, 3:24 a.m., Adam B wrote: src/common/http.cpp, line 84 https://reviews.apache.org/r/35717/diff/4/?file=989671#file989671line84 I wonder if you could templatize this to work for modeling any hashmapstring, T, where T is an already modeled type. I'm not sure what

Re: Review Request 35717: Add reservations support to master's state.json

2015-06-25 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35717/#review89456 --- Ship it! Ship It! - Adam B On June 22, 2015, 4:37 a.m

Re: Review Request 35721: Set the owner of persistent volumes to frameworkInfo.user .

2015-06-24 Thread Adam B
On June 24, 2015, 1:04 a.m., Adam B wrote: This seems like a positive step forward, since all volumes were previously owned by root (or whatever user the slave was run as). However, if a persistent volume is passed from one framework to another within the same role, they might have

Re: Review Request 35721: Set the owner of persistent volumes to frameworkInfo.user .

2015-06-24 Thread Adam B
as the slave user, and volumes will need to stay as that user in order to be accessible by the tasks. - Adam B On June 21, 2015, 6:56 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 35782: fix web UI shows YYYY for year instead of year

2015-06-24 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35782/#review89141 --- Ship it! Ship It! - Adam B On June 23, 2015, 3:14 a.m

Re: Review Request 35782: fix web UI shows YYYY for year instead of year

2015-06-24 Thread Adam B
suggested in the ticket. haosdent, did you do any (manual) testing on this? Maybe you could link to a before/after screenshot? - Adam B On June 23, 2015, 3:14 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply

Re: Review Request 35711: Disallow special characters in role name.

2015-06-24 Thread Adam B
twice, but that's probably not much of a performance hit. - Adam B On June 21, 2015, 7:21 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35711

Re: Review Request 35717: Add reservations support to master's state.json

2015-06-26 Thread Adam B
: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35717/ --- (Updated June 22, 2015, 4:37 a.m.) Review request for mesos, Adam B

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-06-26 Thread Adam B
--slave_ping_timeout and --max_slave_ping_timeouts. Ran unit tests with shorter non-default values for ping timeouts. `make check` with new unit tests: ShortPingTimeoutUnreachableMaster and ShortPingTimeoutUnreachableSlave Thanks, Adam B

Review Request 35958: Updated SlaveTest.PingTimeoutNoPings test to use custom timeout values.

2015-06-26 Thread Adam B
. Thanks, Adam B

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-06-26 Thread Adam B
to `PartitionTest.PartitionedSlave`. For (2), we have SlaveTest.PingTimeoutNoPings, SlaveTest.PingTimeoutNoPings. Adam B wrote: Yes, I see now that SlaveTest.PingTimeoutNoPings already satisfies (1), and PartitionTest.PartitionedSlave could be modified to check for the ShutdownMessage

Re: Review Request 34976: Added installation instructions for Ubuntu 14.04 and OSX

2015-06-11 Thread Adam B
-core or smaller. - Adam B On June 2, 2015, 11:44 p.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34976

Re: Review Request 34976: Added installation instructions for Ubuntu 14.04 and OSX

2015-06-11 Thread Adam B
On June 11, 2015, 3:02 a.m., Adam B wrote: Sorry it took me so long to get back to this. I just had a few minor comments/questions, but you ought to get a Mac user to verify the OSX instructions. Also, for doc changes like this, it's helpful if you can post a link to a personal fork

Re: Review Request 34976: Added installation instructions for Ubuntu 14.04 and OSX

2015-06-12 Thread Adam B
instructions. docs/getting-started.md https://reviews.apache.org/r/34976/#comment140107 s/add/append/ docs/getting-started.md https://reviews.apache.org/r/34976/#comment140108 s/mesos/Mesos/ - Adam B On June 11, 2015, 11:23 p.m., Marco Massenzio wrote

Re: Review Request 35179: MESOS-1733 Variadic Path Join

2015-06-12 Thread Adam B
the Testing section. ReviewBot says the patch doesn't apply cleanly, so please rebase. - Adam B On June 12, 2015, 3:32 a.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 35179: MESOS-1733 Variadic Path Join

2015-06-17 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35179/#review88312 --- Ship it! Looks great! I'll get this committed tonight - Adam B

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-05-28 Thread Adam B
failover/shutdown with master using different --slave_ping_timeout and --max_slave_ping_timeouts. Ran unit tests with shorter non-default values for ping timeouts. `make check` with new unit tests: ShortPingTimeoutUnreachableMaster and ShortPingTimeoutUnreachableSlave Thanks, Adam B

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-05-28 Thread Adam B
://reviews.apache.org/r/29507/#review83758 --- On May 14, 2015, 3:01 a.m., Adam B wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r

Re: Review Request 34655: Use relative url in /help generated links point

2015-05-28 Thread Adam B
did you test this? Please fill out the Testing section appropriately. 3rdparty/libprocess/src/help.cpp https://reviews.apache.org/r/34655/#comment137159 Why does this '/help/' need to be removed? - Adam B On May 28, 2015, 12:54 a.m., haosdent huang wrote

Re: Review Request 34545: Updated the allocator related docs.

2015-05-29 Thread Adam B
://reviews.apache.org/r/34545/#comment137433 `file://path-to-external-allocator.json` is a little confusing, especially if there are multiple modules that need to be loaded. Maybe just call it `modules.json` give an example json? - Adam B On May 26, 2015, 3:37 a.m., Alexander Rukletsov wrote

Re: Review Request 34655: Use relative url in /help generated links point

2015-05-29 Thread Adam B
On May 28, 2015, 10:40 a.m., Adam B wrote: Do you clearly understand why this change is needed? I didn't understand after just reading the JIRA, and had to ask the reporter(s). Mesosphere is hosting the Mesos UI(s) underneath the DCOS UI behind a reverse proxy, so that `http

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

2015-06-01 Thread Adam B
the default id value? All your callers already specify a processId. 3rdparty/libprocess/src/tests/process_tests.cpp https://reviews.apache.org/r/33295/#comment137781 Would like to see a test for nested endpoints, and maybe even wildcards, so that it's clear whether or not those work. - Adam B

Re: Review Request 34362: Include ExecutorInfos in master/state.json

2015-06-01 Thread Adam B
. src/master/http.cpp https://reviews.apache.org/r/34362/#comment137802 Nit: Please insert blank line between these two like in other similar blocks. src/master/http.cpp https://reviews.apache.org/r/34362/#comment137801 std::move(executors); - Adam B On May 18, 2015, 10:41 a.m

Re: Review Request 34646: Redirect to the leader master when current master is not a leader.

2015-06-01 Thread Adam B
On June 1, 2015, 1:34 a.m., Adam B wrote: src/master/http.cpp, line 1038 https://reviews.apache.org/r/34646/diff/3/?file=975522#file975522line1038 Could I ask you to write a quick unit test for this? haosdent huang wrote: yes haosdent huang wrote: @adam-mesos, I have

Re: Review Request 34646: Return empty task list when current master is not a leader.

2015-05-28 Thread Adam B
tasks list. Otherwise, if it's going to return a blank tasks[] array, then it should return something like a 500 (Internal Server Error), or 307 (Temporary Redirect). - Adam B On May 24, 2015, 11:22 a.m., haosdent huang wrote

Re: Review Request 36068: MESOS-2966: Fix 'peer()' call for ssl socket.

2015-07-01 Thread Adam B
approval. 3rdparty/libprocess/src/tests/ssl_tests.cpp (lines 907 - 910) https://reviews.apache.org/r/36068/#comment142966 Would be nice if you commented on the meaning of these equality checks, since they're the heart of this test. - Adam B On June 30, 2015, 5:33 p.m., Joris Van Remoortere

Re: Review Request 34646: Redirect to the leader master when current master is not a leader.

2015-07-01 Thread Adam B
On June 1, 2015, 1:34 a.m., Adam B wrote: src/master/http.cpp, line 1038 https://reviews.apache.org/r/34646/diff/3/?file=975522#file975522line1038 Could I ask you to write a quick unit test for this? haosdent huang wrote: yes haosdent huang wrote: @adam-mesos, I have

Re: Review Request 34646: Redirect to the leader master when current master is not a leader.

2015-06-30 Thread Adam B
or follow the redirect? if the latter, it might result in duplicate collection!). Can you send an email to the dev list? haosdent huang wrote: Sure.Thank you for your review. @vinodkone Adam B wrote: It's important to note that this patch does not redirect on certain master endpoints

Re: Review Request 34646: Redirect to the leader master when current master is not a leader.

2015-07-01 Thread Adam B
On June 1, 2015, 1:34 a.m., Adam B wrote: src/master/http.cpp, line 1038 https://reviews.apache.org/r/34646/diff/3/?file=975522#file975522line1038 Could I ask you to write a quick unit test for this? haosdent huang wrote: yes haosdent huang wrote: @adam-mesos, I have

Re: Review Request 36061: Slave exits gracefully on DNS lookup failure.

2015-07-01 Thread Adam B
ticket (MESOS-2636) discusses segfaults within the actual `getIP()` and `hostname()` methods, so the process would have segfaulted before reaching the LOG(FATAL). Now that MESOS-2636 has been fixed, we should safely get back an `ip.error()` that we can actually log. - Adam B On June 30, 2015

Re: Review Request 36036: Add `version` string to MasterInfo

2015-07-01 Thread Adam B
: 0.22.1 main.cpp:190] Git SHA: d6309f92a7f9af3ab61a878403e3d9c284ea87e0 And if you do include it, I don't think it needs to be inside hostname's parentheses. - Adam B On June 30, 2015, 4:23 p.m., Marco Massenzio wrote

Re: Review Request 36152: Updated CHANGELOG for 0.23.0.

2015-07-03 Thread Adam B
://reviews.apache.org/r/36152/#review90298 --- On July 2, 2015, 4:47 p.m., Adam B wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36152

Re: Review Request 35721: Set the ownership of persistent volume to match the sandbox directory.

2015-06-29 Thread Adam B
/containerizer.cpp (lines 1353 - 1356) https://reviews.apache.org/r/35721/#comment142438 These four lines could easily fit on two. We wrap code at 80 chars. - Adam B On June 28, 2015, 7:36 p.m., haosdent huang wrote

Re: Review Request 35711: Disallow special characters in role name.

2015-06-29 Thread Adam B
` as well? - Adam B On June 27, 2015, 4:35 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35711

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-06-29 Thread Adam B
On June 24, 2015, 6:41 p.m., Ben Mahler wrote: Actually, we should think about one more thing, how does this interact with the zookeeper session timeout? Adam B wrote: The hardcoded individual ping timeout (15secs) was previously longer than the default zk session timeout (10secs

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

2015-07-06 Thread Adam B
--- 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

Re: Review Request 36152: Updated CHANGELOG for 0.23.0.

2015-07-02 Thread Adam B
, categorized. Diffs (updated) - CHANGELOG 252f068 Diff: https://reviews.apache.org/r/36152/diff/ Testing --- Thanks, Adam B

Review Request 36152: Updated CHANGELOG for 0.23.0.

2015-07-02 Thread Adam B
--- Thanks, Adam B

Re: Review Request 35986: Allow slave attributes flag take a value with ':'.

2015-07-06 Thread Adam B
) 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

Re: Review Request 32982: Added reservation user guide.

2015-07-06 Thread Adam B
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

Re: Review Request 36245: Fix compilation error for clang-3.5 type deduction error.

2015-07-06 Thread Adam B
--- 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

Re: Review Request 36010: Updated upgrades.md about Resource API changes.

2015-06-29 Thread Adam B
/upgrades.md (line 17) https://reviews.apache.org/r/36010/#comment142552 s/meta data/metadata/g s/reservation/reservations/ s/object/objects/ Is this a Please do not or a you cannot or you must not? Will it error? Will things break? - Adam B On June 29, 2015, 11:16 a.m., Jie Yu wrote

Re: Review Request 36008: Updated upgrades.md about the new 'acceptOffers' API.

2015-06-29 Thread Adam B
of the decline API. docs/upgrades.md (line 15) https://reviews.apache.org/r/36008/#comment142549 Might also want to mention how (implicitly) declining full/partial offers plays into this API. Or is there fuller documentation/doxygen you could reference/link to? - Adam B On June 29, 2015, 10

Re: Review Request 36011: Updated CHANGELOG about the Resource API changes.

2015-06-29 Thread Adam B
15 - 19) https://reviews.apache.org/r/36011/#comment142553 We usually order these numerically, rather than grouping them by feature/theme. - Adam B On June 29, 2015, 11:17 a.m., Jie Yu wrote: --- This is an automatically generated

Re: Review Request 36145: Update Upgrade Guide for Mesos 0.23.0

2015-07-02 Thread Adam B
Description (updated) --- Added component upgrade order. Diffs (updated) - docs/upgrades.md cef055b Diff: https://reviews.apache.org/r/36145/diff/ Testing (updated) --- Reviewers, please let me know if you know of any features that require special upgrade instructions. Thanks, Adam

Review Request 36145: Update Upgrade Guide for Mesos 0.23.0

2015-07-02 Thread Adam B
://reviews.apache.org/r/36145/diff/ Testing --- Reviewers, please let me know if you know of any features that require special upgrade instructions. - What about handling modules during upgrades? Rebuild them before installing/restarting the other components? Thanks, Adam B

Re: Review Request 36173: Update attributes doc to reflect current supported attributes types.

2015-07-06 Thread Adam B
/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

Re: Review Request 36173: Update attributes doc to reflect current supported attributes types.

2015-07-05 Thread Adam B
31) https://reviews.apache.org/r/36173/#comment143483 No sets? Or are attributes implicitly sets? docs/attributes-resources.md (line 39) https://reviews.apache.org/r/36173/#comment143484 3 types? What about plain text? - Adam B On July 3, 2015, 2:30 p.m., Timothy Chen wrote

Re: Review Request 36269: Update CHANGELOG to reflect obsolete code cleanup

2015-07-07 Thread Adam B
cherry-picking select patches into 0.23.0-rc2. Does this need to be one? If not, create a new `(WIP) Release Notes - Mesos - Version 0.24.0` section at the top to place your Cleanup ticket under. - Adam B On July 7, 2015, 10:59 a.m., Jiang Yan Xu wrote

Re: Review Request 36022: Remove http specific protocol from master browse in webui.

2015-06-29 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36022/#review89797 --- Ship it! Ship It! - Adam B On June 29, 2015, 2:03 p.m., Joris

Re: Review Request 36026: Fixed typo in slave flags.

2015-06-29 Thread Adam B
the right fix. src/slave/flags.cpp (line 284) https://reviews.apache.org/r/36026/#comment142610 s/that // - Adam B On June 29, 2015, 3:56 p.m., Brendan Chang wrote: --- This is an automatically generated e-mail. To reply, visit

Re: Review Request 35728: Fix failing test: SlaveTest.ROOT_RunTaskWithCommandInfoWithUser.

2015-06-29 Thread Adam B
On June 25, 2015, 7:30 p.m., Adam B wrote: Nice work! So, running the command as root first will guarantee that lt-mesos-executor exists before trying to run the task as the test-user `nobody`? We might be able to fix this in a cleaner way, but this looks good enough to me. Just fix

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-06-26 Thread Adam B
--- On June 26, 2015, 3:12 a.m., Adam B wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507

Re: Review Request 32982: Added reservation user guide.

2015-06-29 Thread Adam B
as a certain principal in the request, why do we need to explicitly specify a (potentially different?) principal in the resources message? docs/reservation.md (line 309) https://reviews.apache.org/r/32982/#comment142627 How can there be insufficient resources to unreserve? - Adam B On June 27

Re: Review Request 36026: Fixed typo in slave flags.

2015-06-29 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36026/#review89820 --- Ship it! Ship It! - Adam B On June 29, 2015, 4:03 p.m., Brendan

Re: Review Request 35981: Added persistent volume user guide.

2015-06-29 Thread Adam B
in the list must contain a `disk.volume` to be destroyed. - Adam B On June 27, 2015, 8:35 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35981

Re: Review Request 36003: MESOS-2942: Documentation for SSL.

2015-06-29 Thread Adam B
://reviews.apache.org/r/36003/#comment142675 s/SSL enabled/SSL-enabled/ s/or not// - Adam B On June 29, 2015, 2:50 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 35961: Include protobuf classes in generated Javadoc.

2015-06-29 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35961/#review89839 --- Ship it! Ship It! - Adam B On June 29, 2015, 8:53 a.m., Connor

Re: Review Request 34646: Redirect to the leader master when current master is not a leader.

2015-06-29 Thread Adam B
s/leader master/leading master/g src/master/master.hpp (line 1096) https://reviews.apache.org/r/34646/#comment142700 s/Reredirect/Redirect/ s/leader master/leading master/ - Adam B On June 1, 2015, 8:07 a.m., haosdent huang wrote

Re: Review Request 36003: MESOS-2942: Documentation for SSL.

2015-06-30 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36003/#review89949 --- Ship it! Ship It! - Adam B On June 30, 2015, 11:41 a.m., Joris

Re: Review Request 35919: Firewall rule's apply method returns an HTTP response instead of an error message.

2015-06-29 Thread Adam B
with a path. 3rdparty/libprocess/src/process.cpp (lines 2019 - 2020) https://reviews.apache.org/r/35919/#comment142712 You're not logging the Response body anymore? - Adam B On June 29, 2015, 3:21 a.m., Alexander Rojas wrote

Re: Review Request 36816: Support HTTP checks in Mesos health check program

2015-07-28 Thread Adam B
/HTTP health check returned/ - Adam B On July 25, 2015, 11:57 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36816

Re: Review Request 36197: Documented how to become a committer.

2015-07-28 Thread Adam B
(line 11) https://reviews.apache.org/r/36197/#comment147635 s/elected/voted in/ since an election implies a choice of candidates. - Adam B On July 28, 2015, 1:11 a.m., Bernd Mathiske wrote: --- This is an automatically generated e

Re: Review Request 32700: Removed FrameworkID from FrameworkState.

2015-08-03 Thread Adam B
? Is this some other upgrade issue? Is a CHECK too strong of an error check? Ditto elsewhere. - Adam B On Aug. 1, 2015, 12:13 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 36867: Add labels to FrameworkInfo.

2015-08-03 Thread Adam B
On July 27, 2015, 11:36 p.m., Adam B wrote: Great first patch. Thanks for updating FrameworkInfo on reregistration with the master too! A handful of nits in my first pass. I'll take another look once you've simplified the tests with Kapil's suggestions. Niklas Nielsen wrote

Re: Review Request 36814: Fill executor_id in state.json when task is run in CommandExecutor.

2015-08-03 Thread Adam B
the master does not know the executor ID for the tasks using a command executor.? @bmahler do you have any further thoughts/questions on this simple change? - Adam B On Aug. 2, 2015, 12:57 a.m., haosdent huang wrote

Re: Review Request 36816: Support HTTP checks in Mesos health check program

2015-08-04 Thread Adam B
/36816/#comment147942 Why is this comment relevant here? You aren't even mentioning https here. I'd remove it. include/mesos/mesos.proto (line 207) https://reviews.apache.org/r/36816/#comment147943 s/compile/is compiled/ - Adam B On Aug. 4, 2015, 2:28 a.m., haosdent huang wrote

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

2015-07-27 Thread Adam B
`--with-protobuf=`? All four combinations still need to compile pass the unit tests. - Adam B On July 25, 2015, 1:22 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org

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

2015-07-27 Thread Adam B
`--with-protobuf=`? All four combinations still need to compile pass the unit tests. - Adam B On July 25, 2015, 1:23 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org

Re: Review Request 36867: Add labels to FrameworkInfo.

2015-07-28 Thread Adam B
) https://reviews.apache.org/r/36867/#comment147586 Unused. Remove. src/tests/master_tests.cpp (line 3595) https://reviews.apache.org/r/36867/#comment147588 Nit: Only indent 2 spaces after wrapping on `=`. - Adam B On July 27, 2015, 6:25 p.m., Neil Conway wrote

Re: Review Request 36197: Documented how to become a committer.

2015-07-30 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36197/#review93558 --- Ship it! Ship It! - Adam B On July 28, 2015, 11:02 a.m., Bernd

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

2015-07-30 Thread Adam B
On July 30, 2015, 9:24 a.m., Adam B wrote: src/slave/slave.cpp, lines 1243-1245 https://reviews.apache.org/r/32587/diff/6/?file=1025122#file1025122line1243 Maybe just warn and leave the CopyFrom there? Next release, we'll remove the field entirely, and consequently

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

2015-07-30 Thread Adam B
, @cmaloney - Adam B On July 28, 2015, 7:43 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36810

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

2015-07-30 Thread Adam B
remove the `using namespace` and commit this. 3rdparty/libprocess/src/firewall.cpp (line 22) https://reviews.apache.org/r/36821/#comment147941 Shouldn't need to have a `using` for the namespace, since the code is all inside that same namespace. - Adam B On July 29, 2015, 10:40 a.m

Re: Review Request 36820: Fixed clang warning [-Wunevaluated-expression] in process tests.

2015-07-28 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36820/#review93356 --- Ship it! Ship It! - Adam B On July 26, 2015, 9:51 a.m., Joris

Re: Review Request 36488: Added oversubscription user doc.

2015-07-15 Thread Adam B
/#comment145527 Can you have multiple resource estimators running on the same node simultaneously? What about QoS controllers? - Adam B On July 14, 2015, 5:08 p.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail

Re: Review Request 36547: Fixed fetcher failing for FTP URIs.

2015-07-16 Thread Adam B
://reviews.apache.org/r/36547/#comment145870 Why not just check for any 2xx code then? Aren't they all successful in one way or another? - Adam B On July 16, 2015, 9:55 a.m., Jan Schlicht wrote: --- This is an automatically generated e-mail

Re: Review Request 36488: Added oversubscription user doc.

2015-07-17 Thread Adam B
useful. - Adam B On July 17, 2015, 1 p.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36488/ --- (Updated

Re: Review Request 39385: Fixed link conversion regexp in website.

2015-10-25 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39385/#review103950 --- Ship it! Ship It! - Adam B On Oct. 22, 2015, 4:03 p.m., Neil

Re: Review Request 39385: Fixed link conversion regexp in website.

2015-10-25 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39385/#review103951 --- Ship it! Looks like a legit regexp improvement to me. - Adam B

Re: Review Request 39645: Updates /site to reflect Niklas' 0.25.0 changes that weren't reflected in git.

2015-10-26 Thread Adam B
with this too. site/source/blog/2015-10-12-mesos-0-25-0-released.md (line 37) <https://reviews.apache.org/r/39645/#comment162205> ReviewBot seems to be complaining about this new blank line. I know it's in the original from Nik's blog, but let's remove it before committing to git. -

Re: Review Request 39604: Added function that verifies prerequisites for using Linux launcher.

2015-10-23 Thread Adam B
tps://reviews.apache.org/r/39604/#comment161991> Nit: Wrap '{' to next line, like all function implementations - Adam B On Oct. 23, 2015, 3:21 p.m., Artem Harutyunyan wrote: > > --- > This is an automatically generated e-mail.

  1   2   3   4   5   6   7   >