Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.

2015-06-22 Thread Michael Park
> On June 22, 2015, 1:32 p.m., Alexander Rukletsov wrote: > > src/master/http.cpp, lines 519-520 > > > > > > Some food for thought here. I think the ultimate decision whether to > > grant a request or not, should be t

Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.

2015-06-22 Thread Michael Park
> On June 22, 2015, 10:20 a.m., Alexander Rukletsov wrote: > > Before making a thorough review, let's discuss one high level question. > > > > Here is the problem how I understand it: we reserve for roles, but our code > > works mostly with frameworks (allocator methods, Offer protobuf). To >

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

2015-06-22 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35438/#review88789 --- Patch looks great! Reviews applied: [35438] All tests passed. - M

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

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

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

2015-06-22 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35438/ --- (Updated June 22, 2015, 7:57 a.m.) Review request for mesos, Benjamin Hindman,

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

2015-06-22 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35438/ --- (Updated June 22, 2015, 7:46 a.m.) Review request for mesos, Adam B, Benjamin H

Re: Review Request 35703: Set refuse seconds on the correct filter in reservation test.

2015-06-22 Thread Michael Park
> On June 22, 2015, 2:13 p.m., Alexander Rukletsov wrote: > > Looks like a copy-paste bug. How have you found it? Was the test flaky? I was using these tests as a reference for writing my new tests for [r35702](https://reviews.apache.org/r/35702/) and noticed it. - Michael -

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

2015-06-22 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35438/#review88786 --- src/tests/fetcher_cache_tests.cpp (line 473)

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

2015-06-22 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35438/ --- (Updated June 22, 2015, 7:30 a.m.) Review request for mesos, Adam B, Benjamin H

Re: Review Request 35722: Really removed from stout.

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

Re: Review Request 35703: Set refuse seconds on the correct filter in reservation test.

2015-06-22 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35703/#review88784 --- Ship it! Looks like a copy-paste bug. How have you found it? Was th

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

2015-06-22 Thread Alexander Rukletsov
> On June 20, 2015, 7:35 p.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp, line 52 > > > > > > Let's avoid re-creating iterator: > > > > ``` > > for (auto it

Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.

2015-06-22 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/#review88781 --- src/master/http.cpp (lines 519 - 520)

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

2015-06-22 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35717/#review88779 --- Patch looks great! Reviews applied: [35717] All tests passed. - M

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

2015-06-22 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35694/#review88774 --- Ship it! 3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.

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

2015-06-22 Thread Till Toenshoff
> On June 20, 2015, 7:35 p.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp, line 52 > > > > > > Let's avoid re-creating iterator: > > > > ``` > > for (auto it

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

2015-06-22 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35728/#review88771 --- Patch looks great! Reviews applied: [35728] All tests passed. - M

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

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

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

2015-06-22 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35728/ --- (Updated June 22, 2015, 11:34 a.m.) Review request for mesos and Adam B. Bugs

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

2015-06-22 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35728/ --- Review request for mesos and Adam B. Repository: mesos Description --- F

Re: Review Request 34361: converted hard-coded strings to consts

2015-06-22 Thread Colin Williams
> On June 10, 2015, 1:25 a.m., Ben Mahler wrote: > > src/tests/master_tests.cpp, lines 3031-3034 > > > > > > Why bother with all this? Why not just have `"key1"`, `"value1"`, > > `"key2"`, `"value2"` inlined appropri

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-06-22 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/#review88761 --- Patch looks great! Reviews applied: [29507] All tests passed. - M

Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.

2015-06-22 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/#review88760 --- Before making a thorough review, let's discuss one high level questi

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/ --- (Updated June 22, 2015, 3:03 a.m.) Review request for mesos, Ben Mahler and Nik

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

2015-06-22 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35714/#review88759 --- Ship it! Ship It! - Alexander Rukletsov On June 22, 2015, 5:08 a

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-06-22 Thread Adam B
> On June 3, 2015, 2:24 p.m., Ben Mahler wrote: > > Overall it's looking pretty good! I'd suggest splitting out the tests given > > the comments on the source changes are fairly minor. > > > > Echoing Nik, it would be nice to have some sanity bound checks for this > > flag, to prevent operator

Re: Review Request 35722: Really removed from stout.

2015-06-22 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35722/#review88752 --- Ship it! Ship It! - Alexander Rukletsov On June 22, 2015, 5:59 a

Re: Review Request 33564: stout: Removed and switched from 'memory::' to 'std::'.

2015-06-22 Thread Joerg Schad
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33564/#review88750 --- Follow up in order to remove stout/memory.hpp: https://reviews.apac

Re: Review Request 35722: Really removed from stout.

2015-06-22 Thread Joerg Schad
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35722/#review88749 --- Ship it! - Joerg Schad On June 22, 2015, 5:59 a.m., Michael Park

Re: Review Request 35509: Doxygen Style Guide Improvements.

2015-06-22 Thread Joerg Schad
> On June 19, 2015, 10 a.m., Alexander Rojas wrote: > > docs/mesos-doxygen-style-guide.md, line 9 > > > > > > Just out of curiosity, why was this deleted? Prior this was followed by the -now- mesos-markdown-styleguide.

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-06-22 Thread Adam B
> On June 1, 2015, 1:15 p.m., Niklas Nielsen wrote: > > LGTM - Would it make sense to have sane min/max values for the > > timeouts/counts? > > > > I wonder it would make sense to have a test to exercise an upgrade path > > (the timeout being different in the slaves, than in the master). Maybe

<    1   2