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

> 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

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

dia) claim that javadoc only uses the opening tag, without the closing 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

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

ave tests for both of these. For (1) we could > > just add the shutdown message expectation to > > `PartitionTest.PartitionedSlave`. For (2), we have > > SlaveTest.PingTimeoutNoPings, SlaveTest.PingTimeoutNoPings. > > Adam B wrote: > Yes, I see now that SlaveTest

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

. Thanks, Adam B

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

t; (Updated June 22, 2015, 4:37 a.m.) > > > Review request for mesos, Adam B and Michael Park. > > > Bugs: MESOS-692 > https://issues.apache.org/jira/browse/MESOS-692 > > > Repository: mesos > > > Description > ---

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

ews.apache.org/r/29507/#review89305 --- On June 26, 2015, 3:12 a.m., Adam B wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

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.

--- 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.

e_tests.cpp (line 747) <https://reviews.apache.org/r/35728/#comment142093> s/MergeFrom/CopyFrom/ - Adam B On June 22, 2015, 4:34 a.m., haosdent huang wrote: > > --- >

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

--- 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 35717: Add reservations support to master's state.json

> 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 > > hashmap, where T is

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

x27;re probably fine just listing 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, v

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

s://reviews.apache.org/r/34943 I know it will require you to tokenize/iterate through the roles twice, but that's probably not much of a performance hit. - Adam B On June 21, 2015, 7:21 a.m., haosdent huang wrote: > > --- > T

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

> 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

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

--- 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 35721: Set the owner of persistent volumes to frameworkInfo.user .

rue (default). If it is false, all tasks will run 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: > > --- > T

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

Brian 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 genera

Re: Review Request 35756: Fixed a race condition in hook tests for remove-executor hook.

--- 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 29507: Added Configurable Slave Ping Timeouts

lly tested 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 29507: Added Configurable Slave Ping Timeouts

aves that encountered each type of a one-way partition, with both shorter and longer-than-default ping timeouts, largely for my own understanding of the process. Testing longer timeouts proved unnecessary, and testing unregistered slaves didn't actually test my fea

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

//reviews.apache.org/r/29507/diff/7/?file=973876#file973876line339> > > > > As this will be markdown, how about formatting max_slave_ping_timeouts > > in `` or as "--max_slave_ping_timeouts" ? Fixed. - Adam -----

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

! 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

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

--- 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.

--- 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.

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.

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

utomatically 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

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

- > 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

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

ommit 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 33339: Add a Java example framework to test persistent volumes.

> 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 commen

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

--- 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 - A

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

h::join("/", "", "/ab") 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp <https://reviews.apache.org/r/35179/#comment140338> I don't think a single-argument join makes any sense. We did

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

t removed. Please add them back or update 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 automat

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

e Mac 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., Marc

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

> 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

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

t; Probably better to recommend j= or j=<#GB RAM>, since j=8 could be overkill on a dual-core or smaller. - Adam B On June 2, 2015, 11:44 p.m., Marco Massenzio wrote: > > -

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

976/#comment138727> It was suggested that we should keep this document short&sweet 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 Masse

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

976/#comment138319> Can you merge this in with the Ubuntu 12.04 instructions below? I don't think they're drastically different, and we can probably assume 14.04 instead of 12.04 now. - Adam B On June 2, 2015, 7:25 p.m., Marco

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

