Re: Review Request 39104: Added source address to logging when we receive replicated log events.

2015-10-07 Thread Adam B

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

Ship it!


LGTM.
Since the 'Summary' and 'Description fields are used to create the git commit 
message, I'd like to see the 'Description' be more descriptive; or just say 
"See summary", which means "do not include in commit message".

- Adam B


On Oct. 7, 2015, 2:41 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39104/
> ---
> 
> (Updated Oct. 7, 2015, 2:41 p.m.)
> 
> 
> Review request for mesos, Adam B and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3417
> https://issues.apache.org/jira/browse/MESOS-3417
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3417.
> 
> 
> Diffs
> -
> 
>   src/log/replica.cpp 2bfcc3eccb0dc019dd0dfe6027a3560c6fdbcff0 
> 
> Diff: https://reviews.apache.org/r/39104/diff/
> 
> 
> Testing
> ---
> 
> "make check", ran mesos-master by hand. Example output:
> 
> I1007 21:33:05.434449 27039 replica.cpp:478] Replica received implicit 
> promise request from (4)@10.0.2.15:5050 with proposal 5
> [...]
> I1007 21:33:05.444855 27042 replica.cpp:512] Replica received write request 
> for position 17 from (5)@10.0.2.15:5050
> [...]
> I1007 21:33:05.446867 27039 replica.cpp:660] Replica received learned notice 
> for position 17 from @0.0.0.0:0
> [...]
> I1007 21:33:05.451514 27042 replica.cpp:512] Replica received write request 
> for position 18 from (6)@10.0.2.15:5050
> [...]
> I1007 21:33:05.452555 27042 replica.cpp:660] Replica received learned notice 
> for position 18 from @0.0.0.0:0
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 39104: Added source address to logging when we receive replicated log events.

2015-10-07 Thread Jie Yu

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

Ship it!


Ship It!

- Jie Yu


On Oct. 7, 2015, 9:41 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39104/
> ---
> 
> (Updated Oct. 7, 2015, 9:41 p.m.)
> 
> 
> Review request for mesos, Adam B and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3417
> https://issues.apache.org/jira/browse/MESOS-3417
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3417.
> 
> 
> Diffs
> -
> 
>   src/log/replica.cpp 2bfcc3eccb0dc019dd0dfe6027a3560c6fdbcff0 
> 
> Diff: https://reviews.apache.org/r/39104/diff/
> 
> 
> Testing
> ---
> 
> "make check", ran mesos-master by hand. Example output:
> 
> I1007 21:33:05.434449 27039 replica.cpp:478] Replica received implicit 
> promise request from (4)@10.0.2.15:5050 with proposal 5
> [...]
> I1007 21:33:05.444855 27042 replica.cpp:512] Replica received write request 
> for position 17 from (5)@10.0.2.15:5050
> [...]
> I1007 21:33:05.446867 27039 replica.cpp:660] Replica received learned notice 
> for position 17 from @0.0.0.0:0
> [...]
> I1007 21:33:05.451514 27042 replica.cpp:512] Replica received write request 
> for position 18 from (6)@10.0.2.15:5050
> [...]
> I1007 21:33:05.452555 27042 replica.cpp:660] Replica received learned notice 
> for position 18 from @0.0.0.0:0
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-10-07 Thread Ben Mahler

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



src/tests/script.cpp (line 100)


malloc is not async-signal-safe, and may be used by the string 
implementation, and so we must not use os::strerror (assuming we rename it) 
after a fork! Please do an audit of this diff.


- Ben Mahler


On Oct. 6, 2015, 3:08 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39008/
> ---
> 
> (Updated Oct. 6, 2015, 3:08 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Switch call sites to using safe strerror_r wrapper.
> 
> 
> Diffs
> -
> 
>   src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
>   src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/linux/routing/link/internal.hpp 
> 015c0ef5be516d7786c96a96437cced1ae8487fa 
>   src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
>   src/slave/containerizer/docker.cpp 6c975f904178e01797b67628a2d471ec7b3b1fbf 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 8823b7850a1ac17fc4f4868aadf1b04428d2381b 
>   src/slave/containerizer/isolators/filesystem/posix.cpp 
> eec510c4f7655d67b33ad90210eeb57fcc910684 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
>   src/slave/containerizer/mesos/launch.cpp 
> 09d4d8f4d6837e93a82deef76ca07e2167d6a405 
>   src/slave/containerizer/provisioner/backends/bind.cpp 
> 1fe1746c0bc1c9c12e1378e6438122a91b58316b 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
>   src/tests/containerizer/memory_test_helper.cpp 
> 8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
>   src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 
> 
> Diff: https://reviews.apache.org/r/39008/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 39005: stout: Added thread-safe replacement for strerror.

2015-10-07 Thread Benjamin Mahler
Seems like os::strerror() would be more consistent with our other posix api
wrappers.

On Wed, Oct 7, 2015 at 9:27 AM, Bernd Mathiske  wrote:

> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39005/
> 3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp
>  
> (Diff
> revision 1)
>
> 27
>
>  * @note This function is thread-safe.
>
> Suggestion: "In contast to strerror(), this function is thread-safe."
>
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp
>  
> (Diff
> revision 1)
>
> 37
>
> buffer.resize(buffer.size() * 2, '\0');
>
> If you wanted to be really devensive, not trusting strerror_r() on all 
> unforeseeable platforms for all eternity, we could cut the loop at a max size.
>
> But then maybe we could just use a larger buffer to begin with and be done?
>
> Or, try 34 first and then something large like 1024, and that's it.
>
>
> - Bernd Mathiske
>
> On October 6th, 2015, 8:07 a.m. PDT, Benjamin Bannier wrote:
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> By Benjamin Bannier.
>
> *Updated Oct. 6, 2015, 8:07 a.m.*
> *Bugs: * MESOS-3551 
> *Repository: * mesos
> Description
>
> This adds a thread-safe wrapper around strerror_r which has semantics similar 
> to strerror. We plan to use this at call sites currently relying on strerror.
>
> Testing
>
> make check
>
> Diffs
>
>- 3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp
>(12ba1ca861114e60f8276c0ee91c543abcfc2519)
>- 3rdparty/libprocess/3rdparty/stout/tests/error_tests.cpp
>(9e7605c53e6636e7fea32e4f69fbaff9100a979f)
>
> View Diff 
>


Re: Review Request 38910: Added `-v` flag to `docker rm`.

2015-10-07 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [38910]

All tests passed.

- Mesos ReviewBot


On Oct. 7, 2015, 5:34 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38910/
> ---
> 
> (Updated Oct. 7, 2015, 5:34 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-2613
> https://issues.apache.org/jira/browse/MESOS-2613
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `-v` flag to `docker rm`.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 
> 
> Diff: https://reviews.apache.org/r/38910/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> DockerContainerizerTest.ROOT_DOCKER_Launch_Executor fails on my Ubuntu 14.04 
> VM, but this is a known issue: MESOS-3123
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Review Request 39104: Added source address to logging when we receive replicated log events.

2015-10-07 Thread Neil Conway

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

Review request for mesos, Adam B and Joris Van Remoortere.


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


Repository: mesos


Description
---

MESOS-3417.


Diffs
-

  src/log/replica.cpp 2bfcc3eccb0dc019dd0dfe6027a3560c6fdbcff0 

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


Testing
---

"make check", ran mesos-master by hand. Example output:

I1007 21:33:05.434449 27039 replica.cpp:478] Replica received implicit promise 
request from (4)@10.0.2.15:5050 with proposal 5
[...]
I1007 21:33:05.444855 27042 replica.cpp:512] Replica received write request for 
position 17 from (5)@10.0.2.15:5050
[...]
I1007 21:33:05.446867 27039 replica.cpp:660] Replica received learned notice 
for position 17 from @0.0.0.0:0
[...]
I1007 21:33:05.451514 27042 replica.cpp:512] Replica received write request for 
position 18 from (6)@10.0.2.15:5050
[...]
I1007 21:33:05.452555 27042 replica.cpp:660] Replica received learned notice 
for position 18 from @0.0.0.0:0


