Review Request 41752: Use Path::extension to simplify libprocess code.

2015-12-28 Thread Ben Mahler

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41752/
---

Review request for mesos, Anand Mazumdar and Artem Harutyunyan.


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/src/process.cpp cff635e74d22185de7ae767bc268ef4d56ad89f8 

Diff: https://reviews.apache.org/r/41752/diff/


Testing
---

make check


Thanks,

Ben Mahler



Review Request 41753: Use Path::extension to simplify mesos code.

2015-12-28 Thread Ben Mahler

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41753/
---

Review request for mesos, Anand Mazumdar and Artem Harutyunyan.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/files/files.cpp ec036ecc786bce3c7e64ad64e4f4205eea4a9d84 

Diff: https://reviews.apache.org/r/41753/diff/


Testing
---

make check


Thanks,

Ben Mahler



Review Request 41751: Added Path::extension for obtaining file extensions.

2015-12-28 Thread Ben Mahler

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41751/
---

Review request for mesos, Anand Mazumdar and Artem Harutyunyan.


Repository: mesos


Description
---

This was based on boost's extension method:
http://www.boost.org/doc/libs/1_59_0/libs/filesystem/doc/reference.html#path-extension

Cleanups to libprocess and mesos follow.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp 
0b986f0898da95c4cffd8bde1adfd9994d567096 
  3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp 
821dbb185f09e2f279d95fd354ce2168cddf1bac 

Diff: https://reviews.apache.org/r/41751/diff/


Testing
---

Added tests.


Thanks,

Ben Mahler



Re: Review Request 41751: Added Path::extension for obtaining file extensions.

2015-12-28 Thread Anand Mazumdar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41751/#review112069
---

Ship it!


LGTM, just some minor nits and cleanups.


3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp (line 169)


Nit: `const`



3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp (line 170)


Nit: `const`



3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp (line 174)


Can we remove the `else` here i.e. one level of nesting:

i.e.

``` 
if (_basename == "." || _basename == ".." || index == std::string::npos) {
  return None();
} 

return _basename.substr(index);
```


- Anand Mazumdar


