Re: Review Request 32975: MESOS-1790 Adds chown option to CommandInfo.URI

2015-04-22 Thread Adam B
On April 8, 2015, 2:57 p.m., Vinod Kone wrote: src/tests/fetcher_tests.cpp, lines 112-116 https://reviews.apache.org/r/32975/diff/1/?file=920880#file920880line112 Add a note/todo here mentioning why you are setting these fields but not doing any asserts/expects on them. Jim

Re: Review Request 33274: Fix capture by reference of temporary strings in Libprocess.

2015-04-22 Thread Adam B
that this change doesn't compile into more copies, making Mesos run slower and fatter? - Adam B On April 22, 2015, 11:11 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r

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

2015-04-23 Thread Adam B
here, and/or in the flag itself) that only the first/last value will be used. - Adam B On April 22, 2015, 10:38 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r

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 33490: MESOS-2633 Avoid adding duplicate tasks

2015-05-07 Thread Adam B
, rather than doing a find_if at the call-site. I originally created this TODO because boost::circular_buffer doesn't have a contains(). - Adam B On May 7, 2015, 1:26 a.m., Marco Massenzio wrote: --- This is an automatically generated e

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

2015-05-11 Thread Adam B
/#comment134337 s/list/listed/ - Adam B On April 25, 2015, 2:11 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33109

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 31595: Changed mesos-frameworks torque page dead link

2015-05-05 Thread Adam B
On March 2, 2015, 7:35 a.m., Dave Lester wrote: Is the Torque framework still maintained? I believe the documentation page this previously linked to (which is now broken) was taken offline because it was no longer supported. If that's the case, we should go ahead and remove torque

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

2015-05-13 Thread Adam B
tasks? - Adam B On May 8, 2015, 3:23 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30612

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

2015-05-12 Thread Adam B
/ --- (Updated April 7, 2015, 10 a.m.) Review request for mesos, Adam B and Niklas Nielsen. Bugs: MESOS-2559 https://issues.apache.org/jira/browse/MESOS-2559 Repository: mesos Description --- FrameworkID can be retrieved from RunTaskMessage.framework

Re: Review Request 33372: Added decorator documentation and described the semantic change in Mesos 0.23.0

2015-05-14 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33372/#review83802 --- Ship it! Ship It! - Adam B On May 14, 2015, 10:06 a.m., Niklas

Re: Review Request 34048: Fixed disappearing search bar: MESOS-2479

2015-05-14 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34048/#review83735 --- Ship it! Ship It! - Adam B On May 11, 2015, 11:25 a.m., Ian

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 34295: Added maintainers documentation.

2015-05-17 Thread Adam B
here, if you'll have me. docs/committers.md https://reviews.apache.org/r/34295/#comment135179 There were talks of moving the CLI and/or WebUI outside of the repo, for external maintenance. Then we wouldn't necessarily need Apache Mesos committers to maintain it/them. - Adam B On May

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 34655: Use relative url in /help generated links point

2015-06-02 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34655/#review86174 --- Ship it! Ship It! - Adam B On May 28, 2015, 12:54 a.m

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 34362: Include ExecutorInfos in master/state.json

2015-06-20 Thread Adam B
this soon. src/common/http.cpp (line 159) https://reviews.apache.org/r/34362/#comment141143 Found a typo comment that got copied around: s/in/it/? - Adam B On June 7, 2015, 2:29 a.m., haosdent huang wrote

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

2015-06-20 Thread Adam B
/ --- (Updated March 8, 2015, 5:54 p.m.) Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Vinod Kone. Bugs: MESOS-2464 https://issues.apache.org/jira/browse/MESOS-2464 Repository: mesos-incubating Description --- see summary. Diffs

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

2015-06-20 Thread Adam B
: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31838/ --- (Updated March 8, 2015, 5:54 p.m.) Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Vinod Kone. Bugs: MESOS-2464 https

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 35728: Fix failing test: SlaveTest.ROOT_RunTaskWithCommandInfoWithUser.

2015-06-25 Thread Adam B
/ - Adam B On June 22, 2015, 4:34 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35728/ --- (Updated June

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 35756: Fixed a race condition in hook tests for remove-executor hook.

2015-06-24 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35756/#review89135 --- Ship it! Ship It! - Adam B On June 23, 2015, 2:47 p.m., Kapil

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-24 Thread Adam B
the resources by role, regardless of status. - Adam B On June 22, 2015, 4:37 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35717

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 35961: Include protobuf classes in generated Javadoc.

2015-06-27 Thread Adam B
only uses the opening p tag, without the closing /p tag. Did you find that this rendered incorrectly? - Adam B On June 26, 2015, 6:18 p.m., Connor Doyle wrote: --- This is an automatically generated e-mail. To reply, visit: https

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
with Master::Http::redirect(). Can you please dedupe? src/master/http.cpp https://reviews.apache.org/r/34646/#comment137784 Nit: we prefer trailing +s rather than leading +s. - Adam B On May 31, 2015, 11:35 p.m., haosdent huang wrote

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 36036: Add `version` string to MasterInfo

2015-06-30 Thread Adam B
in question fails. If somebody were to add more checks after yours (e.g. for new MasterInfo fields), it would make sense for the test to continue with other checks that don't care whether MasterInfo has a version. - Adam B On June 29, 2015, 11:28 p.m., Marco Massenzio wrote

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

2015-06-30 Thread Adam B
On June 29, 2015, 5:57 p.m., Adam B wrote: docs/mesos-ssl.md, lines 58-59 https://reviews.apache.org/r/36003/diff/3/?file=995165#file995165line58 Can you list RHEL/Ubuntu instructions as well? (For OpenSSL as well) Joris Van Remoortere wrote: I'm reluctant to add

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
On July 1, 2015, 12:20 a.m., Adam B wrote: I'm sorry, but I don't understand how LOG(FATAL) was segfaulting before. Please explain. Joris Van Remoortere wrote: Hi Adam, This was reported as a segfault by a user in IRC. The specific segfault was fixed by MESOS-2636

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 36152: Updated CHANGELOG for 0.23.0.

2015-07-06 Thread Adam B
On July 3, 2015, 12:18 p.m., Vinod Kone wrote: Thanks for the review. I've addressed your comments in the following commit: commit 7f1d97c93a4492d4b9469301237ce96083f5408d Author: Adam B a...@mesosphere.io Date: Mon Jul 6 16:11:13 2015 -0700 Updated 0.23.0 CHANGELOG per Vinod's

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 35986: Allow slave attributes flag take a value with ':'.

2015-07-07 Thread Adam B
shows the leading/trailing ':' src/tests/attributes_tests.cpp (lines 55 - 63) https://reviews.apache.org/r/35986/#comment143875 These could be combined easily enough. - Adam B On July 7, 2015, 10:55 a.m., haosdent huang 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 35958: Updated SlaveTest.PingTimeoutNoPings test to use custom timeout values.

2015-06-29 Thread Adam B
after configurable ping timeout patch is applied. Thanks, Adam B

  1   2   3   4   5   6   7   8   >