Thanks,

Neil Conway



Re: Review Request 39060: Create master detector per url & not per framework

2015-10-07 Thread Mandeep Chadha

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

(Updated Oct. 7, 2015, 9:42 p.m.)


Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description
---

If the number of framework created exceeds the lib process
threads then during master failover the zookeeper updates can
cause deadlock.


Diffs (updated)
-

  src/sched/sched.cpp 571e00d303009a940f17c8ed4582749a718e846d 
  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 

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


Testing
---

make check successful 
Created 100 framework instances on a 24 CPU machine. Master failover detected 
by the framework process and continue to work as expected.


Thanks,

Mandeep Chadha



Re: Review Request 39037: Allow description empty in help information.

2015-10-07 Thread Ben Mahler

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



3rdparty/libprocess/src/help.cpp (lines 51 - 54)


In your last diff you didn't have this change and the one below to 
conditionally add newlines, why did you introduce it when changing it to Option?

If we want to change this behavior, let's do it in another patch since it 
seems independent from making 'description' optional. Keep independent changes 
in separate patches :)


- Ben Mahler


On Oct. 7, 2015, 3:04 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39037/
> ---
> 
> (Updated Oct. 7, 2015, 3:04 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3104
> https://issues.apache.org/jira/browse/MESOS-3104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow description empty in help information.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> e7dc670648e8abd2fef7be79835f0b71e3e91258 
>   3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7 
> 
> Diff: https://reviews.apache.org/r/39037/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 39102: Added documentation for JSON resources.

2015-10-07 Thread Neil Conway

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



docs/attributes-resources.md (line 39)


What's a "key string"?



docs/attributes-resources.md (line 164)


Wouldn't "--resources=`cat resources.txt`" be a bit more direct?



docs/configuration.md (line 1431)


Add link.


- Neil Conway


On Oct. 7, 2015, 7:19 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39102/
> ---
> 
> (Updated Oct. 7, 2015, 7:19 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for JSON resources.
> 
> 
> Diffs
> -
> 
>   docs/attributes-resources.md f712d094f14426515dabde45f98d6c1ae36c3447 
>   docs/configuration.md 2ab476a2d2c43e309b570d73ecac80e27b296e7e 
> 
> Diff: https://reviews.apache.org/r/39102/diff/
> 
> 
> Testing
> ---
> 
> Viewed the relevant documentation sections ('Attributes and Resources' & 
> 'Configuration') using the mesos-website-container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 39104: Added source address to logging when we receive replicated log events.

2015-10-07 Thread Adam B


> On Oct. 7, 2015, 2:52 p.m., Adam B wrote:
> > LGTM.
> > Since the 'Summary' and 'Description fields are used to create the git 
> > commit message, I'd like to see the 'Description' be more descriptive; or 
> > just say "See summary", which means "do not include in commit message".
> 
> Neil Conway wrote:
> Okay. Just to make sure I understand, I was proposing a Git commit 
> message with two lines:
> 
> ```
> Added source address to logging when we receive replicated log events.
> 
> MESOS-3417
> ```
> 
> i.e., I think it is important to link to the Jira for context, but 
> otherwise that seemed sufficient as a Git message. lmk if I should do that 
> differently...

Yeah, that's ok, but I don't like such short second lines. Better would be:
`MESOS-3417: Added source address to logging when we receive replicated log 
events.`
or
`Added source address to logging when we receive replicated log events.`
`Fixes MESOS-3417: Log source address replicated log recieved broadcasts`
But that's just a personal nit. I'll commit it either way.


- Adam


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


On Oct. 7, 2015, 2:41 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39104/
> ---
> 
> (Updated Oct. 7, 2015, 2:41 p.m.)
> 
> 
> Review request for mesos, Adam B and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3417
> https://issues.apache.org/jira/browse/MESOS-3417
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3417.
> 
> 
> Diffs
> -
> 
>   src/log/replica.cpp 2bfcc3eccb0dc019dd0dfe6027a3560c6fdbcff0 
> 
> Diff: https://reviews.apache.org/r/39104/diff/
> 
> 
> Testing
> ---
> 
> "make check", ran mesos-master by hand. Example output:
> 
> I1007 21:33:05.434449 27039 replica.cpp:478] Replica received implicit 
> promise request from (4)@10.0.2.15:5050 with proposal 5
> [...]
> I1007 21:33:05.444855 27042 replica.cpp:512] Replica received write request 
> for position 17 from (5)@10.0.2.15:5050
> [...]
> I1007 21:33:05.446867 27039 replica.cpp:660] Replica received learned notice 
> for position 17 from @0.0.0.0:0
> [...]
> I1007 21:33:05.451514 27042 replica.cpp:512] Replica received write request 
> for position 18 from (6)@10.0.2.15:5050
> [...]
> I1007 21:33:05.452555 27042 replica.cpp:660] Replica received learned notice 
> for position 18 from @0.0.0.0:0
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 39060: Create master detector per url & not per framework