On Dec. 28, 2015, 10:09 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41751/
> ---
> 
> (Updated Dec. 28, 2015, 10:09 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Artem Harutyunyan.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This was based on boost's extension method:
> http://www.boost.org/doc/libs/1_59_0/libs/filesystem/doc/reference.html#path-extension
> 
> Cleanups to libprocess and mesos follow.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp 
> 0b986f0898da95c4cffd8bde1adfd9994d567096 
>   3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp 
> 821dbb185f09e2f279d95fd354ce2168cddf1bac 
> 
> Diff: https://reviews.apache.org/r/41751/diff/
> 
> 
> Testing
> ---
> 
> Added tests.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>



Re: Review Request 41657: Added user facing documentation for containerizers.

2015-12-28 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41657/#review112068
---


Patch looks great!

Reviews applied: [41656, 41657]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 28, 2015, 8:23 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41657/
> ---
> 
> (Updated Dec. 28, 2015, 8:23 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This documentation is intended for users trying to get an overview of
> containerizer technology. It also provides the criteria for selecting a
> particular type of containerizer.
> 
> 
> Diffs
> -
> 
>   docs/containerizer.md PRE-CREATION 
>   docs/home.md d929838206817a6c49cc2343b4de82fa085da682 
> 
> Diff: https://reviews.apache.org/r/41657/diff/
> 
> 
> Testing
> ---
> 
> mesos website docker.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Re: Review Request 41752: Use Path::extension to simplify libprocess code.

2015-12-28 Thread Anand Mazumdar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41752/#review112072
---

Ship it!


Ship It!

- Anand Mazumdar


On Dec. 28, 2015, 10:09 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41752/
> ---
> 
> (Updated Dec. 28, 2015, 10:09 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Artem Harutyunyan.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> cff635e74d22185de7ae767bc268ef4d56ad89f8 
> 
> Diff: https://reviews.apache.org/r/41752/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>



Re: Review Request 41753: Use Path::extension to simplify mesos code.

2015-12-28 Thread Anand Mazumdar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41753/#review112073
---

Ship it!


Ship It!

- Anand Mazumdar


On Dec. 28, 2015, 10:09 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41753/
> ---
> 
> (Updated Dec. 28, 2015, 10:09 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Artem Harutyunyan.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp ec036ecc786bce3c7e64ad64e4f4205eea4a9d84 
> 
> Diff: https://reviews.apache.org/r/41753/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>



Re: Review Request 41714: Added tests for HDFS URI fetcher plugin.

2015-12-28 Thread Timothy Chen

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41714/#review112108
---



src/tests/uri_fetcher_tests.cpp (line 165)


ValidUri sounded like it was passed a vaild uri format instead of existing 
file. How about FetchExistingFile and FetchNonExistingFile?


- Timothy Chen


On Dec. 28, 2015, 6:07 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41714/
> ---
> 
> (Updated Dec. 28, 2015, 6:07 p.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, Ian Downes, Timothy Chen, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-3925
> https://issues.apache.org/jira/browse/MESOS-3925
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests for HDFS URI fetcher plugin.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8af0115caa67ac8f3193d8f0d0f1a4ae739af275 
>   src/tests/uri_fetcher_tests.cpp d54a7e7b08a741b1d5d371c11747681ae4ddef98 
>   src/uri/schemes/hdfs.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41714/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Review Request 41675: Speed up SlaveTest.HTTPSchedulerSlaveRestart

2015-12-28 Thread Jian Qiu

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41675/
---

Review request for mesos and Alexander Rukletsov.


Bugs: MESOS-4163
https://issues.apache.org/jira/browse/MESOS-4163


Repository: mesos


Description
---

We should advance a reregister timeout to speed up the slave registration.


Diffs
-

  src/tests/slave_tests.cpp 90d56b987c60b99d9ca3e4ffef9cb71815bfc9b7 

Diff: https://reviews.apache.org/r/41675/diff/


Testing
---

make check -j4 GTEST_FILTER=SlaveTest.HTTPSchedulerSlaveRestart

[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from SlaveTest
[ RUN  ] SlaveTest.HTTPSchedulerSlaveRestart
I1229 09:38:10.183754 2138071808 exec.cpp:134] Version: 0.27.0
I1229 09:38:10.188813 328146944 exec.cpp:208] Executor registered on slave 
97a7a987-6edb-4ee1-a79f-4de08bc0af10-S0
Registered executor on 9.110.49.60
Starting task 4f4f7ebe-625c-42eb-aaa8-d569333751e8
sh -c 'sleep 1000'
Forked command at 31243
I1229 09:38:10.212271 329220096 exec.cpp:254] Received reconnect request from 
slave 97a7a987-6edb-4ee1-a79f-4de08bc0af10-S0
I1229 09:38:10.212829 327610368 exec.cpp:231] Executor re-registered on slave 
97a7a987-6edb-4ee1-a79f-4de08bc0af10-S0
Re-registered executor on 9.110.49.60
[   OK ] SlaveTest.HTTPSchedulerSlaveRestart (417 ms)
[--] 1 test from SlaveTest (417 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (426 ms total)
[  PASSED  ] 1 test.


Thanks,

Jian Qiu



Re: Review Request 41675: Speed up SlaveTest.HTTPSchedulerSlaveRestart

2015-12-28 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41675/#review112113
---


Patch looks great!

Reviews applied: [41675]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 29, 2015, 1:48 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41675/
> ---
> 
> (Updated Dec. 29, 2015, 1:48 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Anand Mazumdar.
> 
> 
> Bugs: MESOS-4163
> https://issues.apache.org/jira/browse/MESOS-4163
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We should advance a reregister timeout to speed up the slave registration.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 90d56b987c60b99d9ca3e4ffef9cb71815bfc9b7 
> 
> Diff: https://reviews.apache.org/r/41675/diff/
> 
> 
> Testing
> ---
> 
> make check -j4 GTEST_FILTER=SlaveTest.HTTPSchedulerSlaveRestart
> 
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from SlaveTest
> [ RUN  ] SlaveTest.HTTPSchedulerSlaveRestart
> I1229 09:38:10.183754 2138071808 exec.cpp:134] Version: 0.27.0
> I1229 09:38:10.188813 328146944 exec.cpp:208] Executor registered on slave 
> 97a7a987-6edb-4ee1-a79f-4de08bc0af10-S0
> Registered executor on 9.110.49.60
> Starting task 4f4f7ebe-625c-42eb-aaa8-d569333751e8
> sh -c 'sleep 1000'
> Forked command at 31243
> I1229 09:38:10.212271 329220096 exec.cpp:254] Received reconnect request from 
> slave 97a7a987-6edb-4ee1-a79f-4de08bc0af10-S0
> I1229 09:38:10.212829 327610368 exec.cpp:231] Executor re-registered on slave 
> 97a7a987-6edb-4ee1-a79f-4de08bc0af10-S0
> Re-registered executor on 9.110.49.60
> [   OK ] SlaveTest.HTTPSchedulerSlaveRestart (417 ms)
> [--] 1 test from SlaveTest (417 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (426 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>



Re: Review Request 41600: Speed up SlaveTest.CommandExecutorWithOverride

2015-12-28 Thread Alexander Rukletsov

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41600/#review111967
---



src/tests/slave_tests.cpp (line 435)


Mind explaining why do you think explicitly killing the executor is the 
right solution here? I would even propose to do it in MESOS-4161 directly for 
posterity.

To clarify some approaches here for posterity (I will be glad to continue 
the conversation in the JIRA ticket once you post your explanation there). 
There are two separate techniques used in this review request: explicit process 
kill and `Clock::advance()`. Though the latter may indeed slightly speed up the 
test, it's not longer necessary since `process::reap()` has been accelerated 
(see MESOS-1199). I would let @Jian Qiu comment on the former approach.


- Alexander Rukletsov


On Dec. 23, 2015, 6:43 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41600/
> ---
> 
> (Updated Dec. 23, 2015, 6:43 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4161
> https://issues.apache.org/jira/browse/MESOS-4161
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To speed up SlaveTest.CommandExecutorWithOverride, we need to explicitly kill 
> the executor process and advance reap interval.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 90d56b987c60b99d9ca3e4ffef9cb71815bfc9b7 
> 
> Diff: https://reviews.apache.org/r/41600/diff/
> 
> 
> Testing
> ---
> 
> make check -j4 GTEST_FILTER=SlaveTest.CommandExecutorWithOverride takes 415ms
> 
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from SlaveTest
> [ RUN  ] SlaveTest.CommandExecutorWithOverride
> [   OK ] SlaveTest.CommandExecutorWithOverride (405 ms)
> [--] 1 test from SlaveTest (405 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (415 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-28 Thread Avinash sridharan


> On Dec. 24, 2015, 11:10 a.m., Adam B wrote:
> > Looks good, but I wonder if we need to go so far as to introduce the `enum 
> > Protocol` misnomer in the global IPAddress message now. We could always add 
> > it in later, when we actually get NetworkInfo off of it.
> 
> Anand Mazumdar wrote:
> 1. Adam, can you elaborate a bit more on why do you think the `Protocol` 
> field is a misnomer here ? Are you alluding for that field to be called 
> `version` or something else to suit it better i.e. something like:
> 
> ```
> enum Version {
>   IPV4 = 1;
>   IPV6 = 2;
> }
> ```
> 
> 2. Also, we shouldn't worry too much about how the network isolator deals 
> with the `Protocol` field being set/unset, since that is relevant business 
> logic limited to its functionality and should not influence how the 
> representation of `IPAddress` should look like. We should only be concerned 
> about wire compatibility if we intend to migrate `NetworkInfo` to use this 
> message later. Were you referring to the fact that just renaming `Protocol` 
> to `Version` or something else would make it wire incompatible with the old 
> message inside `NetworkInfo` and hence we should not do it now ?
> 
> PS: I am fine with not introducing the `Protocol` field for now like you 
> suggested. I just wanted to be sure about the reasoning/problems with not 
> doing it now and leaving it off for later.

Will remove the Protocol field for the time being and set the position 
identifier for the ip_address field to 2.


- Avinash


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41380/#review111848
---


On Dec. 23, 2015, 3 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41380/
> ---
> 
> (Updated Dec. 23, 2015, 3 a.m.)
> 
> 
> Review request for mesos, Adam B and Anand Mazumdar.
> 
> 
> Bugs: MESOS-4114
> https://issues.apache.org/jira/browse/MESOS-4114
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added repeated vip field to DiscoveryInfo and an instance_port field to Port
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 2431fdd6b84625c6140a2b3913736bffada4e7f6 
>   include/mesos/v1/mesos.proto 4aed0980b28dc1000aa2821f35303b736bc5bff8 
> 
> Diff: https://reviews.apache.org/r/41380/diff/
> 
> 
> Testing
> ---
> 
> make check, and make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 41461: stout: Added SFINAE-friendly `result_of`.

2015-12-28 Thread Michael Park

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41461/
---

(Updated Dec. 28, 2015, 3:42 p.m.)


Review request for mesos, Alex Clemmer and Joris Van Remoortere.


Bugs: MESOS-4220
https://issues.apache.org/jira/browse/MESOS-4220


Repository: mesos


Description
---

VS 2015 won't support C++14 `std::result_of` SFINAE until Update 2, so 
`result_of` must be replaced with `decltype(invoke)`.

Here, we implement SFINAE `result_of` in `stout`.

Follow-up from [r40114](https://reviews.apache.org/r/40114/).


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe 
  3rdparty/libprocess/3rdparty/stout/include/stout/result_of.hpp PRE-CREATION 

Diff: https://reviews.apache.org/r/41461/diff/


Testing
---

`make check` on OS X, compiled on Windows.


Thanks,

Michael Park



Re: Review Request 41460: Used `std::is_bind_expression` to SFINAE correctly.

2015-12-28 Thread Michael Park

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41460/
---

(Updated Dec. 28, 2015, 3:42 p.m.)


Review request for mesos, Alex Clemmer and Joris Van Remoortere.


Bugs: MESOS-4228
https://issues.apache.org/jira/browse/MESOS-4228


Repository: mesos


Description
---

The Standard (C++11 through 17) does not require `std::bind`'s function call 
operator to SFINAE, and VS 2015's doesn't. `std::is_bind_expression` can be 
used to manually reroute bind expressions to the 1-arg overload, where 
(conveniently) the argument will be ignored if necessary.

Follow-up from [r40114](https://reviews.apache.org/r/40114/).


Diffs
-

  3rdparty/libprocess/include/process/future.hpp 
c9146e3a3ccf09dd37c5a8ac7000fbe84f3c710c 

Diff: https://reviews.apache.org/r/41460/diff/


Testing
---

`make check` on OS X, compiled on Windows.


Thanks,

Michael Park



Re: Review Request 41462: libprocess: Used SFINAE-friendly `result_of`.

2015-12-28 Thread Michael Park

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41462/
---

(Updated Dec. 28, 2015, 3:42 p.m.)


Review request for mesos, Alex Clemmer and Joris Van Remoortere.


Bugs: MESOS-4220
https://issues.apache.org/jira/browse/MESOS-4220


Repository: mesos


Description
---

Used the SFINAE `result_of` implemented in 
[r41461](https://reviews.apache.org/r/41461/).

Follow-up from [r40114](https://reviews.apache.org/r/40114/).


Diffs
-

  3rdparty/libprocess/include/process/future.hpp 
c9146e3a3ccf09dd37c5a8ac7000fbe84f3c710c 

Diff: https://reviews.apache.org/r/41462/diff/


Testing
---

`make check` on OS X, compiled on Windows.


Thanks,

Michael Park



Re: Review Request 41461: stout: Added SFINAE-friendly `result_of`.

2015-12-28 Thread Alexander Rukletsov

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41461/#review111984
---


Do you think it makes sense to add some configure checks?


3rdparty/libprocess/3rdparty/stout/include/stout/result_of.hpp (lines 21 - 22)


Do you want to reference MESOS-3993? Maybe update the ticket as well, once 
this lands.



3rdparty/libprocess/3rdparty/stout/include/stout/result_of.hpp (lines 33 - 34)


Let's make a note that `std::invoke()`, which is c++17 AFAIK, is supported 
by msvc 2015 and it's not in .



3rdparty/libprocess/3rdparty/stout/include/stout/result_of.hpp (line 41)


`int` is dummy here, right? Mind writing a comment about it?


- Alexander Rukletsov


On Dec. 28, 2015, 3:42 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41461/
> ---
> 
> (Updated Dec. 28, 2015, 3:42 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4220
> https://issues.apache.org/jira/browse/MESOS-4220
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> VS 2015 won't support C++14 `std::result_of` SFINAE until Update 2, so 
> `result_of` must be replaced with `decltype(invoke)`.
> 
> Here, we implement SFINAE `result_of` in `stout`.
> 
> Follow-up from [r40114](https://reviews.apache.org/r/40114/).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe 
>   3rdparty/libprocess/3rdparty/stout/include/stout/result_of.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41461/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OS X, compiled on Windows.
> 
> 
> Thanks,
> 
> Michael Park
> 
>



Re: Review Request 41462: libprocess: Used SFINAE-friendly `result_of`.

2015-12-28 Thread Alexander Rukletsov

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41462/#review112021
---


I see some `result_of` in "async.hpp" and "lambda.hpp". Do you want to update 
those as well?

- Alexander Rukletsov


On Dec. 28, 2015, 3:42 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41462/
> ---
> 
> (Updated Dec. 28, 2015, 3:42 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4220
> https://issues.apache.org/jira/browse/MESOS-4220
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used the SFINAE `result_of` implemented in 
> [r41461](https://reviews.apache.org/r/41461/).
> 
> Follow-up from [r40114](https://reviews.apache.org/r/40114/).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> c9146e3a3ccf09dd37c5a8ac7000fbe84f3c710c 
> 
> Diff: https://reviews.apache.org/r/41462/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OS X, compiled on Windows.
> 
> 
> Thanks,
> 
> Michael Park
> 
>



Re: Review Request 41459: Invoked `_Deferred`'s `operator F()` explicitly.

2015-12-28 Thread Michael Park

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41459/
---

(Updated Dec. 28, 2015, 3:42 p.m.)


Review request for mesos, Alex Clemmer and Joris Van Remoortere.


Bugs: MESOS-4221
https://issues.apache.org/jira/browse/MESOS-4221


Repository: mesos


Description
---

VS 2015 won't support C++14 `std::function` SFINAE until Update 2, so 
converting `_Deferred` to `std::function` must be done by explicitly calling 
`_Deferred`'s conversion function.

Follow-up from [r40114](https://reviews.apache.org/r/40114/).


Diffs
-

  3rdparty/libprocess/include/process/future.hpp 
c9146e3a3ccf09dd37c5a8ac7000fbe84f3c710c 

Diff: https://reviews.apache.org/r/41459/diff/


Testing
---

`make check` on OS X, compiled on Windows.


Thanks,

Michael Park



Re: Review Request 41731: Removed docker puller flag.

2015-12-28 Thread Gilbert Song


> On Dec. 26, 2015, 9:42 p.m., Gilbert Song wrote:
> > src/slave/flags.cpp, lines 145-146
> > 
> >
> > Could we use pure directory only for local puller?
> > 
> > Instead of adding `file://` to the flag & strings::remove it, because 
> > some users may easily neglect a slash for this pattern.
> 
> Timothy Chen wrote:
> We need to differentiate based on the flag's value to be local or 
> registry as we consolidated a bunch of flags down to a single one now.

Is it possible to select local or registry by:

`if (!strings::startsWith(imagesUrl, "https://;))`

Not 100% sure it is safe enough. If no, we could drop this issue. However, we 
may still consider user with `--docker_image=file:///tmp/mesos/images/docker` 
VS `--docker_image=/tmp/mesos/images/docker` in the future.


- Gilbert


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41731/#review111921
---


On Dec. 26, 2015, 4:22 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> ---
> 
> (Updated Dec. 26, 2015, 4:22 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed docker puller flag.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f71b572a32443a6715acb4d3541aec60e0437b30 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> dd17acf6a6e5c306029198dbb2a7e2d059f87f75 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 7cdb15b9529eab82867b3470a016bb8ad09ff250 
>   src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9 
>   src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98 
> 
> Diff: https://reviews.apache.org/r/41731/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 41731: Removed docker puller flag.

2015-12-28 Thread Timothy Chen


> On Dec. 27, 2015, 5:42 a.m., Gilbert Song wrote:
> > src/slave/flags.cpp, lines 145-146
> > 
> >
> > Could we use pure directory only for local puller?
> > 
> > Instead of adding `file://` to the flag & strings::remove it, because 
> > some users may easily neglect a slash for this pattern.
> 
> Timothy Chen wrote:
> We need to differentiate based on the flag's value to be local or 
> registry as we consolidated a bunch of flags down to a single one now.
> 
> Gilbert Song wrote:
> Is it possible to select local or registry by:
> 
> `if (!strings::startsWith(imagesUrl, "https://;))`
> 
> Not 100% sure it is safe enough. If no, we could drop this issue. 
> However, we may still consider user with 
> `--docker_image=file:///tmp/mesos/images/docker` VS 
> `--docker_image=/tmp/mesos/images/docker` in the future.

That's just not possible if we stick with this configuration.
Btw Jie I just realize that the downside of consolidating flags like this is 
that, users that want to use the docker registry will then have to specify the 
docker registry URL themselves everytime now, which we used to have a default 
value for a seperate flag. To me that's a pretty bad experience as well. What 
did you guys conclude last time when you guys discussed about this?


- Timothy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41731/#review111921
---


On Dec. 27, 2015, 12:22 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> ---
> 
> (Updated Dec. 27, 2015, 12:22 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed docker puller flag.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f71b572a32443a6715acb4d3541aec60e0437b30 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> dd17acf6a6e5c306029198dbb2a7e2d059f87f75 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 7cdb15b9529eab82867b3470a016bb8ad09ff250 
>   src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9 
>   src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98 
> 
> Diff: https://reviews.apache.org/r/41731/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 41604: CMake: Added missing protobuf files to CMake build.

2015-12-28 Thread Daniel Pravat

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41604/#review112019
---

Ship it!


Ship It!

- Daniel Pravat


On Dec. 23, 2015, 6:52 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41604/
> ---
> 
> (Updated Dec. 23, 2015, 6:52 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Diana Arroyo, Daniel Pravat, Artem 
> Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Added missing protobuf files to CMake build.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt bdc45ae604c940dadc27ab6e8b8a3327bd00642b 
> 
> Diff: https://reviews.apache.org/r/41604/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Review Request 41754: Added reference to docker registry bearer token spec.

2015-12-28 Thread Jojy Varghese

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41754/
---

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added reference to docker registry bearer token spec.


Diffs
-

  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
93e7b3b466283a7732290419060ebedad3445133 
  src/slave/containerizer/mesos/provisioner/docker/token_manager.hpp 
3a01aca659ca2b31d0b31ec17382c953cbfc1c2b 

Diff: https://reviews.apache.org/r/41754/diff/


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 41754: Added reference to docker registry bearer token spec.

2015-12-28 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41754/#review112080
---


Patch looks great!

Reviews applied: [41754]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 28, 2015, 10 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41754/
> ---
> 
> (Updated Dec. 28, 2015, 10 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added reference to docker registry bearer token spec.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> 93e7b3b466283a7732290419060ebedad3445133 
>   src/slave/containerizer/mesos/provisioner/docker/token_manager.hpp 
> 3a01aca659ca2b31d0b31ec17382c953cbfc1c2b 
> 
> Diff: https://reviews.apache.org/r/41754/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Re: Review Request 40944: Fixed protobuf parse failure when pulling a docker image.

2015-12-28 Thread Gilbert Song

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40944/
---

(Updated Dec. 28, 2015, 10:31 a.m.)


Review request for mesos, Artem Harutyunyan, Jojy Varghese, and Timothy Chen.


Repository: mesos


Description
---

Fixed protobuf parse failure when pulling a docker image.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
7cdb15b9529eab82867b3470a016bb8ad09ff250 
  src/slave/containerizer/mesos/provisioner/docker/spec.cpp 
5bbd98cb1c5f3aefd9050859d066b43360e6eb75 
  src/slave/containerizer/mesos/provisioner/docker/v2.proto 
fdf159d003f8e5718de0f4d8bf9a2b1374cc11a2 
  src/tests/containerizer/provisioner_docker_tests.cpp 
bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98 

Diff: https://reviews.apache.org/r/40944/diff/


Testing
---

make check(ubuntu14.04 + clang3.6)


Thanks,

Gilbert Song



Re: Review Request 41661: Added documentation for API versioning.

2015-12-28 Thread Jojy Varghese


> On Dec. 27, 2015, 3:25 a.m., Jojy Varghese wrote:
> > docs/versioning.md, line 62
> > 
> >
> > This section could use some formatting. 
> > 
> > Also, would be useful to take the reader through a complete example 
> > covering a message's external and internal path.
> 
> Anand Mazumdar wrote:
> Can you elaborate a bit more on what did you mean by:
> 
> ```This section could use some formatting.```

With the formatting fixed for the protobuff section, this looks good now. So we 
dont need formatting here. Dropping this one.

Could we walk the uninitiated reader through an example of how the protobuff 
versioning is handled internally and externally for 1 message?


- Jojy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41661/#review111919
---


On Dec. 27, 2015, 9:08 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41661/
> ---
> 
> (Updated Dec. 27, 2015, 9:08 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4192
> https://issues.apache.org/jira/browse/MESOS-4192
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds documentation on how Mesos does API versioning. It also has 
> a section:
> - On how API versioning and Release versioning are different.
> - API compatibility/upgrade guarantees.
> - Implementation Details.
> 
> Most of the information is taken from the design doc on Mesos HTTP API 
> Versioning:
> https://docs.google.com/document/d/1-iQjo6778H_fU_1Zi_Yk6szg8qj-wqYgVgnx7u3h6OU/edit#
> 
> 
> Diffs
> -
> 
>   docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
>   docs/versioning.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41661/diff/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/hatred/1cc6db05d0ca51397886
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 41715: Support parsing url in libprocess.

2015-12-28 Thread Timothy Chen


> On Dec. 28, 2015, 8:02 p.m., Jojy Varghese wrote:
> > 3rdparty/libprocess/src/http.cpp, line 183
> > 
> >
> > Have we considered using a regex parser for doing this? C++11 regex 
> > support is added since gcc 4.9 and has been in clang for sometime now.

We can do this later, I took the toURL method you did in the registry client, 
made it more general moved it into libprocess for now.
I can add a TODO later for making it better.


- Timothy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41715/#review112061
---


On Dec. 26, 2015, 8:24 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41715/
> ---
> 
> (Updated Dec. 26, 2015, 8:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support parsing url in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> f0666f0fa48c4f3a98332d12066561a02a715236 
>   3rdparty/libprocess/src/http.cpp e937df6875c8024ea9c178833b9faceede990969 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 19261502be220aaa40add7ce30a9b2b65d1d9fdc 
> 
> Diff: https://reviews.apache.org/r/41715/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 41705: Added support for enforcing quota on (persistent) volumes (MESOS-4198).

2015-12-28 Thread Artem Harutyunyan

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41705/
---

(Updated Dec. 28, 2015, 11:36 a.m.)


Review request for mesos and Jie Yu.


Summary (updated)
-

Added support for enforcing quota on (persistent) volumes (MESOS-4198).


Bugs: MESOS-4198
https://issues.apache.org/jira/browse/MESOS-4198


Repository: mesos


Description
---

Added support for enforcing quota on (persistent) volumes (MESOS-4198).


Diffs
-

  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
d971db09083faad08f3cf18c25a79245321d1d9a 
  src/tests/disk_quota_tests.cpp ce736c32a9e78a8a6d0793a06fe87911b9b0558d 

Diff: https://reviews.apache.org/r/41705/diff/


Testing
---

GTEST_FILTER='DiskQuotaTest*' make check -j8


Thanks,

Artem Harutyunyan



Re: Review Request 41704: Added support for checking whether a given path is absolute.

2015-12-28 Thread Artem Harutyunyan

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41704/
---

(Updated Dec. 28, 2015, 11:35 a.m.)


Review request for mesos and Jie Yu.


Summary (updated)
-

Added support for checking whether a given path is absolute.


Bugs: MESOS-4198
https://issues.apache.org/jira/browse/MESOS-4198


Repository: mesos


Description
---

Added support for checking whether a given path is absolute.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp 
0b986f0898da95c4cffd8bde1adfd9994d567096 
  3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp 
821dbb185f09e2f279d95fd354ce2168cddf1bac 

Diff: https://reviews.apache.org/r/41704/diff/


Testing
---

Ran stout tests.


Thanks,

Artem Harutyunyan



Re: Review Request 41657: Added user facing documentation for containerizers.

2015-12-28 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41657/#review112060
---


Bad patch!

Reviews applied: [41656]

Failed command: ./support/apply-review.sh -n -r 41656

Error:
 2015-12-28 20:00:51 URL:https://reviews.apache.org/r/41656/diff/raw/ 
[2380/2380] -> "41656.patch" [1]
error: patch failed: docs/home.md:15
error: docs/home.md: patch does not apply

- Mesos ReviewBot


On Dec. 28, 2015, 7:49 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41657/
> ---
> 
> (Updated Dec. 28, 2015, 7:49 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This documentation is intended for users trying to get an overview of
> containerizer technology. It also provides the criteria for selecting a
> particular type of containerizer.
> 
> 
> Diffs
> -
> 
>   docs/containerizer.md PRE-CREATION 
>   docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
> 
> Diff: https://reviews.apache.org/r/41657/diff/
> 
> 
> Testing
> ---
> 
> mesos website docker.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Re: Review Request 41715: Support parsing url in libprocess.

2015-12-28 Thread Timothy Chen


> On Dec. 27, 2015, 2:56 a.m., Jojy Varghese wrote:
> > 3rdparty/libprocess/src/http.cpp, line 182
> > 
> >
> > I would add some comments through the function to explain each 
> > sub-section's intent.

We usually try not to comment on everything unless it's confusing or not easy 
to understand. Which parts you feel like we need to comment on if it's not easy 
to understand?


- Timothy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41715/#review111918
---


On Dec. 26, 2015, 8:24 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41715/
> ---
> 
> (Updated Dec. 26, 2015, 8:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support parsing url in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> f0666f0fa48c4f3a98332d12066561a02a715236 
>   3rdparty/libprocess/src/http.cpp e937df6875c8024ea9c178833b9faceede990969 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 19261502be220aaa40add7ce30a9b2b65d1d9fdc 
> 
> Diff: https://reviews.apache.org/r/41715/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 40944: Fixed protobuf parse failure when pulling a docker image.

2015-12-28 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40944/#review112045
---


Patch looks great!

Reviews applied: [40944]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 28, 2015, 6:31 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40944/
> ---
> 
> (Updated Dec. 28, 2015, 6:31 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jojy Varghese, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed protobuf parse failure when pulling a docker image.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 7cdb15b9529eab82867b3470a016bb8ad09ff250 
>   src/slave/containerizer/mesos/provisioner/docker/spec.cpp 
> 5bbd98cb1c5f3aefd9050859d066b43360e6eb75 
>   src/slave/containerizer/mesos/provisioner/docker/v2.proto 
> fdf159d003f8e5718de0f4d8bf9a2b1374cc11a2 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98 
> 
> Diff: https://reviews.apache.org/r/40944/diff/
> 
> 
> Testing
> ---
> 
> make check(ubuntu14.04 + clang3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>



Re: Review Request 41657: Added user facing documentation for containerizers.

2015-12-28 Thread Jojy Varghese

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41657/
---

(Updated Dec. 28, 2015, 7:49 p.m.)


Review request for mesos, Jie Yu and Joerg Schad.


Changes
---

review(joerg) addressed.


Repository: mesos


Description
---

This documentation is intended for users trying to get an overview of
containerizer technology. It also provides the criteria for selecting a
particular type of containerizer.


Diffs (updated)
-

  docs/containerizer.md PRE-CREATION 
  docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 

Diff: https://reviews.apache.org/r/41657/diff/


Testing
---

mesos website docker.


Thanks,

Jojy Varghese



Re: Review Request 41715: Support parsing url in libprocess.

2015-12-28 Thread Jojy Varghese

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41715/#review112061
---



3rdparty/libprocess/src/http.cpp (line 183)


Have we considered using a regex parser for doing this? C++11 regex support 
is added since gcc 4.9 and has been in clang for sometime now.


- Jojy Varghese


On Dec. 26, 2015, 8:24 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41715/
> ---
> 
> (Updated Dec. 26, 2015, 8:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support parsing url in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> f0666f0fa48c4f3a98332d12066561a02a715236 
>   3rdparty/libprocess/src/http.cpp e937df6875c8024ea9c178833b9faceede990969 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 19261502be220aaa40add7ce30a9b2b65d1d9fdc 
> 
> Diff: https://reviews.apache.org/r/41715/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 41715: Support parsing url in libprocess.

2015-12-28 Thread Timothy Chen


> On Dec. 27, 2015, 2:56 a.m., Jojy Varghese wrote:
> > 3rdparty/libprocess/src/http.cpp, line 190
> > 
> >
> > Can we avoid using magic numbers ('3' here)?

It's paired with the magic string '://' :) I'm not sure what I would even call 
this if we move it out.


- Timothy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41715/#review111918
---


On Dec. 26, 2015, 8:24 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41715/
> ---
> 
> (Updated Dec. 26, 2015, 8:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support parsing url in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> f0666f0fa48c4f3a98332d12066561a02a715236 
>   3rdparty/libprocess/src/http.cpp e937df6875c8024ea9c178833b9faceede990969 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 19261502be220aaa40add7ce30a9b2b65d1d9fdc 
> 
> Diff: https://reviews.apache.org/r/41715/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 41715: Support parsing url in libprocess.

2015-12-28 Thread Timothy Chen

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41715/
---

(Updated Dec. 28, 2015, 8:23 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.


Repository: mesos


Description
---

Support parsing url in libprocess.


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp 
f0666f0fa48c4f3a98332d12066561a02a715236 
  3rdparty/libprocess/src/http.cpp e937df6875c8024ea9c178833b9faceede990969 
  3rdparty/libprocess/src/tests/http_tests.cpp 
19261502be220aaa40add7ce30a9b2b65d1d9fdc 

Diff: https://reviews.apache.org/r/41715/diff/


Testing
---

make check


Thanks,

Timothy Chen



Re: Review Request 41657: Added user facing documentation for containerizers.

2015-12-28 Thread Jojy Varghese

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41657/
---

(Updated Dec. 28, 2015, 8:23 p.m.)


Review request for mesos, Jie Yu and Joerg Schad.


Changes
---

rebased with master.


Repository: mesos


Description
---

This documentation is intended for users trying to get an overview of
containerizer technology. It also provides the criteria for selecting a
particular type of containerizer.


Diffs (updated)
-

  docs/containerizer.md PRE-CREATION 
  docs/home.md d929838206817a6c49cc2343b4de82fa085da682 

Diff: https://reviews.apache.org/r/41657/diff/


Testing
---

mesos website docker.


Thanks,

Jojy Varghese



Re: Review Request 41656: Renamed containerizer.md to mesos-containerizer.md.

2015-12-28 Thread Jojy Varghese

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41656/
---

(Updated Dec. 28, 2015, 8:23 p.m.)


Review request for mesos, Jie Yu and Joerg Schad.


Changes
---

rebased with master


Repository: mesos


Description
---

The change reflects the content of the documentation. A separate document that
  describes containerizer from a user perspective will be created.


Diffs (updated)
-

  docs/containerizer.md  
  docs/home.md d929838206817a6c49cc2343b4de82fa085da682 
  docs/mesos-provisioner.md fdb298c2a954e903317ef56abbcfe2470a2dfd23 

Diff: https://reviews.apache.org/r/41656/diff/


Testing
---

mesos website docker.


Thanks,

Jojy Varghese



Re: Review Request 41283: Introduced an Executor Library based on the new executor HTTP API.

2015-12-28 Thread Anand Mazumdar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41283/
---

(Updated Dec. 29, 2015, 3:11 a.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Review comments.


Bugs: MESOS-3550
https://issues.apache.org/jira/browse/MESOS-3550


Repository: mesos


Description
---

This change introduces the implementation for the executor library. 

This uses the new HTTP Connection interface to ensure calls are properly 
pipelined.

`connected` -> Callback invoked when a TCP connection is established with the 
agent.
`disconnected` -> When the TCP connection is interrupted possibly due to an 
agent restart.
`received` -> When the executor receives events from the agent upon subscribing.


Diffs (updated)
-

  src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
  src/executor/executor.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/41283/diff/


Testing
---

make check


Thanks,

Anand Mazumdar



Re: Review Request 41283: Introduced an Executor Library based on the new executor HTTP API.

2015-12-28 Thread Anand Mazumdar


> On Dec. 29, 2015, 2:46 a.m., Jojy Varghese wrote:
> > src/executor/executor.cpp, line 221
> > 
> >
> > "else" case?

Nothing needs to be done for the `else` case. These env variables **might** be 
set by the agent when framework checkpointing is enabled. For more info see how 
the old executor library checks it:
https://github.com/apache/mesos/blob/master/src/exec/exec.cpp#L690


> On Dec. 29, 2015, 2:46 a.m., Jojy Varghese wrote:
> > src/executor/executor.cpp, line 232
> > 
> >
> > "else" case?

Ditto as above


> On Dec. 29, 2015, 2:46 a.m., Jojy Varghese wrote:
> > src/executor/executor.cpp, line 266
> > 
> >
> > Why not use universal initialization ({})?

`Request` struct has a lot of fields all of which are not used by us. Hence, 
univeral initialization is not possible. 
https://github.com/apache/mesos/blob/master/3rdparty/libprocess/include/process/http.hpp#L224


- Anand


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41283/#review112114
---


On Dec. 29, 2015, 12:11 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41283/
> ---
> 
> (Updated Dec. 29, 2015, 12:11 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3550
> https://issues.apache.org/jira/browse/MESOS-3550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces the implementation for the executor library. 
> 
> This uses the new HTTP Connection interface to ensure calls are properly 
> pipelined.
> 
> `connected` -> Callback invoked when a TCP connection is established with the 
> agent.
> `disconnected` -> When the TCP connection is interrupted possibly due to an 
> agent restart.
> `received` -> When the executor receives events from the agent upon 
> subscribing.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
>   src/executor/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41283/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 41283: Introduced an Executor Library based on the new executor HTTP API.

2015-12-28 Thread Jojy Varghese

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41283/#review112114
---



src/executor/executor.cpp (line 54)


Do you really need the entire slave namespace or something narrower like 
``` mesos::internal::slave::validation::executor```



src/executor/executor.cpp (line 118)


const?



src/executor/executor.cpp (line 183)


Why not UPID ```upid(value.get());```



src/executor/executor.cpp (line 221)


"else" case?



src/executor/executor.cpp (line 232)


"else" case?



src/executor/executor.cpp (line 255)


See comment about namespace scope.



src/executor/executor.cpp (line 256)


why blank line?



src/executor/executor.cpp (line 264)


Why declare response far from its use? Its preferred that the declarations 
be closer to their use.



src/executor/executor.cpp (line 266)


Why not use universal initialization ({})?


- Jojy Varghese


On Dec. 29, 2015, 12:11 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41283/
> ---
> 
> (Updated Dec. 29, 2015, 12:11 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3550
> https://issues.apache.org/jira/browse/MESOS-3550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces the implementation for the executor library. 
> 
> This uses the new HTTP Connection interface to ensure calls are properly 
> pipelined.
> 
> `connected` -> Callback invoked when a TCP connection is established with the 
> agent.
> `disconnected` -> When the TCP connection is interrupted possibly due to an 
> agent restart.
> `received` -> When the executor receives events from the agent upon 
> subscribing.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
>   src/executor/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41283/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 41283: Introduced an Executor Library based on the new executor HTTP API.

2015-12-28 Thread Jojy Varghese


> On Dec. 29, 2015, 2:46 a.m., Jojy Varghese wrote:
> > src/executor/executor.cpp, line 221
> > 
> >
> > "else" case?
> 
> Anand Mazumdar wrote:
> Nothing needs to be done for the `else` case. These env variables 
> **might** be set by the agent when framework checkpointing is enabled. For 
> more info see how the old executor library checks it:
> https://github.com/apache/mesos/blob/master/src/exec/exec.cpp#L690

hrm in the else case, what would be the value of recoveryTimeout?


- Jojy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41283/#review112114
---


On Dec. 29, 2015, 3:11 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41283/
> ---
> 
> (Updated Dec. 29, 2015, 3:11 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3550
> https://issues.apache.org/jira/browse/MESOS-3550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces the implementation for the executor library. 
> 
> This uses the new HTTP Connection interface to ensure calls are properly 
> pipelined.
> 
> `connected` -> Callback invoked when a TCP connection is established with the 
> agent.
> `disconnected` -> When the TCP connection is interrupted possibly due to an 
> agent restart.
> `received` -> When the executor receives events from the agent upon 
> subscribing.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
>   src/executor/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41283/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 41283: Introduced an Executor Library based on the new executor HTTP API.

2015-12-28 Thread Jojy Varghese

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41283/#review112125
---



src/executor/executor.cpp (line 354)


What is the purpose of mutex? It looks like we do the callbacks 
asynchronously. Which means, we have the lock only till its dispatched. We dont 
know when its executed. So wouldnt that mean two callbacks can be executed 
asyncronously at the same time?


- Jojy Varghese


On Dec. 29, 2015, 3:11 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41283/
> ---
> 
> (Updated Dec. 29, 2015, 3:11 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3550
> https://issues.apache.org/jira/browse/MESOS-3550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces the implementation for the executor library. 
> 
> This uses the new HTTP Connection interface to ensure calls are properly 
> pipelined.
> 
> `connected` -> Callback invoked when a TCP connection is established with the 
> agent.
> `disconnected` -> When the TCP connection is interrupted possibly due to an 
> agent restart.
> `received` -> When the executor receives events from the agent upon 
> subscribing.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
>   src/executor/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41283/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 41283: Introduced an Executor Library based on the new executor HTTP API.

2015-12-28 Thread Anand Mazumdar


> On Dec. 29, 2015, 2:46 a.m., Jojy Varghese wrote:
> > src/executor/executor.cpp, line 221
> > 
> >
> > "else" case?
> 
> Anand Mazumdar wrote:
> Nothing needs to be done for the `else` case. These env variables 
> **might** be set by the agent when framework checkpointing is enabled. For 
> more info see how the old executor library checks it:
> https://github.com/apache/mesos/blob/master/src/exec/exec.cpp#L690
> 
> Jojy Varghese wrote:
> hrm in the else case, what would be the value of recoveryTimeout?

The value would be the default value of `Duration` i.e. `0ns` i.e. you would 
like your executor to not retry and shut down as soon as you detect a 
disconnection with the agent (as is the case for frameworks that do not have 
checkpointing enabled). Makes sense ?


- Anand


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41283/#review112114
---


On Dec. 29, 2015, 3:11 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41283/
> ---
> 
> (Updated Dec. 29, 2015, 3:11 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3550
> https://issues.apache.org/jira/browse/MESOS-3550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces the implementation for the executor library. 
> 
> This uses the new HTTP Connection interface to ensure calls are properly 
> pipelined.
> 
> `connected` -> Callback invoked when a TCP connection is established with the 
> agent.
> `disconnected` -> When the TCP connection is interrupted possibly due to an 
> agent restart.
> `received` -> When the executor receives events from the agent upon 
> subscribing.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
>   src/executor/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41283/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Review Request 41757: Unified Container: Added passing Env in docker runtime config.

2015-12-28 Thread Gilbert Song

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41757/
---

Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


Bugs: MESOS-4226
https://issues.apache.org/jira/browse/MESOS-4226


Repository: mesos


Description
---

Unified Container: Added passing Env in docker runtime config.


Diffs
-

  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
e951c96a7f9c5ff72f6993981e2e3744e3b837f6 
  src/slave/containerizer/mesos/provisioner/store.hpp 
aec725f789f7aeb92abfcc6718c2e6e2f1f37981 

Diff: https://reviews.apache.org/r/41757/diff/


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song



Re: Review Request 41491: Unified Container: Implemented passing entrypoint in runtime config.

2015-12-28 Thread Gilbert Song

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41491/
---

(Updated Dec. 28, 2015, 3:49 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


Bugs: MESOS-4225
https://issues.apache.org/jira/browse/MESOS-4225


Repository: mesos


Description
---

Unified Container: Implemented passing entrypoint in runtime config.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
e951c96a7f9c5ff72f6993981e2e3744e3b837f6 
  src/slave/containerizer/mesos/provisioner/store.hpp 
aec725f789f7aeb92abfcc6718c2e6e2f1f37981 
  src/tests/containerizer/provisioner_docker_tests.cpp 
bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98 

Diff: https://reviews.apache.org/r/41491/diff/


Testing
---

make check


Thanks,

Gilbert Song



Review Request 41758: Unified Container: Added passing Cmd in docker runtime config.

2015-12-28 Thread Gilbert Song

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41758/
---

Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


Bugs: MESOS-4227
https://issues.apache.org/jira/browse/MESOS-4227


Repository: mesos


Description
---

Unified Container: Added passing Cmd in docker runtime config.


Diffs
-

  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
e951c96a7f9c5ff72f6993981e2e3744e3b837f6 
  src/slave/containerizer/mesos/provisioner/store.hpp 
aec725f789f7aeb92abfcc6718c2e6e2f1f37981 

Diff: https://reviews.apache.org/r/41758/diff/


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song



Re: Review Request 41753: Use Path::extension to simplify mesos code.

2015-12-28 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41753/#review112092
---


Patch looks great!

Reviews applied: [41751, 41752, 41753]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 28, 2015, 10:09 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41753/
> ---
> 
> (Updated Dec. 28, 2015, 10:09 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Artem Harutyunyan.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp ec036ecc786bce3c7e64ad64e4f4205eea4a9d84 
> 
> Diff: https://reviews.apache.org/r/41753/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>



Re: Review Request 41459: Invoked `_Deferred`'s `operator F()` explicitly.

2015-12-28 Thread Daniel Pravat

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41459/#review112086
---



3rdparty/libprocess/include/process/future.hpp (line 347)


Other similar constructs will be converted when the code using them will 
compile on Windows.


- Daniel Pravat


On Dec. 28, 2015, 3:42 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41459/
> ---
> 
> (Updated Dec. 28, 2015, 3:42 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4221
> https://issues.apache.org/jira/browse/MESOS-4221
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> VS 2015 won't support C++14 `std::function` SFINAE until Update 2, so 
> converting `_Deferred` to `std::function` must be done by explicitly calling 
> `_Deferred`'s conversion function.
> 
> Follow-up from [r40114](https://reviews.apache.org/r/40114/).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> c9146e3a3ccf09dd37c5a8ac7000fbe84f3c710c 
> 
> Diff: https://reviews.apache.org/r/41459/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OS X, compiled on Windows.
> 
> 
> Thanks,
> 
> Michael Park
> 
>



Re: Review Request 41283: Introduced an Executor Library based on the new executor HTTP API.

2015-12-28 Thread Anand Mazumdar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41283/
---

(Updated Dec. 29, 2015, 12:11 a.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Added logic for retries with linear backoff once disconnected for the agent.


Bugs: MESOS-3550
https://issues.apache.org/jira/browse/MESOS-3550


Repository: mesos


Description
---

This change introduces the implementation for the executor library. 

This uses the new HTTP Connection interface to ensure calls are properly 
pipelined.

`connected` -> Callback invoked when a TCP connection is established with the 
agent.
`disconnected` -> When the TCP connection is interrupted possibly due to an 
agent restart.
`received` -> When the executor receives events from the agent upon subscribing.


Diffs (updated)
-

  src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
  src/executor/executor.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/41283/diff/


Testing
---

make check


Thanks,

Anand Mazumdar



Re: Review Request 41713: Added HDFS URI fetcher plugin.

2015-12-28 Thread Timothy Chen

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41713/#review112105
---

Ship it!


Ship It!


src/uri/fetcher.cpp (line 57)


Missing starting single quote



src/uri/fetcher.cpp (line 63)


I think we should warn when there is already a plugin for the scheme.


- Timothy Chen


On Dec. 28, 2015, 6:07 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41713/
> ---
> 
> (Updated Dec. 28, 2015, 6:07 p.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, Ian Downes, Timothy Chen, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-3925
> https://issues.apache.org/jira/browse/MESOS-3925
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added HDFS URI fetcher plugin. This patch also adjusted the plugin 
> initialization logic so that flags can be used to initialize each plugin.
> 
> 
> Diffs
> -
> 
>   include/mesos/uri/fetcher.hpp 6cf7b37818ceff64ffe56138ebd4bc7e2abe5a7c 
>   include/mesos/uri/uri.hpp aa3ab5d24bb8c501dd19e93d7563cb0afd889f23 
>   src/Makefile.am 8af0115caa67ac8f3193d8f0d0f1a4ae739af275 
>   src/tests/uri_fetcher_tests.cpp d54a7e7b08a741b1d5d371c11747681ae4ddef98 
>   src/uri/fetcher.hpp PRE-CREATION 
>   src/uri/fetcher.cpp 8bd7cf3d638a737c40183294cb4548a4851e0886 
>   src/uri/fetchers/curl.hpp 401829d1990ec817ea069ce45f902c521cd1a937 
>   src/uri/fetchers/curl.cpp 6c607a6a800374b6e14b37bfdc6b04e7a4e87892 
>   src/uri/fetchers/hadoop.hpp PRE-CREATION 
>   src/uri/fetchers/hadoop.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41713/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-28 Thread Avinash sridharan

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41380/
---

(Updated Dec. 29, 2015, 1:10 a.m.)


Review request for mesos, Adam B and Anand Mazumdar.


Bugs: MESOS-4114
https://issues.apache.org/jira/browse/MESOS-4114


Repository: mesos


Description
---

Added repeated vip field to DiscoveryInfo and an instance_port field to Port


Diffs (updated)
-

  include/mesos/mesos.proto 2431fdd6b84625c6140a2b3913736bffada4e7f6 
  include/mesos/v1/mesos.proto 4aed0980b28dc1000aa2821f35303b736bc5bff8 

Diff: https://reviews.apache.org/r/41380/diff/


Testing
---

make check, and make


Thanks,

Avinash sridharan



Re: Review Request 41758: Unified Container: Added passing Cmd in docker runtime config.

2015-12-28 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41758/#review112106
---


Patch looks great!

Reviews applied: [41491, 41757, 41758]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 28, 2015, 11:50 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41758/
> ---
> 
> (Updated Dec. 28, 2015, 11:50 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4227
> https://issues.apache.org/jira/browse/MESOS-4227
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified Container: Added passing Cmd in docker runtime config.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> e951c96a7f9c5ff72f6993981e2e3744e3b837f6 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> aec725f789f7aeb92abfcc6718c2e6e2f1f37981 
> 
> Diff: https://reviews.apache.org/r/41758/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>



Re: Review Request 41283: Introduced an Executor Library based on the new executor HTTP API.

2015-12-28 Thread Anand Mazumdar


> On Dec. 29, 2015, 6:09 a.m., Jojy Varghese wrote:
> > src/executor/executor.cpp, line 354
> > 
> >
> > What is the purpose of mutex? It looks like we do the callbacks 
> > asynchronously. Which means, we have the lock only till its dispatched. We 
> > dont know when its executed. So wouldnt that mean two callbacks can be 
> > executed asyncronously at the same time?

Can you elaborate a bit more ? If you have a look at the implementation of 
`async` the `Future` is only fullfilled once the callback has been successfully 
invoked not **before**. Hence, the mutex is only unlocked after the callback is 
**completed**. 

https://github.com/apache/mesos/blob/master/3rdparty/libprocess/include/process/async.hpp#L93

The scheduler library also follows a similar pattern for invoking the callbacks 
: https://github.com/apache/mesos/blob/master/src/scheduler/scheduler.cpp

Hope this info helps.


- Anand


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41283/#review112125
---


On Dec. 29, 2015, 3:11 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41283/
> ---
> 
> (Updated Dec. 29, 2015, 3:11 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3550
> https://issues.apache.org/jira/browse/MESOS-3550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces the implementation for the executor library. 
> 
> This uses the new HTTP Connection interface to ensure calls are properly 
> pipelined.
> 
> `connected` -> Callback invoked when a TCP connection is established with the 
> agent.
> `disconnected` -> When the TCP connection is interrupted possibly due to an 
> agent restart.
> `received` -> When the executor receives events from the agent upon 
> subscribing.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
>   src/executor/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41283/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 41597: Extending allocator interface to support dynamic weights

2015-12-28 Thread Yongqiao Wang

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41597/
---

(Updated Dec. 29, 2015, 6:38 a.m.)


Review request for mesos, Adam B, Neil Conway, and Qian Zhang.


Changes
---

rebase.


Bugs: MESOS-3943
https://issues.apache.org/jira/browse/MESOS-3943


Repository: mesos


Description
---

Add the interface in allocator to support updating weight
at runtime, and the allocator is invoked to allocate the
resources based on the updated weights later.


Diffs (updated)
-

  include/mesos/master/allocator.hpp f7ada68d7111486d264284990996413bb3d6 
  src/master/allocator/mesos/allocator.hpp 
50ef3b20f34bc6d87cbeccabcebec9a5031a6554 
  src/master/allocator/mesos/hierarchical.hpp 
86ea5a402ed67f8f22f11d5730147cd907d66a08 
  src/master/allocator/mesos/hierarchical.cpp 
775182515dcb52bd873ecdf98c827320251a59c8 
  src/master/allocator/sorter/drf/sorter.hpp 
050896e8b12cd4097ccd137d5284d6b39b0f06ab 
  src/master/allocator/sorter/drf/sorter.cpp 
3a442f121f3a1505513877a5c78458a4b8d0a824 
  src/master/allocator/sorter/sorter.hpp 
7be6b44a762fd62c2cd7f28b4dc4865a4587ed26 
  src/tests/allocator.hpp 9bdfaecf1a148f113ad52956b50ed7cabe0902ef 

Diff: https://reviews.apache.org/r/41597/diff/


Testing
---

Make & Make check successfully!

Test case: https://reviews.apache.org/r/41672/


Thanks,

Yongqiao Wang



Re: Review Request 41681: Introduce HTTP endpoint /weights for updating weight.

2015-12-28 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41681/#review112132
---


Patch looks great!

Reviews applied: [41597, 41681]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 29, 2015, 6:40 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41681/
> ---
> 
> (Updated Dec. 29, 2015, 6:40 a.m.)
> 
> 
> Review request for mesos, Adam B, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4214
> https://issues.apache.org/jira/browse/MESOS-4214
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduce HTTP endpoint /weights for updating weight.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 2431fdd6b84625c6140a2b3913736bffada4e7f6 
>   include/mesos/v1/mesos.proto 4aed0980b28dc1000aa2821f35303b736bc5bff8 
>   src/CMakeLists.txt 8169fe4f1a0eda5c2e2b30e22a7346478420a9a0 
>   src/Makefile.am 8af0115caa67ac8f3193d8f0d0f1a4ae739af275 
>   src/master/http.cpp deb0c8f9852dc0eec1e8c0ff35c684f982e8b110 
>   src/master/master.hpp 1cc5531de70bdc0bdce9835c4930dc774406fac9 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
>   src/master/registry.proto 9958f9c2bdb785390fca2f292b65d5a9310434d5 
>   src/master/weights_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41681/diff/
> 
> 
> Testing
> ---
> 
> Make & Make check successfully!
> 
> $ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/Users/yqwyq/tmp/mesos-master  
> >> /tmp/mesos-master.log 2>&1 &)
> $ curl -d 
> weights="[{\"weight\":1.0,\"role\":\"role1\"},{\"weight\":8.0,\"role\":\"role2\"}]"
>  -X PUT http://localhost:5050/weights
> $ curl http://localhost:5050/roles
> {
> "roles": [
> {
> "frameworks": [ ], 
> "name": "*", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
> }, 
> "weight": 1
> }, 
> {
> "frameworks": [ ], 
> "name": "role1", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
> }, 
> "weight": 1
> }, 
> {
> "frameworks": [ ], 
> "name": "role2", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
> }, 
> "weight": 8
> }
> ]
> }
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>



Re: Review Request 41681: Introduce HTTP endpoint /weights for updating weight.

2015-12-28 Thread Yongqiao Wang

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41681/
---

(Updated Dec. 29, 2015, 6:40 a.m.)


Review request for mesos, Adam B, Neil Conway, and Qian Zhang.


Changes
---

Code refactory and add persistence logic.


Bugs: MESOS-4214
https://issues.apache.org/jira/browse/MESOS-4214


Repository: mesos


Description
---

Introduce HTTP endpoint /weights for updating weight.


Diffs (updated)
-

  include/mesos/mesos.proto 2431fdd6b84625c6140a2b3913736bffada4e7f6 
  include/mesos/v1/mesos.proto 4aed0980b28dc1000aa2821f35303b736bc5bff8 
  src/CMakeLists.txt 8169fe4f1a0eda5c2e2b30e22a7346478420a9a0 
  src/Makefile.am 8af0115caa67ac8f3193d8f0d0f1a4ae739af275 
  src/master/http.cpp deb0c8f9852dc0eec1e8c0ff35c684f982e8b110 
  src/master/master.hpp 1cc5531de70bdc0bdce9835c4930dc774406fac9 
  src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
  src/master/registry.proto 9958f9c2bdb785390fca2f292b65d5a9310434d5 
  src/master/weights_handler.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/41681/diff/


Testing
---

Make & Make check successfully!

$ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/Users/yqwyq/tmp/mesos-master  
>> /tmp/mesos-master.log 2>&1 &)
$ curl -d 
weights="[{\"weight\":1.0,\"role\":\"role1\"},{\"weight\":8.0,\"role\":\"role2\"}]"
 -X PUT http://localhost:5050/weights
$ curl http://localhost:5050/roles
{
"roles": [
{
"frameworks": [ ], 
"name": "*", 
"resources": {
"cpus": 0, 
"disk": 0, 
"mem": 0
}, 
"weight": 1
}, 
{
"frameworks": [ ], 
"name": "role1", 
"resources": {
"cpus": 0, 
"disk": 0, 
"mem": 0
}, 
"weight": 1
}, 
{
"frameworks": [ ], 
"name": "role2", 
"resources": {
"cpus": 0, 
"disk": 0, 
"mem": 0
}, 
"weight": 8
}
]
}


Thanks,

Yongqiao Wang



Re: Review Request 41597: Extending allocator interface to support dynamic weights

2015-12-28 Thread Yongqiao Wang

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41597/
---

(Updated Dec. 29, 2015, 7:03 a.m.)


Review request for mesos, Adam B, Neil Conway, and Qian Zhang.


Changes
---

Update the parameter to WeightInfos protobuf rather than a hashmap.


Bugs: MESOS-3943
https://issues.apache.org/jira/browse/MESOS-3943


Repository: mesos


Description
---

Add the interface in allocator to support updating weight
at runtime, and the allocator is invoked to allocate the
resources based on the updated weights later.


Diffs (updated)
-

  include/mesos/master/allocator.hpp f7ada68d7111486d264284990996413bb3d6 
  include/mesos/mesos.proto 2431fdd6b84625c6140a2b3913736bffada4e7f6 
  include/mesos/v1/mesos.proto 4aed0980b28dc1000aa2821f35303b736bc5bff8 
  src/master/allocator/mesos/allocator.hpp 
50ef3b20f34bc6d87cbeccabcebec9a5031a6554 
  src/master/allocator/mesos/hierarchical.hpp 
86ea5a402ed67f8f22f11d5730147cd907d66a08 
  src/master/allocator/mesos/hierarchical.cpp 
7f900c4e024485704d79e57ae22407557598fe6c 
  src/master/allocator/sorter/drf/sorter.hpp 
050896e8b12cd4097ccd137d5284d6b39b0f06ab 
  src/master/allocator/sorter/drf/sorter.cpp 
3a442f121f3a1505513877a5c78458a4b8d0a824 
  src/master/allocator/sorter/sorter.hpp 
7be6b44a762fd62c2cd7f28b4dc4865a4587ed26 
  src/tests/allocator.hpp 9bdfaecf1a148f113ad52956b50ed7cabe0902ef 

Diff: https://reviews.apache.org/r/41597/diff/


Testing
---

Make & Make check successfully!

Test case: https://reviews.apache.org/r/41672/


Thanks,

Yongqiao Wang



Re: Review Request 41597: Extending allocator interface to support dynamic weights

2015-12-28 Thread Yongqiao Wang

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41597/
---

(Updated Dec. 29, 2015, 7:20 a.m.)


Review request for mesos, Adam B, Neil Conway, and Qian Zhang.


Changes
---

Rebase.


Bugs: MESOS-3943
https://issues.apache.org/jira/browse/MESOS-3943


Repository: mesos


Description
---

Add the interface in allocator to support updating weight
at runtime, and the allocator is invoked to allocate the
resources based on the updated weights later.


Diffs (updated)
-

  include/mesos/master/allocator.hpp f7ada68d7111486d264284990996413bb3d6 
  include/mesos/mesos.proto 2431fdd6b84625c6140a2b3913736bffada4e7f6 
  include/mesos/v1/mesos.proto 4aed0980b28dc1000aa2821f35303b736bc5bff8 
  src/master/allocator/mesos/allocator.hpp 
50ef3b20f34bc6d87cbeccabcebec9a5031a6554 
  src/master/allocator/mesos/hierarchical.hpp 
86ea5a402ed67f8f22f11d5730147cd907d66a08 
  src/master/allocator/mesos/hierarchical.cpp 
7f900c4e024485704d79e57ae22407557598fe6c 
  src/master/allocator/sorter/drf/sorter.hpp 
050896e8b12cd4097ccd137d5284d6b39b0f06ab 
  src/master/allocator/sorter/drf/sorter.cpp 
3a442f121f3a1505513877a5c78458a4b8d0a824 
  src/master/allocator/sorter/sorter.hpp 
7be6b44a762fd62c2cd7f28b4dc4865a4587ed26 
  src/tests/allocator.hpp 9bdfaecf1a148f113ad52956b50ed7cabe0902ef 

Diff: https://reviews.apache.org/r/41597/diff/


Testing
---

Make & Make check successfully!

Test case: https://reviews.apache.org/r/41672/


Thanks,

Yongqiao Wang



Re: Review Request 41672: Test case(s) for weights + allocation behavior

2015-12-28 Thread Yongqiao Wang

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41672/
---

(Updated Dec. 29, 2015, 7:21 a.m.)


Review request for mesos, Adam B, Neil Conway, and Qian Zhang.


Changes
---

Update this test due to the allocator UpdateWeight() interface is changed.


Bugs: MESOS-4200
https://issues.apache.org/jira/browse/MESOS-4200


Repository: mesos


Description
---

Test case(s) for weights + allocation behavior


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
e044f832c2c16e53e663c6ced5452649bb0dcb59 

Diff: https://reviews.apache.org/r/41672/diff/


Testing
---

Make check done:
Yongs-MacBook-Pro:build yqwyq$ ./src/mesos-tests 
--gtest_filter=HierarchicalAllocatorTest.UpdateWeight
Source directory: /Users/yqwyq/Desktop/mesos
Build directory: /Users/yqwyq/Desktop/mesos/build
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from HierarchicalAllocatorTest
[ RUN  ] HierarchicalAllocatorTest.UpdateWeight
[   OK ] HierarchicalAllocatorTest.UpdateWeight (257 ms)
[--] 1 test from HierarchicalAllocatorTest (257 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (340 ms total)
[  PASSED  ] 1 test.


Thanks,

Yongqiao Wang