gt; design state (here's to hoping we had a Design Doc for this API back in the > > day: this patch pre-dates me by more than two months!) so that when we come > > to the implementation stage, there are no longer discussions about URI > > structure, payload, etc. > >

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

l. To reply, visit: > https://reviews.apache.org/r/33058/ > --- > > (Updated 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 > >

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

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

> 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

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

ttps://reviews.apache.org/r/33295/#comment138103> What happened to your .WillOnce tabbing? Please make it a consistent 2-space indent. - Adam B On June 1, 2015, 8:52 a.m., Alexander Rojas wrote: > > ---

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

> 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 @pa

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

> 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 wro

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

g the executors array each time. 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(exec

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

#comment137786> Looks like a lot of duplicated code 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 "+&q

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

views.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 On May 26, 2015, 1:22 a.m., Alexander Rojas wrote: >

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

#review85628 --- On May 29, 2015, 7:47 a.m., Alexander Rojas wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30612/ > ---

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

/to Mesos master/to the Mesos master/ docs/modules.md <https://reviews.apache.org/r/34545/#comment137432> s/as an allocator/as the allocator/ There can be only one! docs/modules.md <https://reviews.apache.org/r/34545/#comment137433> `file://.json` is a little confusing

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

> 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

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

pache.org/r/29507/#review84726 --- On May 28, 2015, 4:13 p.m., Adam B wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://revie

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

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 29507: Added Configurable Slave Ping Timeouts

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

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

lease 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, 1

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

actual 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

Re: Review Request 34295: Added maintainers documentation.

alks 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 15, 2015, 3:25 p.m., Ben Mahler wrote: >

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

? Just ran across the fact that master/state.json doesn't seem to report executorInfos (and their resources). - Adam B On May 8, 2015, 3:23 p.m., Alexander Rojas wrote: > > --- > This is an automatically generated e-mail. T

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

--- 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 29507: Added Configurable Slave Ping Timeouts

r and ShortPingTimeoutUnreachableSlave Thanks, Adam B

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

--- 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

ping timeouts. `make check` with new unit tests: ShortPingTimeoutUnreachableMaster and ShortPingTimeoutUnreachableSlave Thanks, Adam B

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

t134593> s/pat/path/ src/master/http.cpp <https://reviews.apache.org/r/30612/#comment134594> Completed tasks? - Adam B On May 8, 2015, 3:23 p.m., Alexander Rojas wrote: > > ---

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

; https://reviews.apache.org/r/32587/ > --- > > (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:

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

would 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

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

> 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 &g

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

views.apache.org/r/33109/#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:

Re: Review Request 33490: MESOS-2633 Avoid adding duplicate tasks

o the Archive::contains() implementation, 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: > > --

Re: Review Request 31595: Changed mesos-frameworks torque page dead link

> 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 t

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

. 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

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

up creating two new tests. Both extract a framework, one sets > chown true and expects a failure due to the unknown user name. The other sets > chown to false and expects the fetcher to finish without error. > > Adam B wrote: > FYI, if you prefix the test name with `ROOT_` th

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

> On April 23, 2015, 12:25 a.m., Adam B wrote: > > src/cli/execute.cpp, line 249 > > <https://reviews.apache.org/r/33109/diff/5/?file=939687#file939687line249> > > > > Was this namespacing change necessary? > > haosdent huang wrote: > Bec

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

ail. To reply, visit: > https://reviews.apache.org/r/33295/ > --- > > (Updated April 22, 2015, 7:35 a.m.) > > > Review request for mesos, Adam B, Benjamin Hindman, and Till Toenshoff. > > > Bugs: MESOS-2620 > htt

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

109/#comment131657> Double blank line between top-level methods, please. - Adam B On April 22, 2015, 10:38 a.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://re

Re: Review Request 33257: Fixed recover tasks only by the intiated containerizer.

lt;https://reviews.apache.org/r/33257/#comment131594> s/recovering/to recover/ - Adam B On April 20, 2015, 2:18 p.m., Timothy Chen wrote: > > --- > This is an automatically generated e-mail. To reply,

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

iple times? Probably worth calling out somewhere (maybe just a comment 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: > > --- &g

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

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://r

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

> On April 8, 2015, 2:57 p.m., Vinod Kone wrote: > > src/tests/fetcher_tests.cpp, lines 112-116 > > > > > > Add a note/todo here mentioning why you are setting these fields but > > not doing any asserts/expects on the

<    3   4   5   6   7   8