2015-10-07 Thread Mandeep Chadha

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

(Updated Oct. 7, 2015, 9:49 p.m.)


Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description
---

If the number of framework created exceeds the lib process
threads then during master failover the zookeeper updates can
cause deadlock.


Diffs (updated)
-

  src/sched/sched.cpp 571e00d303009a940f17c8ed4582749a718e846d 

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


Testing
---

make check successful 
Created 100 framework instances on a 24 CPU machine. Master failover detected 
by the framework process and continue to work as expected.


Thanks,

Mandeep Chadha



Re: Review Request 39104: Added source address to logging when we receive replicated log events.

2015-10-07 Thread Neil Conway


> On Oct. 7, 2015, 9:52 p.m., Adam B wrote:
> > LGTM.
> > Since the 'Summary' and 'Description fields are used to create the git 
> > commit message, I'd like to see the 'Description' be more descriptive; or 
> > just say "See summary", which means "do not include in commit message".

Okay. Just to make sure I understand, I was proposing a Git commit message with 
two lines:

```
Added source address to logging when we receive replicated log events.

MESOS-3417
```

i.e., I think it is important to link to the Jira for context, but otherwise 
that seemed sufficient as a Git message. lmk if I should do that differently...


- Neil


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


On Oct. 7, 2015, 9:41 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39104/
> ---
> 
> (Updated Oct. 7, 2015, 9:41 p.m.)
> 
> 
> Review request for mesos, Adam B and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3417
> https://issues.apache.org/jira/browse/MESOS-3417
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3417.
> 
> 
> Diffs
> -
> 
>   src/log/replica.cpp 2bfcc3eccb0dc019dd0dfe6027a3560c6fdbcff0 
> 
> Diff: https://reviews.apache.org/r/39104/diff/
> 
> 
> Testing
> ---
> 
> "make check", ran mesos-master by hand. Example output:
> 
> I1007 21:33:05.434449 27039 replica.cpp:478] Replica received implicit 
> promise request from (4)@10.0.2.15:5050 with proposal 5
> [...]
> I1007 21:33:05.444855 27042 replica.cpp:512] Replica received write request 
> for position 17 from (5)@10.0.2.15:5050
> [...]
> I1007 21:33:05.446867 27039 replica.cpp:660] Replica received learned notice 
> for position 17 from @0.0.0.0:0
> [...]
> I1007 21:33:05.451514 27042 replica.cpp:512] Replica received write request 
> for position 18 from (6)@10.0.2.15:5050
> [...]
> I1007 21:33:05.452555 27042 replica.cpp:660] Replica received learned notice 
> for position 18 from @0.0.0.0:0
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Review Request 39076: CMake: Added ability of Windows builds to include protobuf headers.

2015-10-07 Thread Alex Clemmer

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

Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

CMake: Added ability of Windows builds to include protobuf headers.


Diffs
-

  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
ee1c74d31e28136bf289f4100d79a8ce568cd3af 

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


Testing
---


Thanks,

Alex Clemmer



Re: Review Request 39098: Changed secret field in Credential from 'bytes' to 'string'

2015-10-07 Thread Isabel Jimenez

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

(Updated Oct. 8, 2015, 1 a.m.)


Review request for mesos and Michael Park.


Changes
---

Added java framework example changes


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


Repository: mesos


Description
---

When decoding the JSON credential file into the Credential protobuf object, the 
'secret' which is in 'bytes' is mapped into base64 string automatically by 
protobuf from JSON. This creates an unintended behavior, forcing users to 
encode in base64 their secret when wanting to pass a JSON file to the 
--credentials flag.


Diffs (updated)
-

  include/mesos/mesos.proto 4a16be1 
  src/examples/java/TestExceptionFramework.java 78720b0 
  src/examples/java/TestFramework.java aad94c0 

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


Testing
---

make check


Thanks,

Isabel Jimenez



Re: Review Request 38580: Added docker registry RemotePuller

2015-10-07 Thread Jojy Varghese

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

(Updated Oct. 8, 2015, 2:37 a.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

rebased.


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
4a0b7d11f013941084571f2d89d835a4668a3d8b 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp 
cbb67686d45513f0395a0cf1bc5c43cb4935adae 
  src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
  src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 39037: Allow description empty in help information.

2015-10-07 Thread Guangya Liu

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



3rdparty/libprocess/src/help.cpp (line 52)


I think that the tldr logic does not needs to be changed, we can keep it in 
line 47


- Guangya Liu


On 十月 7, 2015, 3:04 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39037/
> ---
> 
> (Updated 十月 7, 2015, 3:04 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3104
> https://issues.apache.org/jira/browse/MESOS-3104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow description empty in help information.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> e7dc670648e8abd2fef7be79835f0b71e3e91258 
>   3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7 
> 
> Diff: https://reviews.apache.org/r/39037/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 39037: Allow description empty in help information.

2015-10-07 Thread Guangya Liu


> On 十月 7, 2015, 10:01 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/help.cpp, lines 60-63
> > 
> >
> > In your last diff you didn't have this change and the one below to 
> > conditionally add newlines, why did you introduce it when changing it to 
> > Option?
> > 
> > If we want to change this behavior, let's do it in another patch since 
> > it seems independent from making 'description' optional. Keep independent 
> > changes in separate patches :)

bmahler, I think that the changes are needed to be in this patch since adding 
"\n" is the old behavior. What haosdent is doing now is updating the logic to 
align with his new change of making description as optional.


- Guangya


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


On 十月 7, 2015, 3:04 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39037/
> ---
> 
> (Updated 十月 7, 2015, 3:04 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3104
> https://issues.apache.org/jira/browse/MESOS-3104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow description empty in help information.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> e7dc670648e8abd2fef7be79835f0b71e3e91258 
>   3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7 
> 
> Diff: https://reviews.apache.org/r/39037/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 39037: Allow description empty in help information.

2015-10-07 Thread Guangya Liu


- Guangya


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


On 十月 7, 2015, 3:04 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39037/
> ---
> 
> (Updated 十月 7, 2015, 3:04 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3104
> https://issues.apache.org/jira/browse/MESOS-3104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow description empty in help information.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> e7dc670648e8abd2fef7be79835f0b71e3e91258 
>   3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7 
> 
> Diff: https://reviews.apache.org/r/39037/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 39060: Create master detector per url & not per framework

2015-10-07 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [39060]

All tests passed.

- Mesos ReviewBot


On Oct. 7, 2015, 9:49 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39060/
> ---
> 
> (Updated Oct. 7, 2015, 9:49 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3595
> https://issues.apache.org/jira/browse/MESOS-3595
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the number of framework created exceeds the lib process
> threads then during master failover the zookeeper updates can
> cause deadlock.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 571e00d303009a940f17c8ed4582749a718e846d 
> 
> Diff: https://reviews.apache.org/r/39060/diff/
> 
> 
> Testing
> ---
> 
> make check successful 
> Created 100 framework instances on a 24 CPU machine. Master failover detected 
> by the framework process and continue to work as expected.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>



Review Request 39112: RegistryClient refactor: fixed variable names

2015-10-07 Thread Jojy Varghese

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

RegistryClient refactor: fixed variable names. This patch will serve as 
catch-all for any variable name related changes in the refactor.


Diffs
-

  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 38747: Adding digest utilities

2015-10-07 Thread Jojy Varghese

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

(Updated Oct. 8, 2015, 2:37 a.m.)


Review request for mesos, Ben Mahler, Gilbert Song, and Timothy Chen.


Changes
---

replaced raw pointers with smart ptrs.


Repository: mesos


Description
---

Added SHA256, SHA512 implementation.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
a14b5b8fe22d3e75bef3c716ae7865ddaf132927 
  3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 39104: Added source address to logging when we receive replicated log events.

2015-10-07 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [39104]

All tests passed.

- Mesos ReviewBot


On Oct. 7, 2015, 9:41 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39104/
> ---
> 
> (Updated Oct. 7, 2015, 9:41 p.m.)
> 
> 
> Review request for mesos, Adam B and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3417
> https://issues.apache.org/jira/browse/MESOS-3417
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3417.
> 
> 
> Diffs
> -
> 
>   src/log/replica.cpp 2bfcc3eccb0dc019dd0dfe6027a3560c6fdbcff0 
> 
> Diff: https://reviews.apache.org/r/39104/diff/
> 
> 
> Testing
> ---
> 
> "make check", ran mesos-master by hand. Example output:
> 
> I1007 21:33:05.434449 27039 replica.cpp:478] Replica received implicit 
> promise request from (4)@10.0.2.15:5050 with proposal 5
> [...]
> I1007 21:33:05.444855 27042 replica.cpp:512] Replica received write request 
> for position 17 from (5)@10.0.2.15:5050
> [...]
> I1007 21:33:05.446867 27039 replica.cpp:660] Replica received learned notice 
> for position 17 from @0.0.0.0:0
> [...]
> I1007 21:33:05.451514 27042 replica.cpp:512] Replica received write request 
> for position 18 from (6)@10.0.2.15:5050
> [...]
> I1007 21:33:05.452555 27042 replica.cpp:660] Replica received learned notice 
> for position 18 from @0.0.0.0:0
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 39102: Added documentation for JSON resources.

2015-10-07 Thread Mesos ReviewBot

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


Bad review!

Reviews applied: []

Error:
 No reviewers specified. Please find a reviewer by asking on JIRA or the 
mailing list.

- Mesos ReviewBot


On Oct. 7, 2015, 11:59 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39102/
> ---
> 
> (Updated Oct. 7, 2015, 11:59 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for JSON resources.
> 
> 
> Diffs
> -
> 
>   docs/attributes-resources.md f712d094f14426515dabde45f98d6c1ae36c3447 
>   docs/configuration.md 2ab476a2d2c43e309b570d73ecac80e27b296e7e 
> 
> Diff: https://reviews.apache.org/r/39102/diff/
> 
> 
> Testing
> ---
> 
> Viewed the relevant documentation sections ('Attributes and Resources' & 
> 'Configuration') using the mesos-website-container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 39102: Added documentation for JSON resources.

2015-10-07 Thread Greg Mann

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

(Updated Oct. 8, 2015, 4:42 a.m.)


Review request for mesos, Adam B and Neil Conway.


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


Repository: mesos


Description
---

Added documentation for JSON resources.


Diffs
-

  docs/attributes-resources.md f712d094f14426515dabde45f98d6c1ae36c3447 
  docs/configuration.md 2ab476a2d2c43e309b570d73ecac80e27b296e7e 

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


Testing
---

Viewed the relevant documentation sections ('Attributes and Resources' & 
'Configuration') using the mesos-website-container: 
https://github.com/mesosphere/mesos-website-container


Thanks,

Greg Mann



Re: Review Request 39018: Added JSON parsing for Resources.

2015-10-07 Thread Guangya Liu

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

Ship it!


Ship It!

- Guangya Liu


On 十月 7, 2015, 3:24 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated 十月 7, 2015, 3:24 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Review Request 39102: Added documentation for JSON resources.

2015-10-07 Thread Greg Mann

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

Review request for mesos.


Repository: mesos


Description
---

Added documentation for JSON resources.


Diffs
-

  docs/attributes-resources.md f712d094f14426515dabde45f98d6c1ae36c3447 
  docs/configuration.md 2ab476a2d2c43e309b570d73ecac80e27b296e7e 

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


Testing
---

Viewed the relevant documentation sections ('Attributes and Resources' & 
'Configuration') using the mesos-website-container: 
https://github.com/mesosphere/mesos-website-container


Thanks,

Greg Mann



Re: Review Request 38899: Added handler for Executor->Framework message for the Executor HTTP API

2015-10-07 Thread Isabel Jimenez


> On Oct. 7, 2015, 6:40 p.m., Isabel Jimenez wrote:
> > Ship It!

LGTM, just might need to get rebase on changes for switch syntax in other 
patches.


- Isabel


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


On Oct. 7, 2015, 6:39 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38899/
> ---
> 
> (Updated Oct. 7, 2015, 6:39 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-2295
> https://issues.apache.org/jira/browse/MESOS-2295
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Trivial change to call the `executorMessage(...)` function that already 
> existed in `src/slave/slave.cpp` to handle ExecutorToFrameworkMessage.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
> 
> Diff: https://reviews.apache.org/r/38899/diff/
> 
> 
> Testing
> ---
> 
> make check. Would add tests later after the MESOS-3480 review chain gets 
> committed.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 37996: Added InheritanceTree a tree based container where children nodes inherit the values associated with their parent.

2015-10-07 Thread Alexander Rojas

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

(Updated Oct. 7, 2015, 11:54 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Update summary.


Summary (updated)
-

Added InheritanceTree a tree based container where children nodes inherit the 
values associated with their parent.


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


Repository: mesos


Description
---

Introduces the `InheritanceTree` class which allows to create a tree where 
nodes can be tag with some property. This property is then inherited by 
children nodes.

Two behaviors are implemented, overriding, i.e. Adding a property to the child 
node of another node with a property already will result in the ancestor 
property being lost. The second behavior, accumulating, takes a function and 
accumulates
properties of all ancestors.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/Makefile.am 
76e1674e08bbe65a4fdf86731823a61f231d6d12 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
94292f8a46ec31bbaf6e52f48109322bbe123f70 
  3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp 
PRE-CREATION 

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


Testing
---

make check


Thanks,

Alexander Rojas



Re: Review Request 38910: Added `-v` flag to `docker rm`.

2015-10-07 Thread Guangya Liu


> On 十月 1, 2015, 4:31 a.m., Jojy Varghese wrote:
> > src/docker/docker.cpp, line 681
> > 
> >
> > wondering this behavior should be defaulted or not. We might be 
> > overloading stop with more than what it should be doing isnt it? Do we 
> > always want to force remove the volumes when docker stops?
> 
> Greg Mann wrote:
> My & Tim's original thought was that because persistent volumes aren't 
> explicitly implemented for the Docker containerizer currently, we could 
> safely make `-v` the default behavior. However, after considering a bit more, 
> I'm thinking that existing user workflows might establish a Docker volume on 
> an agent with the intention of returning to it later with a subsequent task, 
> in which case defaulting to `-v` would break this workflow, so making this an 
> optional argument may be a good idea. I'll have to think about what is the 
> best way to pass this option to the agent, any thoughts on that matter would 
> be appreciated!
> 
> Greg Mann wrote:
> After further review, I do think that we should make the default behavior 
> include the `-v` flag. By default, Docker will create directories in 
> `/var/lib/docker/aufs/`, `/var/lib/docker/vfs`, etc. depending on the 
> filesystem used. These directories contain information related to the Docker 
> image itself, and they will not be deleted if the `-v` flag isn't used. Thus, 
> if an agent runs long enough and runs enough new Docker images, it can fill 
> up the disk with these default volumes.
> 
> While it's possible that a user might hack together a "persistent volume" 
> using Docker volumes as they're currently implemented, we should discourage 
> this in favor of utilizing Mesos-monitored persistent volumes. Always using 
> `docker rm -v` allows us to properly clean the agent, leaving it in a good 
> state after the containerized task has completed.
> 
> It's also worth noting that `docker rm -v` will NOT remove volumes where 
> a host path has been explicitly mounted into the container (i.e. using 
> `docker run -v /host/path:/container/path mycontainer`). So, in fact, some 
> volumes will be unaffected by this change. The primary purpose of this patch 
> is to clean the agent by removing the default volumes created in 
> `/var/lib/docker/`.
> 
> Guangya Liu wrote:
> Greg, so if I was using -v /host/path:/container/path when create a 
> container, I will always need to remove the files explictly on the docker 
> server when the container stopped?
> 
> Greg Mann wrote:
> Guangya, yes you have to remove these volumes manually. Even if the 
> directory /host/path does not exist prior to running the container, Docker 
> does not remove volumes mounted in that way, and I don't believe Mesos 
> removes such volumes either. This is perhaps another feature that we should 
> add into the Docker containerizer: when attaching a volume of the form `-v 
> /host/path:/container/path`, Mesos could check to see if that directory 
> exists already; if it doesn't, then it could be removed along with the 
> container after the task is complete.

Thanks Greg, +1 on using -v as default.


- Guangya


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


On 九月 30, 2015, 11:51 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38910/
> ---
> 
> (Updated 九月 30, 2015, 11:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-2613
> https://issues.apache.org/jira/browse/MESOS-2613
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `-v` flag to `docker rm`.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 
> 
> Diff: https://reviews.apache.org/r/38910/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> DockerContainerizerTest.ROOT_DOCKER_Launch_Executor fails on my Ubuntu 14.04 
> VM, but this is a known issue: MESOS-3123
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 39076: CMake: Added ability of Windows builds to include protobuf headers.

2015-10-07 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [39019, 39076]

All tests passed.

- Mesos ReviewBot


On Oct. 7, 2015, 8:31 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39076/
> ---
> 
> (Updated Oct. 7, 2015, 8:31 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Added ability of Windows builds to include protobuf headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> ee1c74d31e28136bf289f4100d79a8ce568cd3af 
> 
> Diff: https://reviews.apache.org/r/39076/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Review Request 39086: Fixed typos in comments and docs.

2015-10-07 Thread Gastón Kleiman

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

Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

Fixed typos in comments and docs.


Diffs
-

  docs/fetcher.md 36351a3d0377d1b4523110c299aa7c5f9549506e 
  include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999 
  include/mesos/v1/mesos.proto eadbc9d6b097f0652dfc33af28a67eed6631a799 
  src/slave/containerizer/docker.cpp 6c975f904178e01797b67628a2d471ec7b3b1fbf 
  src/zookeeper/authentication.hpp 5919ec021465cf4f27a51041ff635730d0043eb9 

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


Testing
---

I only changed comments and docs, but I made sure that "make check" passes on 
OS X 10.10.5.


Thanks,

Gastón Kleiman



Re: Review Request 39086: Fixed typos in comments and docs.

2015-10-07 Thread Mesos ReviewBot

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


Bad patch!

Reviews applied: [39086]

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

Error:
 2015-10-07 14:12:55 URL:https://reviews.apache.org/r/39086/diff/raw/ 
[5430/5430] -> "39086.patch" [1]
Traceback (most recent call last):
  File "./support/jsonurl.py", line 25, in 
print data
UnicodeEncodeError: 'ascii' codec can't encode character u'\xf3' in position 4: 
ordinal not in range(128)
Successfully applied: Fixed typos in comments and docs.

Fixed typos in comments and docs.


Review: https://reviews.apache.org/r/39086
fatal: empty ident name (for ) not allowed
Failed to commit patch

- Mesos ReviewBot


On Oct. 7, 2015, 1:46 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39086/
> ---
> 
> (Updated Oct. 7, 2015, 1:46 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typos in comments and docs.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md 36351a3d0377d1b4523110c299aa7c5f9549506e 
>   include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999 
>   include/mesos/v1/mesos.proto eadbc9d6b097f0652dfc33af28a67eed6631a799 
>   src/slave/containerizer/docker.cpp 6c975f904178e01797b67628a2d471ec7b3b1fbf 
>   src/zookeeper/authentication.hpp 5919ec021465cf4f27a51041ff635730d0043eb9 
> 
> Diff: https://reviews.apache.org/r/39086/diff/
> 
> 
> Testing
> ---
> 
> I only changed comments and docs, but I made sure that "make check" passes on 
> OS X 10.10.5.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>



Review Request 39088: Made shell test locale-independent.

2015-10-07 Thread Benjamin Bannier

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

Review request for mesos, Alexander Rojas, Jan Schlicht, and Till Toenshoff.


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


Repository: mesos


Description
---

The test environment does not enforce that we run with a defined locale,
so instead enforce it for our particular use case (here: potentially
localized error messages from shell).


Diffs
-

  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
e6d36ec1bf414b52d0899f0edf83e0ad8910dd0e 

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


Testing
---

With German locale installed:

% LANG=de_DE.UTF-8 make check


Thanks,

Benjamin Bannier



Re: Review Request 39086: Fixed typos in comments and docs.

2015-10-07 Thread Gastón Kleiman


> On Oct. 7, 2015, 2:12 p.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [39086]
> > 
> > Failed command: ./support/apply-review.sh -n -r 39086
> > 
> > Error:
> >  2015-10-07 14:12:55 URL:https://reviews.apache.org/r/39086/diff/raw/ 
> > [5430/5430] -> "39086.patch" [1]
> > Traceback (most recent call last):
> >   File "./support/jsonurl.py", line 25, in 
> > print data
> > UnicodeEncodeError: 'ascii' codec can't encode character u'\xf3' in 
> > position 4: ordinal not in range(128)
> > Successfully applied: Fixed typos in comments and docs.
> > 
> > Fixed typos in comments and docs.
> > 
> > 
> > Review: https://reviews.apache.org/r/39086
> > fatal: empty ident name (for ) not allowed
> > Failed to commit patch

https://reviews.apache.org/r/39087/ should make this patch apply.


- Gastón


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


On Oct. 7, 2015, 1:46 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39086/
> ---
> 
> (Updated Oct. 7, 2015, 1:46 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typos in comments and docs.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md 36351a3d0377d1b4523110c299aa7c5f9549506e 
>   include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999 
>   include/mesos/v1/mesos.proto eadbc9d6b097f0652dfc33af28a67eed6631a799 
>   src/slave/containerizer/docker.cpp 6c975f904178e01797b67628a2d471ec7b3b1fbf 
>   src/zookeeper/authentication.hpp 5919ec021465cf4f27a51041ff635730d0043eb9 
> 
> Diff: https://reviews.apache.org/r/39086/diff/
> 
> 
> Testing
> ---
> 
> I only changed comments and docs, but I made sure that "make check" passes on 
> OS X 10.10.5.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>



Re: Review Request 39018: Added JSON parsing for Resources.

2015-10-07 Thread Greg Mann

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

(Updated Oct. 7, 2015, 3:24 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael Park.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

This includes code changes necessary for JSON parsing of Resources. 
Documentation changes will be posted soon in another review.


Diffs (updated)
-

  include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
  include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
  src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
  src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
  src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 

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


Testing
---

`make check`


Thanks,

Greg Mann



Re: Review Request 39018: Added JSON parsing for Resources.

2015-10-07 Thread Greg Mann


> On Oct. 7, 2015, 3:44 a.m., Guangya Liu wrote:
> > src/common/resources.cpp, line 354
> > 
> >
> > What does this mean? In my understanding, the resource falg should 
> > support both string and json format, why remove old resource flag format?

My original thought was that we would eventually deprecate the old resources 
flag format. However, having spent some time testing the JSON flags at the 
command line, the JSON format is a bit verbose for that purpose :-) I agree 
that it would be nice to keep compatibility with both formats, as the 
colon-delimited format is nice for quickly running the agent at the command 
line.


- Greg


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


On Oct. 7, 2015, 3:24 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 7, 2015, 3:24 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 38910: Added `-v` flag to `docker rm`.

2015-10-07 Thread Greg Mann

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

(Updated Oct. 7, 2015, 5:34 p.m.)


Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added `-v` flag to `docker rm`.


Diffs (updated)
-

  src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 

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


Testing
---

`make check`

DockerContainerizerTest.ROOT_DOCKER_Launch_Executor fails on my Ubuntu 14.04 
VM, but this is a known issue: MESOS-3123


Thanks,

Greg Mann



Review Request 39091: CMake:[1/3] Prepared stout tests to run with `make check`.

2015-10-07 Thread Alex Clemmer

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

Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


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


Repository: mesos


Description
---

Right now running the Mesos tests from the default tests target will
(1) aggregate all our tests into a semi-useless report that hides all
the errors, and (2) strip all our colors when you do pass the verbosity
flag to get rid of the "report" structure.

This commit will prepare us to run the tests with with color, without
this default reporting structure. We do this by moving the tests target
to a `CACHE STRING` which can be referenced by the tests.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
c4d1b7b3f896f8037d922fafcdbdd0e31a70e140 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
94292f8a46ec31bbaf6e52f48109322bbe123f70 

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


Testing
---


Thanks,

Alex Clemmer



Review Request 39092: CMake:[2/3] prepared process library tests to run with `make check`.

2015-10-07 Thread Alex Clemmer

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

Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


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


Repository: mesos


Description
---

Right now running the Mesos tests from the default tests target will (1)
aggregate all our tests into a semi-useless report that hides all the
errors, and (2) strip all our colors when you do pass the verbosity flag
to get rid of the "report" structure.

This commit will prepare us to run the tests with with color, without
this default reporting structure. We do this by moving the tests target
to a `CACHE STRING` which can be referenced by the tests.


Diffs
-

  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
ea6db09e1a1aa01450aee93814e07f09feae7ac9 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
a14b5b8fe22d3e75bef3c716ae7865ddaf132927 

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


Testing
---


Thanks,

Alex Clemmer



Review Request 39093: CMake:[3/3] Add `make check` target.

2015-10-07 Thread Alex Clemmer

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

Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


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


Repository: mesos


Description
---

CMake:[3/3] Add `make check` target.


Diffs
-

  CMakeLists.txt a5a66c13496a95bb7095504a4645909d887fb49f 

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


Testing
---


Thanks,

Alex Clemmer



Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-10-07 Thread Bernd Mathiske

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

Ship it!


Once it compiles (when the preceding patch is fixed), LGTM.

- Bernd Mathiske


On Oct. 6, 2015, 8:08 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39008/
> ---
> 
> (Updated Oct. 6, 2015, 8:08 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Switch call sites to using safe strerror_r wrapper.
> 
> 
> Diffs
> -
> 
>   src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
>   src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/linux/routing/link/internal.hpp 
> 015c0ef5be516d7786c96a96437cced1ae8487fa 
>   src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
>   src/slave/containerizer/docker.cpp 6c975f904178e01797b67628a2d471ec7b3b1fbf 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 8823b7850a1ac17fc4f4868aadf1b04428d2381b 
>   src/slave/containerizer/isolators/filesystem/posix.cpp 
> eec510c4f7655d67b33ad90210eeb57fcc910684 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
>   src/slave/containerizer/mesos/launch.cpp 
> 09d4d8f4d6837e93a82deef76ca07e2167d6a405 
>   src/slave/containerizer/provisioner/backends/bind.cpp 
> 1fe1746c0bc1c9c12e1378e6438122a91b58316b 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
>   src/tests/containerizer/memory_test_helper.cpp 
> 8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
>   src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 
> 
> Diff: https://reviews.apache.org/r/39008/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 39087: Added support for non-ascii chars to apply-review.sh.

2015-10-07 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [39087]

All tests passed.

- Mesos ReviewBot


On Oct. 7, 2015, 2:54 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39087/
> ---
> 
> (Updated Oct. 7, 2015, 2:54 p.m.)
> 
> 
> Review request for mesos and Jan Schlicht.
> 
> 
> Bugs: MESOS-3268
> https://issues.apache.org/jira/browse/MESOS-3268
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for non-ascii chars to apply-review.sh.
> 
> 
> Diffs
> -
> 
>   support/jsonurl.py f6b54d8f8b0402f703d5758704a3f783dd9b3ef9 
> 
> Diff: https://reviews.apache.org/r/39087/diff/
> 
> 
> Testing
> ---
> 
> Tested this on OS X.
> 
> Without this patch:
> 
> ```
> $ ./support/apply-review.sh -n -r 39086
> 2015-10-07 16:41:06 URL:https://reviews.apache.org/r/39086/diff/raw/ 
> [5430/5430] -> "39086.patch" [1]
> Traceback (most recent call last):
>   File "./support/jsonurl.py", line 25, in 
> print data
> UnicodeEncodeError: 'ascii' codec can't encode character u'\xf3' in position 
> 4: ordinal not in range(128)
> Successfully applied: Fixed typos in comments and docs.
> 
> Fixed typos in comments and docs.
> 
> 
> Review: https://reviews.apache.org/r/39086
> fatal: empty ident name (for ) not allowed
> Failed to commit patch
> $
> ```
> 
> With this patch:
> 
> ```
> $ ./support/apply-review.sh -n -r 39086
> 2015-10-07 16:42:09 URL:https://reviews.apache.org/r/39086/diff/raw/ 
> [5430/5430] -> "39086.patch" [1]
> Successfully applied: Fixed typos in comments and docs.
> 
> Fixed typos in comments and docs.
> 
> 
> Review: https://reviews.apache.org/r/39086
> Checking 2 files using filter 
> --filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/operators,+whitespace/semicolon,+whitespace/tab,+whitespace/todo
> Total errors found: 0
> [fix_3268 d51d32a] Fixed typos in comments and docs.
>  Author: Gastón Kleiman 
>  6 files changed, 12 insertions(+), 12 deletions(-)
> $
> ```
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>



Re: Review Request 39005: stout: Added thread-safe replacement for strerror.

2015-10-07 Thread Bernd Mathiske

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



3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp (line 27)


Suggestion: "In contast to strerror(), this function is thread-safe."



3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp (line 37)


If you wanted to be really devensive, not trusting strerror_r() on all 
unforeseeable platforms for all eternity, we could cut the loop at a max size.

But then maybe we could just use a larger buffer to begin with and be done?

Or, try 34 first and then something large like 1024, and that's it.


- Bernd Mathiske


On Oct. 6, 2015, 8:07 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39005/
> ---
> 
> (Updated Oct. 6, 2015, 8:07 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds a thread-safe wrapper around strerror_r which has semantics similar 
> to strerror. We plan to use this at call sites currently relying on strerror.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp 
> 12ba1ca861114e60f8276c0ee91c543abcfc2519 
>   3rdparty/libprocess/3rdparty/stout/tests/error_tests.cpp 
> 9e7605c53e6636e7fea32e4f69fbaff9100a979f 
> 
> Diff: https://reviews.apache.org/r/39005/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Review Request 39100: Changed Credential validation

2015-10-07 Thread Isabel Jimenez

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

We should validate credentials before anything else in HTTP endpoints.


Diffs
-

  src/master/http.cpp 4b9f9ed 

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


Testing
---

make check


Thanks,

Isabel Jimenez



Re: Review Request 39100: Changed Credential validation

2015-10-07 Thread Isabel Jimenez

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

(Updated Oct. 7, 2015, 7:15 p.m.)


Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

We should validate credentials before anything else in HTTP endpoints.


Diffs
-

  src/master/http.cpp 4b9f9ed 

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


Testing
---

make check


Thanks,

Isabel Jimenez



Review Request 39097: CMake:[2/2] remove `__WINDOWS__` flag definition from Stout config.

2015-10-07 Thread Alex Clemmer

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

Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

CMake:[2/2] remove `__WINDOWS__` flag definition from Stout config.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
c4d1b7b3f896f8037d922fafcdbdd0e31a70e140 

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


Testing
---


Thanks,

Alex Clemmer



Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-10-07 Thread Timothy Chen


> On Oct. 6, 2015, 9:53 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/http.cpp, line 76
> > 
> >
> > We avoid static non-POD types due to destruction issues. Can you put 
> > this on the heap?
> > 
> > Per my suggestion above, perhaps we can have `Status::string(uint16_t 
> > code)` as a static function on the `Status` class that provides the string 
> > version of the status.
> > 
> > This should clean up the tests as well.

I'm not sure how this will clean up the tests since you need to still check if 
the code is in the statuses map, I think it just avoids the lookup yourself.
Anyhow I'm going to update the review as you suggested.


- Timothy


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


On Oct. 6, 2015, 8:27 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Oct. 6, 2015, 8:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 591c1a959057155e1bf0f5bd73352e78d1c15cb3 
>   3rdparty/libprocess/src/decoder.hpp 
> 53b8079968a0145651bad9d11aa4be2b504de57a 
>   3rdparty/libprocess/src/http.cpp ebf76093d654397260fc8da84c4f2b0fd6541483 
>   3rdparty/libprocess/src/process.cpp 
> d1c81f1d244f02bf42cab97198587ce1b8c7c407 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> bb9ced8933bf2bb97ae6b3cffdb5528676e53c11 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 38f3ad7e46f5b6ef4850cdf7fdcc115715e98472 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> ffd260a3fa2e49b3de183ba7b392b71afaaba2e5 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 39088: Made shell test locale-independent.

2015-10-07 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [39088]

All tests passed.

- Mesos ReviewBot


On Oct. 7, 2015, 3:24 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39088/
> ---
> 
> (Updated Oct. 7, 2015, 3:24 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Jan Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3517
> https://issues.apache.org/jira/browse/MESOS-3517
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The test environment does not enforce that we run with a defined locale,
> so instead enforce it for our particular use case (here: potentially
> localized error messages from shell).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> e6d36ec1bf414b52d0899f0edf83e0ad8910dd0e 
> 
> Diff: https://reviews.apache.org/r/39088/diff/
> 
> 
> Testing
> ---
> 
> With German locale installed:
> 
> % LANG=de_DE.UTF-8 make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 39091: CMake:[1/3] Prepared stout tests to run with `make check`.

2015-10-07 Thread Joseph Wu

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

Ship it!


Ship It!

- Joseph Wu


On Oct. 7, 2015, 9 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39091/
> ---
> 
> (Updated Oct. 7, 2015, 9 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3597
> https://issues.apache.org/jira/browse/MESOS-3597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Right now running the Mesos tests from the default tests target will
> (1) aggregate all our tests into a semi-useless report that hides all
> the errors, and (2) strip all our colors when you do pass the verbosity
> flag to get rid of the "report" structure.
> 
> This commit will prepare us to run the tests with with color, without
> this default reporting structure. We do this by moving the tests target
> to a `CACHE STRING` which can be referenced by the tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
> c4d1b7b3f896f8037d922fafcdbdd0e31a70e140 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
> 
> Diff: https://reviews.apache.org/r/39091/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 39093: CMake:[3/3] Add `make check` target.

2015-10-07 Thread Joseph Wu

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


Applied the chain and ran:
```
cmake ..
make
make check
```

Only libprocess tests ran.  But there was **color** (yay! :)


CMakeLists.txt (line 97)


This only runs the libprocess tests.  Did you mean to do this?


- Joseph Wu


On Oct. 7, 2015, 9:01 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39093/
> ---
> 
> (Updated Oct. 7, 2015, 9:01 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3597
> https://issues.apache.org/jira/browse/MESOS-3597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake:[3/3] Add `make check` target.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt a5a66c13496a95bb7095504a4645909d887fb49f 
> 
> Diff: https://reviews.apache.org/r/39093/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 39018: Added JSON parsing for Resources.

2015-10-07 Thread haosdent huang

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

Ship it!


Ship It!

- haosdent huang


On Oct. 7, 2015, 3:24 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 7, 2015, 3:24 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 39076: CMake: Added ability of Windows builds to include protobuf headers.

2015-10-07 Thread Joseph Wu

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

Ship it!



3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake (lines 105 - 109)


Nit: Can you change this to a if-else (minus the elseif)?


- Joseph Wu


On Oct. 7, 2015, 1:31 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39076/
> ---
> 
> (Updated Oct. 7, 2015, 1:31 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Added ability of Windows builds to include protobuf headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> ee1c74d31e28136bf289f4100d79a8ce568cd3af 
> 
> Diff: https://reviews.apache.org/r/39076/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 39092: CMake:[2/3] prepared process library tests to run with `make check`.

2015-10-07 Thread Joseph Wu

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

Ship it!


Ship It!

- Joseph Wu


On Oct. 7, 2015, 9 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39092/
> ---
> 
> (Updated Oct. 7, 2015, 9 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3597
> https://issues.apache.org/jira/browse/MESOS-3597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Right now running the Mesos tests from the default tests target will (1)
> aggregate all our tests into a semi-useless report that hides all the
> errors, and (2) strip all our colors when you do pass the verbosity flag
> to get rid of the "report" structure.
> 
> This commit will prepare us to run the tests with with color, without
> this default reporting structure. We do this by moving the tests target
> to a `CACHE STRING` which can be referenced by the tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> ea6db09e1a1aa01450aee93814e07f09feae7ac9 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> a14b5b8fe22d3e75bef3c716ae7865ddaf132927 
> 
> Diff: https://reviews.apache.org/r/39092/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>