Re: Review Request 38750: Updated changelog for 0.25.0

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38750]

All tests passed.

- Mesos ReviewBot


On Sept. 25, 2015, 5:13 a.m., Niklas Nielsen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38750/
> ---
> 
> (Updated Sept. 25, 2015, 5:13 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated changelog for 0.25.0
> 
> 
> Diffs
> -
> 
>   CHANGELOG 18af16785ca969740bd0eb5e1dee985e2609dfb2 
> 
> Diff: https://reviews.apache.org/r/38750/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>

Re: Review Request 38747: Adding digest utilities

[Alex Clemmer]

> On Sept. 25, 2015, 6:24 a.m., Alex Clemmer wrote:
> > Hey, it doesn't look like `src/tests/digest_tests.cpp` is being added to 
> > the `CMakeLists.txt` file in `3rdparty/libprocess/src/tests`. Is that 
> > correct? If not, could we please add it there as well?
> > 
> > (I have forgotten to add a list of header files as dependencies, so you 
> > can't add that quite yet.)

Ah, after testing, we won't have to include the header lists at all, so you 
don't have to add that to a list. It turns out that if you pass the `include/` 
folder into the `include_directory` function, it automatically will monitor 
those files for changes. :) So we're good.

But, unless I'm mistaken, we'd still have to add that test to the 
`CMakeLists.txt`. Let me know if my thinking is wrong here.


- Alex


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


On Sept. 25, 2015, 5:46 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> ---
> 
> (Updated Sept. 25, 2015, 5:46 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added SHA256 implementation.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 064310c61d8a0c5c5a87f305f1f62149dd3a6699 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
>   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 37999: Implemented http::AuthenticatorManager

[Alex Clemmer]

> On Sept. 23, 2015, 5:39 p.m., Alex Clemmer wrote:
> > Can we please add this file to the CMakeLists.txt file in 
> > 3rdparty/libprocess/src as well?
> 
> Alexander Rojas wrote:
> The included file is a header and so far cmake file doesn't seem to care 
> about the include directory (can you correct me if I'm wrong). Still, 
> shouln't the headers be there too? otherwise cmake wouldn't do proper 
> dependency analysis.
> 
> Alex Clemmer wrote:
> Oh, yes, my bad, those changes aren't committed yet. (I have too many 
> reviews up and lose track of them all, sorry.)

To follow up, actually... we won't have to commit the header file lists after 
all! After investigation, it turns out that all CMake needs is to have the 
directory linked with `include_directory` and it just works. (I apologize for 
the lack of clarity here... I had never used CMake before I started this, and 
the truth is that when you write hundreds of lines of code in an unfamiliar 
language, you sometimes forget what is true and what is false. :) )


- Alex


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


On Sept. 23, 2015, 8:57 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Sept. 23, 2015, 8:57 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the authenticator manager, which is a class which handles the 
> actual authentication procedure during the execution of 
> `ProcessManager::handle()` and it also takes care of the life cycle of 
> instances of http::Authenticator.
> 
> No tests are added at this point since no public API is generated, the goal 
> of this patch is to implement the manager and verify nothing breaks 
> afterwards. Authenticator manager tests proper come in a latter patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38747: Adding digest utilities

[Alex Clemmer]

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


Hey, it doesn't look like `src/tests/digest_tests.cpp` is being added to the 
`CMakeLists.txt` file in `3rdparty/libprocess/src/tests`. Is that correct? If 
not, could we please add it there as well?

(I have forgotten to add a list of header files as dependencies, so you can't 
add that quite yet.)

- Alex Clemmer


On Sept. 25, 2015, 5:46 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> ---
> 
> (Updated Sept. 25, 2015, 5:46 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added SHA256 implementation.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 064310c61d8a0c5c5a87f305f1f62149dd3a6699 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38747/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Review Request 38756: CMake: Add support for compiling the agent with CMake.

[Alex Clemmer]

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

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


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


Repository: mesos


Description
---

CMake: Add support for compiling the agent with CMake.


Diffs
-

  CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
  cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
  src/CMakeLists.txt PRE-CREATION 
  src/cmake/MesosProtobuf.cmake PRE-CREATION 
  src/slave/cmake/AgentConfigure.cmake PRE-CREATION 

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


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer

Review Request 38754: CMake: Add build/configure/install logic for Zookeeper.

[Alex Clemmer]

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

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


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


Repository: mesos


Description
---

CMake: Add build/configure/install logic for Zookeeper.


Diffs
-

  3rdparty/CMakeLists.txt PRE-CREATION 
  3rdparty/cmake/Mesos3rdpartyConfigure.cmake PRE-CREATION 
  3rdparty/cmake/Versions.cmake PRE-CREATION 
  CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
  cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 

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


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer

Review Request 38755: CMake: Move compiler configuration logic to its own file.

[Alex Clemmer]

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

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


Repository: mesos


Description
---

CMake: Move compiler configuration logic to its own file.


Diffs
-

  cmake/CompilationConfigure.cmake PRE-CREATION 
  cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 

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


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer

Review Request 38753: CMake: Transition Stout tests to use new third-party build scripts.

[Alex Clemmer]

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

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


Repository: mesos


Description
---

CMake: Transition Stout tests to use new third-party build scripts.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
08325297ceb79b80c305ba4f2164ffd37591a0e8 

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


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer

Review Request 38752: CMake: Transition Mesos to use new third-party build scripts.

[Alex Clemmer]

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

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


Repository: mesos


Description
---

CMake: Transition Mesos to use new third-party build scripts.


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
  cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 

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


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer

Review Request 38751: CMake: Pull third-party configuration logic into its own .cmake file.

[Alex Clemmer]

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

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


Repository: mesos


Description
---

CMake: Pull third-party configuration logic into its own .cmake file.


Diffs
-

  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake PRE-CREATION 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake 
a5f8d399e151acad87bb72ecb1f7372b2c467423 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 

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


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer

Re: Review Request 38747: Adding digest utilities

[Jojy Varghese]

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

(Updated Sept. 25, 2015, 5:46 a.m.)


Review request for mesos, Gilbert Song and Timothy Chen.


Changes
---

build fix


Repository: mesos


Description
---

Added SHA256 implementation.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 064310c61d8a0c5c5a87f305f1f62149dd3a6699 
  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
  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 38747: Adding digest utilities

[Jojy Varghese]

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

(Updated Sept. 25, 2015, 5:43 a.m.)


Review request for mesos, Gilbert Song and Timothy Chen.


Changes
---

minor fixes.


Repository: mesos


Description
---

Added SHA256 implementation.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 064310c61d8a0c5c5a87f305f1f62149dd3a6699 
  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38747: Adding digest utilities

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [38443, 38579, 38580, 38747]

Failed command: make -j3 distcheck

Error:
 make  dist-gzip am__post_remove_distdir='@:'
make[1]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
if test -d "mesos-0.25.0"; then find "mesos-0.25.0" -type d ! -perm -200 -exec 
chmod u+w {} ';' && rm -rf "mesos-0.25.0" || { sleep 5 && rm -rf 
"mesos-0.25.0"; }; else :; fi
test -d "mesos-0.25.0" || mkdir "mesos-0.25.0"
 (cd 3rdparty && make  top_distdir=../mesos-0.25.0 
distdir=../mesos-0.25.0/3rdparty \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
 (cd libprocess && make  top_distdir=../../mesos-0.25.0 
distdir=../../mesos-0.25.0/3rdparty/libprocess \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[3]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
make[3]: *** No rule to make target `src/tests/digest_tests.cpp', needed by 
`distdir'.  Stop.
make[3]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
make[2]: *** [distdir] Error 1
make[2]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
make[1]: *** [distdir] Error 1
make[1]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
make: *** [dist] Error 2

- Mesos ReviewBot


On Sept. 25, 2015, 3:07 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> ---
> 
> (Updated Sept. 25, 2015, 3:07 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added SHA256 implementation.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 064310c61d8a0c5c5a87f305f1f62149dd3a6699 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38747/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Review Request 38750: Updated changelog for 0.25.0

[Niklas Nielsen]

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

Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

Updated changelog for 0.25.0


Diffs
-

  CHANGELOG 18af16785ca969740bd0eb5e1dee985e2609dfb2 

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


Testing
---


Thanks,

Niklas Nielsen

Re: Review Request 38749: Added `image_providers` flags to configuration.md.

[Michael Park]

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

Ship it!


Ship It!

- Michael Park


On Sept. 25, 2015, 4:57 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38749/
> ---
> 
> (Updated Sept. 25, 2015, 4:57 a.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `image_providers` flags to configuration.md.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md a09bed10bcdac86dd992c5437dfaebecb7f423d4 
> 
> Diff: https://reviews.apache.org/r/38749/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38749: Added `image_providers` flags to configuration.md.

[Niklas Nielsen]

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

Ship it!


Ship It!

- Niklas Nielsen


On Sept. 24, 2015, 9:57 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38749/
> ---
> 
> (Updated Sept. 24, 2015, 9:57 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `image_providers` flags to configuration.md.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md a09bed10bcdac86dd992c5437dfaebecb7f423d4 
> 
> Diff: https://reviews.apache.org/r/38749/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38746: Added TaskStatus::Reason to containerizer Termination message.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38746]

All tests passed.

- Mesos ReviewBot


On Sept. 25, 2015, 2:14 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38746/
> ---
> 
> (Updated Sept. 25, 2015, 2:14 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Timothy Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-2035
> https://issues.apache.org/jira/browse/MESOS-2035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added TaskStatus::Reason to containerizer Termination message.
> 
> 
> Diffs
> -
> 
>   include/mesos/containerizer/containerizer.proto 
> f16ccc89f83da28c413ccfa0687a06b7515a605c 
>   include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999 
>   include/mesos/slave/isolator.proto 9d38a25470a79af1eda9122c037c82b8cbbad6ed 
>   src/common/protobuf_utils.hpp 8793851fb927ab1326da6b6a424b3c6a75eb5001 
>   src/common/protobuf_utils.cpp c1e8e011e6d0b8e1cd8a836e3168685ec401b21b 
>   src/slave/containerizer/docker.cpp efa37266368ee12fea9134b35ebc5047a2820f94 
>   src/slave/containerizer/isolators/cgroups/mem.cpp 
> 89c86beb9227eb8a6e70a413e7b3934add652981 
>   src/slave/containerizer/isolators/posix/disk.cpp 
> c324c79f8d598095d07fbcb26e806a0978c2a520 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 4c1419290645ad4c44360a81618a6cea7ad190df 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 8771ef661039310d79845513ea4602e15b2ad13d 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 5bc7d408bda0c249e1b66747d8bd87e688362e6c 
>   src/tests/slave_recovery_tests.cpp fd285fbba0ec08fe2afbdd8da0b0b451126da0eb 
>   src/tests/slave_tests.cpp dccdbb0ec5352df5da63d5ef7261bfc7fd599acb 
> 
> Diff: https://reviews.apache.org/r/38746/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> @tnachen, I need you to help test this patch with docker containerizer.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 38749: Added `image_providers` flags to configuration.md.

[Joris Van Remoortere]

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

Review request for mesos and Niklas Nielsen.


Repository: mesos


Description
---

Added `image_providers` flags to configuration.md.


Diffs
-

  docs/configuration.md a09bed10bcdac86dd992c5437dfaebecb7f423d4 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 38338: Enhanced option for Docker cli volume plugin.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38338]

All tests passed.

- Mesos ReviewBot


On Sept. 25, 2015, 1:12 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38338/
> ---
> 
> (Updated Sept. 25, 2015, 1:12 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3392
> https://issues.apache.org/jira/browse/MESOS-3392
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhanced option for Docker cli volume plugin.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999 
>   src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 
> 
> Diff: https://reviews.apache.org/r/38338/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

[Klaus Ma]

> On Sept. 24, 2015, 3:52 p.m., Jan Schlicht wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 609
> > 
> >
> > s/protobuf(const T& message)/protobuf(const google::protobuf::Message& 
> > message)
> > 
> > The templatization in 608 isn't necessary here, as the static_assert in 
> > 611 has the same effect as just using a google::protobuf::Message as 
> > function parameter. Please change the function signature and remove line 
> > 608 & 611,612.
> > 
> > Also, it'd make sense to return a `Try` as parsing `message` 
> > may fail. The current behavior is to call ABORT, one could as well return 
> > `None` here.

Used template here because there's a link error if we used `protobuf(const 
google::protobuf::Message& message)`; the way to resovle is to move 
implementation into cpp file, but stout is a library only including headers.

`static_assert` will generate error when compiling, it will check error before 
running; so we can use `Object` directly :).


> On Sept. 24, 2015, 3:52 p.m., Jan Schlicht wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 774
> > 
> >
> > Same as before: It'd make sense to return a `Try` here.

Same answer to `Object` :).


> On Sept. 24, 2015, 3:52 p.m., Jan Schlicht wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 615
> > 
> >
> > This comment describes a behavior of the function and should be moved 
> > to 608 (= above the function signature).

I'll address it :).


- Klaus


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


On Sept. 24, 2015, 3 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38342/
> ---
> 
> (Updated Sept. 24, 2015, 3 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 2285ce9 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 68328a2 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 8ebb798 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 34eb6d0 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 920f5c9 
> 
> Diff: https://reviews.apache.org/r/38342/diff/
> 
> 
> Testing
> ---
> 
> cd 3rdparty/libprocess/3rdparty/stout
> ./boostrap
> ./configure
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38738: Make common public header type_utils symmetrical to v1 mesos.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [38726]

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

Error:
 2015-09-25 03:10:05 URL:https://reviews.apache.org/r/38726/diff/raw/ 
[1327/1327] -> "38726.patch" [1]
error: patch failed: src/common/attributes.cpp:101
error: src/common/attributes.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Sept. 25, 2015, 12:45 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38738/
> ---
> 
> (Updated Sept. 25, 2015, 12:45 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-3510
> https://issues.apache.org/jira/browse/MESOS-3510
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This aids in verifying the files are kept in sync.
> diff include/mesos/type_utils.cpp include/mesos/v1/mesos.cpp should
> result in only include and namespace differences.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/module.hpp e83be2822b7c0e7935ab1c8af36e5cb9b5180f20 
>   include/mesos/scheduler/scheduler.hpp 
> 5c311914eaabfa1358da5ae78c1d969089468cc1 
>   include/mesos/type_utils.hpp 6cedf079c710c7d6ab8f95c47c133e6a1efe9a82 
>   include/mesos/v1/mesos.hpp 260e1125dea792ebcece404f94363e8c4bc36f28 
>   include/mesos/v1/mesos.proto 58e5a6b7912d6f96d495a34d0913b9d27ad65fb5 
> 
> Diff: https://reviews.apache.org/r/38738/diff/
> 
> 
> Testing
> ---
> 
> Most notable is the ACL streamer which I wasn't sure if we purposely moved 
> into mesos as opposed to keeping the authorization directory in the previous 
> api.
> diff include/mesos/type_utils.hpp include/mesos/v1/mesos.hpp
> ```
> 19,20c19,20
> < #ifndef __MESOS_TYPE_UTILS_H__
> < #define __MESOS_TYPE_UTILS_H__
> ---
> > #ifndef __MESOS_V1_HPP__
> > #define __MESOS_V1_HPP__
> 26c26
> < #include 
> ---
> > #include  // ONLY USEFUL AFTER RUNNING PROTOC.
> 28,33d27
> < #include 
> < 
> < #include 
> < 
> < #include 
> < #include 
> 35d28
> < #include 
> 40c33
> < // corresponding definitions are in src/common/type_utils.cpp.
> ---
> > // corresponding definitions are in src/v1/type_utils.cpp.
> 44c37
> < // operators declared in type_utils.hpp. Exposing type_utils.hpp
> ---
> > // operators declared in mesos.hpp. Exposing mesos.hpp
> 48a42
> > namespace v1 {
> 61c55
> < bool operator==(const SlaveInfo& left, const SlaveInfo& right);
> ---
> > bool operator==(const AgentInfo& left, const AgentInfo& right);
> 104c98
> < inline bool operator==(const SlaveID& left, const SlaveID& right)
> ---
> > inline bool operator==(const AgentID& left, const AgentID& right)
> 152c146
> < inline bool operator==(const SlaveID& left, const std::string& right)
> ---
> > inline bool operator==(const AgentID& left, const std::string& right)
> 197c191
> < inline bool operator!=(const SlaveID& left, const SlaveID& right)
> ---
> > inline bool operator!=(const AgentID& left, const AgentID& right)
> 239c233
> < inline bool operator<(const SlaveID& left, const SlaveID& right)
> ---
> > inline bool operator<(const AgentID& left, const AgentID& right)
> 309c303
> < inline std::ostream& operator<<(std::ostream& stream, const SlaveID& 
> slaveId)
> ---
> > inline std::ostream& operator<<(std::ostream& stream, const AgentID& 
> > agentId)
> 311c305
> <   return stream << slaveId.value();
> ---
> >   return stream << agentId.value();
> 315c309
> < inline std::ostream& operator<<(std::ostream& stream, const SlaveInfo& 
> slave)
> ---
> > inline std::ostream& operator<<(std::ostream& stream, const AgentInfo& 
> > agent)
> 317c311
> <   return stream << slave.DebugString();
> ---
> >   return stream << agent.DebugString();
> 412a407
> > } // namespace v1 {
> 418c413
> < struct hash
> ---
> > struct hash
> 422c417
> <   typedef mesos::CommandInfo_URI argument_type;
> ---
> >   typedef mesos::v1::CommandInfo::URI argument_type;
> 443c438
> < struct hash
> ---
> > struct hash
> 447c442
> <   typedef mesos::ContainerID argument_type;
> ---
> >   typedef mesos::v1::ContainerID argument_type;
> 459c454
> < struct hash
> ---
> > struct hash
> 463c458
> <   typedef mesos::ExecutorID argument_type;
> ---
> >   typedef mesos::v1::ExecutorID argument_type;
> 475c470
> < struct hash
> ---
> > struct hash
> 479c474
> <   typedef mesos::FrameworkID argument_type;
> ---
> >   typedef mesos::v1::FrameworkID argument_type;
> 491c486
> < struct hash
> ---
> > struct hash
> 495c490
> <   typedef mesos::OfferID argument_type;
> ---
> >   typedef mesos::v1::OfferID argument_type;
> 507c502
> < struct hash
> ---
> > struct hash
> 511c506
> <   typedef mesos::SlaveID argument_type;
> ---
> >   typedef mesos::v

Review Request 38747: Adding digest utilities

[Jojy Varghese]

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

Review request for mesos, Gilbert Song and Timothy Chen.


Repository: mesos


Description
---

Added SHA256 implementation.


Diffs
-

  3rdparty/libprocess/Makefile.am 064310c61d8a0c5c5a87f305f1f62149dd3a6699 
  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38654: Added digest verifier for image blobs

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38443, 38579, 38580, 38654]

All tests passed.

- Mesos ReviewBot


On Sept. 25, 2015, 12:38 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38654/
> ---
> 
> (Updated Sept. 25, 2015, 12:38 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provides interface for verifying a blob of data for its digest and given 
> digest type.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 2286366852b272c808490b6bd87ac9e894de57ac 
>   src/slave/containerizer/provisioner/verifier/digest_verifier.hpp 
> PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 1b0c30450a07d01d1a38d460bc7d921c80be7e3b 
> 
> Diff: https://reviews.apache.org/r/38654/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Review Request 38746: Added TaskStatus::Reason to containerizer Termination message.

[Jie Yu]

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

Review request for mesos, Ben Mahler, Timothy Chen, and Vinod Kone.


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


Repository: mesos


Description
---

Added TaskStatus::Reason to containerizer Termination message.


Diffs
-

  include/mesos/containerizer/containerizer.proto 
f16ccc89f83da28c413ccfa0687a06b7515a605c 
  include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999 
  include/mesos/slave/isolator.proto 9d38a25470a79af1eda9122c037c82b8cbbad6ed 
  src/common/protobuf_utils.hpp 8793851fb927ab1326da6b6a424b3c6a75eb5001 
  src/common/protobuf_utils.cpp c1e8e011e6d0b8e1cd8a836e3168685ec401b21b 
  src/slave/containerizer/docker.cpp efa37266368ee12fea9134b35ebc5047a2820f94 
  src/slave/containerizer/isolators/cgroups/mem.cpp 
89c86beb9227eb8a6e70a413e7b3934add652981 
  src/slave/containerizer/isolators/posix/disk.cpp 
c324c79f8d598095d07fbcb26e806a0978c2a520 
  src/slave/containerizer/mesos/containerizer.hpp 
4c1419290645ad4c44360a81618a6cea7ad190df 
  src/slave/containerizer/mesos/containerizer.cpp 
b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
  src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
  src/tests/containerizer/docker_containerizer_tests.cpp 
8771ef661039310d79845513ea4602e15b2ad13d 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
5bc7d408bda0c249e1b66747d8bd87e688362e6c 
  src/tests/slave_recovery_tests.cpp fd285fbba0ec08fe2afbdd8da0b0b451126da0eb 
  src/tests/slave_tests.cpp dccdbb0ec5352df5da63d5ef7261bfc7fd599acb 

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


Testing
---

sudo make check

@tnachen, I need you to help test this patch with docker containerizer.


Thanks,

Jie Yu

Re: Review Request 38646: Added WIP note for Executor endpoint in changelog

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38646]

All tests passed.

- Mesos ReviewBot


On Sept. 25, 2015, 12:09 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38646/
> ---
> 
> (Updated Sept. 25, 2015, 12:09 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Note for the 0.25 release that `api/v1/executor` endpoint is incomplete.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 7a0c813847590e8e384054551381e4f13f5c67d2 
> 
> Diff: https://reviews.apache.org/r/38646/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 38743: files: fix potential fd leaks under error conditions.

[Ben Mahler]

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

Ship it!


Can't wait to have a ref-counted abstraction to avoid this!

- Ben Mahler


On Sept. 24, 2015, 11:39 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38743/
> ---
> 
> (Updated Sept. 24, 2015, 11:39 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> files: fix potential fd leaks under error conditions.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp 1d4b1e6d2fcd99321084515f538aa357f8d9b305 
> 
> Diff: https://reviews.apache.org/r/38743/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 38338: Enhanced option for Docker cli volume plugin.

[haosdent huang]

> On Sept. 24, 2015, 11:28 p.m., Vaibhav Khanduja wrote:
> > src/docker/docker.cpp, line 422
> > 
> >
> > volumn - "volume"

fixed.


- haosdent


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


On Sept. 25, 2015, 1:12 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38338/
> ---
> 
> (Updated Sept. 25, 2015, 1:12 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3392
> https://issues.apache.org/jira/browse/MESOS-3392
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhanced option for Docker cli volume plugin.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999 
>   src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 
> 
> Diff: https://reviews.apache.org/r/38338/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38338: Enhanced option for Docker cli volume plugin.

[haosdent huang]

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

(Updated Sept. 25, 2015, 1:12 a.m.)


Review request for mesos and Timothy Chen.


Changes
---

Update accroding @vaibhavkhanduja reviews.


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


Repository: mesos


Description
---

Enhanced option for Docker cli volume plugin.


Diffs (updated)
-

  include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999 
  src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 37903: stout: Fix bug in IPNetwork::create() with zero prefix.

[Mesos ReviewBot]

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


Bad review!

Reviews applied: []

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

- Mesos ReviewBot


On Sept. 24, 2015, 9:29 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37903/
> ---
> 
> (Updated Sept. 24, 2015, 9:29 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3328
> https://issues.apache.org/jira/browse/MESOS-3328
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous coding would try to shift a uint32_t value 32 bits to the left; 
> per
> C++ spec, this yields undefined behavior. On my machine, this resulted in
> treating a prefix of 0 as equivalent to a prefix of 32, which is obviously
> wrong.
> 
> Spotted via ubsan: see MESOS-3328.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
> 1ad119d54820e97497b1773518875be25ddbf98a 
>   3rdparty/libprocess/3rdparty/stout/tests/ip_tests.cpp 
> b0cbcb38cfcb923ec7c185bacf139ceb0a28924f 
> 
> Diff: https://reviews.apache.org/r/37903/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38738: Make common public header type_utils symmetrical to v1 mesos.

[Joris Van Remoortere]

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

(Updated Sept. 25, 2015, 12:45 a.m.)


Review request for mesos, Benjamin Hindman and Ben Mahler.


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


Repository: mesos


Description (updated)
---

This aids in verifying the files are kept in sync.
diff include/mesos/type_utils.cpp include/mesos/v1/mesos.cpp should
result in only include and namespace differences.


Diffs (updated)
-

  include/mesos/module/module.hpp e83be2822b7c0e7935ab1c8af36e5cb9b5180f20 
  include/mesos/scheduler/scheduler.hpp 
5c311914eaabfa1358da5ae78c1d969089468cc1 
  include/mesos/type_utils.hpp 6cedf079c710c7d6ab8f95c47c133e6a1efe9a82 
  include/mesos/v1/mesos.hpp 260e1125dea792ebcece404f94363e8c4bc36f28 
  include/mesos/v1/mesos.proto 58e5a6b7912d6f96d495a34d0913b9d27ad65fb5 

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


Testing (updated)
---

Most notable is the ACL streamer which I wasn't sure if we purposely moved into 
mesos as opposed to keeping the authorization directory in the previous api.
diff include/mesos/type_utils.hpp include/mesos/v1/mesos.hpp
```
19,20c19,20
< #ifndef __MESOS_TYPE_UTILS_H__
< #define __MESOS_TYPE_UTILS_H__
---
> #ifndef __MESOS_V1_HPP__
> #define __MESOS_V1_HPP__
26c26
< #include 
---
> #include  // ONLY USEFUL AFTER RUNNING PROTOC.
28,33d27
< #include 
< 
< #include 
< 
< #include 
< #include 
35d28
< #include 
40c33
< // corresponding definitions are in src/common/type_utils.cpp.
---
> // corresponding definitions are in src/v1/type_utils.cpp.
44c37
< // operators declared in type_utils.hpp. Exposing type_utils.hpp
---
> // operators declared in mesos.hpp. Exposing mesos.hpp
48a42
> namespace v1 {
61c55
< bool operator==(const SlaveInfo& left, const SlaveInfo& right);
---
> bool operator==(const AgentInfo& left, const AgentInfo& right);
104c98
< inline bool operator==(const SlaveID& left, const SlaveID& right)
---
> inline bool operator==(const AgentID& left, const AgentID& right)
152c146
< inline bool operator==(const SlaveID& left, const std::string& right)
---
> inline bool operator==(const AgentID& left, const std::string& right)
197c191
< inline bool operator!=(const SlaveID& left, const SlaveID& right)
---
> inline bool operator!=(const AgentID& left, const AgentID& right)
239c233
< inline bool operator<(const SlaveID& left, const SlaveID& right)
---
> inline bool operator<(const AgentID& left, const AgentID& right)
309c303
< inline std::ostream& operator<<(std::ostream& stream, const SlaveID& slaveId)
---
> inline std::ostream& operator<<(std::ostream& stream, const AgentID& agentId)
311c305
<   return stream << slaveId.value();
---
>   return stream << agentId.value();
315c309
< inline std::ostream& operator<<(std::ostream& stream, const SlaveInfo& slave)
---
> inline std::ostream& operator<<(std::ostream& stream, const AgentInfo& agent)
317c311
<   return stream << slave.DebugString();
---
>   return stream << agent.DebugString();
412a407
> } // namespace v1 {
418c413
< struct hash
---
> struct hash
422c417
<   typedef mesos::CommandInfo_URI argument_type;
---
>   typedef mesos::v1::CommandInfo::URI argument_type;
443c438
< struct hash
---
> struct hash
447c442
<   typedef mesos::ContainerID argument_type;
---
>   typedef mesos::v1::ContainerID argument_type;
459c454
< struct hash
---
> struct hash
463c458
<   typedef mesos::ExecutorID argument_type;
---
>   typedef mesos::v1::ExecutorID argument_type;
475c470
< struct hash
---
> struct hash
479c474
<   typedef mesos::FrameworkID argument_type;
---
>   typedef mesos::v1::FrameworkID argument_type;
491c486
< struct hash
---
> struct hash
495c490
<   typedef mesos::OfferID argument_type;
---
>   typedef mesos::v1::OfferID argument_type;
507c502
< struct hash
---
> struct hash
511c506
<   typedef mesos::SlaveID argument_type;
---
>   typedef mesos::v1::AgentID argument_type;
513c508
<   result_type operator()(const argument_type& slaveId) const
---
>   result_type operator()(const argument_type& agentId) const
516c511
< boost::hash_combine(seed, slaveId.value());
---
> boost::hash_combine(seed, agentId.value());
523c518
< struct hash
---
> struct hash
527c522
<   typedef mesos::TaskID argument_type;
---
>   typedef mesos::v1::TaskID argument_type;
539c534
< struct hash
---
> struct hash
543c538
<   typedef mesos::TaskState argument_type;
---
>   typedef mesos::v1::TaskState argument_type;
554c549
< struct hash
---
> struct hash
558c553
<   typedef mesos::TaskStatus_Source argument_type;
---
>   typedef mesos::v1::TaskStatus_Source argument_type;
569c564
< struct hash
---
> struct hash
573c568
<   typedef mesos::TaskStatus_Reason argument_type;
---
>   typedef mesos::v1::TaskStatus_Reason argument_type;
584c579
< struct hash
---
> struct hash
588c583
<   typedef mesos::Image::Type 

Re: Review Request 38468: docs: Added discussion of finding a shepherd.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38468]

All tests passed.

- Mesos ReviewBot


On Sept. 24, 2015, 9:29 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38468/
> ---
> 
> (Updated Sept. 24, 2015, 9:29 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added suggestion that new contributors seek to find a shepherd for their work
> before their begin writing code. Made a few other improvements.
> 
> 
> Diffs
> -
> 
>   docs/reporting-a-bug.md 6c7f74719a8586f0608eb0f0f77d15c8d534321d 
>   docs/submitting-a-patch.md 754a16f9b43630880f0f6c4a8e8e2f5e081b0a87 
> 
> Diff: https://reviews.apache.org/r/38468/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38654: Added digest verifier for image blobs

[Jojy Varghese]

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

(Updated Sept. 25, 2015, 12:38 a.m.)


Review request for mesos, Gilbert Song and Timothy Chen.


Changes
---

added file digest.


Repository: mesos


Description
---

Provides interface for verifying a blob of data for its digest and given digest 
type.


Diffs (updated)
-

  src/Makefile.am 2286366852b272c808490b6bd87ac9e894de57ac 
  src/slave/containerizer/provisioner/verifier/digest_verifier.hpp PRE-CREATION 
  src/tests/containerizer/provisioner_docker_tests.cpp 
1b0c30450a07d01d1a38d460bc7d921c80be7e3b 

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


Testing
---


Thanks,

Jojy Varghese

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Joseph Wu]

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


Mostly just python style and commenting.  My python is a bit rusty though.

Could you also mention in "Testing Done" whether this is python 2 or 3 or both?


support/apply-reviews.py (line 9)


Spaces around `=`?



support/apply-reviews.py (line 11)


Could you change these method comments to python docstrings comments?

i.e.
```
def review_url(id):
"""Returns the Review Board URL for the given review ID."""
```



support/apply-reviews.py (line 13)


Might be cleaner/safer to use urlparse.urljoin for this:
https://docs.python.org/2/library/urlparse.html#urlparse.urljoin



support/apply-reviews.py (line 15)


Docstring-ify.

Looks like this method is just json-ifying an HTTP GET from the url.  
(Reword comment?)



support/apply-reviews.py (line 20)


Docstring-ify.



support/apply-reviews.py (lines 26 - 28)


Docstring-ify.

Is the list of reviews in reverse order of the chain?



support/apply-reviews.py (line 29)


I believe the python convention for (not-really) "private" helper methods 
is a leading underscore.

i.e. `def _parent_review(...)`



support/apply-reviews.py (line 40)


Extra newline here.



support/apply-reviews.py (lines 44 - 45)


Docstring-ify.



support/apply-reviews.py (line 46)


Seems like you could combine this into `parent_review_ex` by changing the 
function prototype to:

`parent_review(id, list=[])`



support/apply-reviews.py (line 49)


Docstring-ify.



support/apply-reviews.py (line 55)


Docstring-ify.



support/apply-reviews.py (line 56)


It would be great if this included a snippet of the review's summary.



support/apply-reviews.py (line 68)


Seems like you don't need a `-r` in this case, because we'd only use this 
script for reviewboard.

i.e. s/-r/review/



support/apply-reviews.py (line 72)


Add `'--dry-run'` to this option?


- Joseph Wu


On Sept. 23, 2015, 9:26 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Sept. 23, 2015, 9:26 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38646: Added WIP note for Executor endpoint in changelog

[Anand Mazumdar]

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

(Updated Sept. 25, 2015, 12:09 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebased


Repository: mesos


Description
---

WIP Note for the 0.25 release that `api/v1/executor` endpoint is incomplete.


Diffs (updated)
-

  CHANGELOG 7a0c813847590e8e384054551381e4f13f5c67d2 

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


Testing
---


Thanks,

Anand Mazumdar

Re: Review Request 38731: Make common resources symmetrical to v1 resources.

[Joris Van Remoortere]

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

(Updated Sept. 25, 2015, 12:07 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

This aids in verifying the files are kept in sync.
diff src/common/resources.cpp src/v1/resources.cpp should result
in only include and namespace differences.


Diffs (updated)
-

  src/common/resources.cpp abfc6f34cd444cab7c95c2706a408ec9d3a66025 

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


Testing
---

diff src/common/resources.cpp src/v1/resources.cpp
```
27,29c27,29
< #include 
< #include 
< #include 
---
> #include 
> #include 
> #include 
43a44
> namespace v1 {
1263a1265
> } // namespace v1 {
```


Thanks,

Joris Van Remoortere

Re: Review Request 38733: Make common type_utils symmetrical to v1 mesos.

[Joris Van Remoortere]

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

(Updated Sept. 25, 2015, 12:06 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

This aids in verifying the files are kept in sync.
diff src/common/type_utils.cpp src/v1/mesos.cpp should result
in only include and namespace differences.

Moved some things out of type_utils.cpp that are not part of the
public API. They were moved into messages.cpp.


Diffs (updated)
-

  include/mesos/type_utils.hpp 6cedf079c710c7d6ab8f95c47c133e6a1efe9a82 
  include/mesos/v1/mesos.hpp 260e1125dea792ebcece404f94363e8c4bc36f28 
  src/Makefile.am 776483bfe54255eb44dffed9fb43f3b26235fb40 
  src/common/type_utils.cpp 5f74daba208bf22f1ab159a8e0c920e14c7a4614 
  src/messages/messages.cpp PRE-CREATION 
  src/v1/mesos.cpp 631d6e5290b27b6fcb8cbe56378dad620e55fbbf 

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


Testing (updated)
---

diff src/common/type_utils.cpp src/v1/mesos.cpp
```
19,24c19,21
< #include 
< #include 
< #include 
< #include 
< 
< #include "messages/messages.hpp"
---
> #include 
> #include 
> #include 
26a24
> namespace v1 {
331c329
< bool operator==(const SlaveInfo& left, const SlaveInfo& right)
---
> bool operator==(const AgentInfo& left, const AgentInfo& right)
337d334
< left.checkpoint() == right.checkpoint() &&
349c346
< left.slave_id() == right.slave_id() &&
---
> left.agent_id() == right.agent_id() &&
363a361,362
> 
> } // namespace v1 {
```


Thanks,

Joris Van Remoortere

Re: Review Request 38738: Make common public header type_utils symmetrical to v1 mesos.

[Benjamin Hindman]

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

Ship it!


Let's actually keep the comment (but change the wording as necessary), that was 
my bad. Also, let's just move out ACLs from v1, as well as the modules stuff 
for now. Thanks!

- Benjamin Hindman


On Sept. 24, 2015, 10:39 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38738/
> ---
> 
> (Updated Sept. 24, 2015, 10:39 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-3510
> https://issues.apache.org/jira/browse/MESOS-3510
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This aids in verifying the files are kept in sync.
> diff include/mesos/type_utils.cpp include/mesos/v1/mesos.cpp should
> result in only include and namespace differences.
> 
> Also included Modules as part of the V1 API to aid in symmetry.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/module.proto 821fc0e72ece7c497595859fc5efc1c64ea49b9b 
>   include/mesos/type_utils.hpp 6cedf079c710c7d6ab8f95c47c133e6a1efe9a82 
>   include/mesos/v1/mesos.hpp 260e1125dea792ebcece404f94363e8c4bc36f28 
>   include/mesos/v1/module/module.hpp PRE-CREATION 
>   include/mesos/v1/module/module.proto PRE-CREATION 
>   src/Makefile.am 776483bfe54255eb44dffed9fb43f3b26235fb40 
> 
> Diff: https://reviews.apache.org/r/38738/diff/
> 
> 
> Testing
> ---
> 
> Most notable is the ACL streamer which I wasn't sure if we purposely moved 
> into mesos as opposed to keeping the authorization directory in the previous 
> api.
> diff include/mesos/type_utils.hpp include/mesos/v1/mesos.hpp
> ```
> 19,20c19,20
> < #ifndef __MESOS_TYPE_UTILS_H__
> < #define __MESOS_TYPE_UTILS_H__
> ---
> > #ifndef __MESOS_V1_HPP__
> > #define __MESOS_V1_HPP__
> 26c26
> < #include 
> ---
> > #include  // ONLY USEFUL AFTER RUNNING PROTOC.
> 28c28
> < #include 
> ---
> > #include 
> 30c30
> < #include 
> ---
> > #include 
> 32,33d31
> < #include 
> < #include 
> 35,46d32
> < #include 
> < 
> < // This file includes definitions for operators on public protobuf
> < // classes (defined in mesos.proto, module.proto, etc.) that don't
> < // have these operators generated by the protobuf compiler. The
> < // corresponding definitions are in src/common/type_utils.cpp.
> < //
> < // Mesos modules need some of the protobuf classes defined in
> < // mesos.proto, module.proto, etc., and require some of these
> < // operators declared in type_utils.hpp. Exposing type_utils.hpp
> < // allows us to build modules without having a dependency on mesos
> < // source tree (src/*).
> 48a35
> > namespace v1 {
> 61c48
> < bool operator==(const SlaveInfo& left, const SlaveInfo& right);
> ---
> > bool operator==(const AgentInfo& left, const AgentInfo& right);
> 104c91
> < inline bool operator==(const SlaveID& left, const SlaveID& right)
> ---
> > inline bool operator==(const AgentID& left, const AgentID& right)
> 152c139
> < inline bool operator==(const SlaveID& left, const std::string& right)
> ---
> > inline bool operator==(const AgentID& left, const std::string& right)
> 197c184
> < inline bool operator!=(const SlaveID& left, const SlaveID& right)
> ---
> > inline bool operator!=(const AgentID& left, const AgentID& right)
> 239c226
> < inline bool operator<(const SlaveID& left, const SlaveID& right)
> ---
> > inline bool operator<(const AgentID& left, const AgentID& right)
> 250a238,243
> > inline std::ostream& operator<<(std::ostream& stream, const ACLs& acls)
> > {
> >   return stream << acls.DebugString();
> > }
> > 
> > 
> 309c302
> < inline std::ostream& operator<<(std::ostream& stream, const SlaveID& 
> slaveId)
> ---
> > inline std::ostream& operator<<(std::ostream& stream, const AgentID& 
> > agentId)
> 311c304
> <   return stream << slaveId.value();
> ---
> >   return stream << agentId.value();
> 315c308
> < inline std::ostream& operator<<(std::ostream& stream, const SlaveInfo& 
> slave)
> ---
> > inline std::ostream& operator<<(std::ostream& stream, const AgentInfo& 
> > agent)
> 317c310
> <   return stream << slave.DebugString();
> ---
> >   return stream << agent.DebugString();
> 422c415
> < const Modules& modules)
> ---
> > const module::Modules& modules)
> 435a429
> > } // namespace v1 {
> 441c435
> < struct hash
> ---
> > struct hash
> 445c439
> <   typedef mesos::CommandInfo_URI argument_type;
> ---
> >   typedef mesos::v1::CommandInfo::URI argument_type;
> 466c460
> < struct hash
> ---
> > struct hash
> 470c464
> <   typedef mesos::ContainerID argument_type;
> ---
> >   typedef mesos::v1::ContainerID argument_type;
> 482c476
> < struct hash
> ---
> > struct hash
> 486c480
> <   typedef mesos::Exec

Re: Review Request 38607: Pulled out an encode function for http::Request encoding.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Sept. 22, 2015, 6:18 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38607/
> ---
> 
> (Updated Sept. 22, 2015, 6:18 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now we have all the information within Request necessary to encode it, so 
> this extracts encoding into a function to clean up the code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
> 
> Diff: https://reviews.apache.org/r/38607/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 38734: Cleaned up function signatures to use Option.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38734]

All tests passed.

- Mesos ReviewBot


On Sept. 24, 2015, 9:27 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38734/
> ---
> 
> (Updated Sept. 24, 2015, 9:27 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also make use of delegating constructors to reduce redundancy in the master
> detector and master contender.
> 
> 
> Diffs
> -
> 
>   src/master/contender.hpp 927601c73f3ba56cafb366502a5f58a71c581837 
>   src/master/contender.cpp 67562f19754bd57e293b62d0b7547ff2e6cf18ad 
>   src/master/detector.hpp d31ce532e0b26569e521b21893ef91d771fc20bc 
>   src/master/detector.cpp d0d10e58cc32f5ba9da1bd35a66dcbf5c204 
>   src/master/main.cpp bafc605d6c20bd264b932e44ee80373a3f692734 
> 
> Diff: https://reviews.apache.org/r/38734/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38645: Fixed Flaky Executor HTTP tests

[Anand Mazumdar]

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

(Updated Sept. 24, 2015, 11:48 p.m.)


Review request for mesos, Isabel Jimenez and Vinod Kone.


Changes
---

rebased


Repository: mesos


Description
---

This showed up on ASF CI. From the logs:

`I0922 17:31:49.819221 28463 slave.cpp:4104] Finished recovery”`
Then 
`../../src/tests/executor_http_api_tests.cpp:290: Failure`
`Failed to wait 15secs for __recover`

Instead of doing a `FUTURE_DISPATCH` after `StartSlave()` we should be doing it 
before starting the slave. In some cases, slave would have already recovered by 
the time we invoke `FUTURE_DISPATCH` leading to the flakiness.


Diffs (updated)
-

  src/tests/executor_http_api_tests.cpp 
9dbc5191b5950df2faa693720f3740e97c7df758 

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


Testing
---

Introduced a sleep before `AWAIT_READY`, the test failed. After this change 
with the sleep it still passed.

Ran in a loop 100 times.

ASF CI error log: 
https://builds.apache.org/job/Mesos/COMPILER=gcc,CONFIGURATION=--verbose,OS=ubuntu%3A14.04,label_exp=docker%7C%7CHadoop/839/changes


Thanks,

Anand Mazumdar

Re: Review Request 38645: Fixed Flaky Executor HTTP tests

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Sept. 24, 2015, 11:24 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38645/
> ---
> 
> (Updated Sept. 24, 2015, 11:24 p.m.)
> 
> 
> Review request for mesos, Isabel Jimenez and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This showed up on ASF CI. From the logs:
> 
> `I0922 17:31:49.819221 28463 slave.cpp:4104] Finished recovery”`
> Then 
> `../../src/tests/executor_http_api_tests.cpp:290: Failure`
> `Failed to wait 15secs for __recover`
> 
> Instead of doing a `FUTURE_DISPATCH` after `StartSlave()` we should be doing 
> it before starting the slave. In some cases, slave would have already 
> recovered by the time we invoke `FUTURE_DISPATCH` leading to the flakiness.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp 
> 9dbc5191b5950df2faa693720f3740e97c7df758 
> 
> Diff: https://reviews.apache.org/r/38645/diff/
> 
> 
> Testing
> ---
> 
> Introduced a sleep before `AWAIT_READY`, the test failed. After this change 
> with the sleep it still passed.
> 
> Ran in a loop 100 times.
> 
> ASF CI error log: 
> https://builds.apache.org/job/Mesos/COMPILER=gcc,CONFIGURATION=--verbose,OS=ubuntu%3A14.04,label_exp=docker%7C%7CHadoop/839/changes
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 38743: files: fix potential fd leaks under error conditions.

[Chi Zhang]

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

Review request for mesos.


Repository: mesos


Description
---

files: fix potential fd leaks under error conditions.


Diffs
-

  src/files/files.cpp 1d4b1e6d2fcd99321084515f538aa357f8d9b305 

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


Testing
---

make check


Thanks,

Chi Zhang

Re: Review Request 38726: Make common attributes symmetrical to v1 attributes.

[Benjamin Hindman]

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

Ship it!


Ship It!

- Benjamin Hindman


On Sept. 24, 2015, 6:30 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38726/
> ---
> 
> (Updated Sept. 24, 2015, 6:30 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-3510
> https://issues.apache.org/jira/browse/MESOS-3510
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This aids in verifying the files are kept in sync.
> diff src/common/attributes.cpp src/v1/attributes.cpp should result
> in only include and namespace differences.
> 
> 
> Diffs
> -
> 
>   src/common/attributes.cpp 8a8d6245135aff0a6cfde8dee46640770c38ed0d 
> 
> Diff: https://reviews.apache.org/r/38726/diff/
> 
> 
> Testing
> ---
> 
> diff src/common/attributes.cpp src/v1/attributes.cpp 
> ```
> 24,25c24,25
> < #include 
> < #include 
> ---
> > #include 
> > #include 
> 34d33
> < 
> 35a35
> > namespace v1 {
> 227a228
> > } // namespace v1 {
> ```
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38731: Make common resources symmetrical to v1 resources.

[Benjamin Hindman]

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

Ship it!


Ship It!

- Benjamin Hindman


On Sept. 24, 2015, 8:20 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38731/
> ---
> 
> (Updated Sept. 24, 2015, 8:20 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-3510
> https://issues.apache.org/jira/browse/MESOS-3510
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This aids in verifying the files are kept in sync.
> diff src/common/resources.cpp src/v1/resources.cpp should result
> in only include and namespace differences.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp abfc6f34cd444cab7c95c2706a408ec9d3a66025 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/38731/diff/
> 
> 
> Testing
> ---
> 
> diff src/common/resources.cpp src/v1/resources.cpp
> ```
> 27,29c27,29
> < #include 
> < #include 
> < #include 
> ---
> > #include 
> > #include 
> > #include 
> 43a44
> > namespace v1 {
> 1263a1265
> > } // namespace v1 {
> ```
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38733: Make common type_utils symmetrical to v1 mesos.

[Benjamin Hindman]

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

Ship it!


But let's keep the 'checkpoint' stuff in type_utils.cpp for now just to be 
conservative.

- Benjamin Hindman


On Sept. 24, 2015, 9:24 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38733/
> ---
> 
> (Updated Sept. 24, 2015, 9:24 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-3510
> https://issues.apache.org/jira/browse/MESOS-3510
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This aids in verifying the files are kept in sync.
> diff src/common/type_utils.cpp src/v1/mesos.cpp should result
> in only include and namespace differences.
> 
> Moved some things out of type_utils.cpp that are not part of the
> public API. They were moved into messages.cpp.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp 6cedf079c710c7d6ab8f95c47c133e6a1efe9a82 
>   include/mesos/v1/mesos.hpp 260e1125dea792ebcece404f94363e8c4bc36f28 
>   src/Makefile.am 776483bfe54255eb44dffed9fb43f3b26235fb40 
>   src/common/type_utils.cpp 5f74daba208bf22f1ab159a8e0c920e14c7a4614 
>   src/messages/messages.cpp PRE-CREATION 
>   src/v1/mesos.cpp 631d6e5290b27b6fcb8cbe56378dad620e55fbbf 
> 
> Diff: https://reviews.apache.org/r/38733/diff/
> 
> 
> Testing
> ---
> 
> diff src/common/type_utils.cpp src/v1/mesos.cpp
> ```
> 19,24c19,21
> < #include 
> < #include 
> < #include 
> < #include 
> < 
> < #include "messages/messages.hpp"
> ---
> > #include 
> > #include 
> > #include 
> 26a24
> > namespace v1 {
> 331c329
> < bool operator==(const SlaveInfo& left, const SlaveInfo& right)
> ---
> > bool operator==(const AgentInfo& left, const AgentInfo& right)
> 348c346
> < left.slave_id() == right.slave_id() &&
> ---
> > left.agent_id() == right.agent_id() &&
> 362a361,362
> > 
> > } // namespace v1 {
> ```
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38727: Make common values symmetrical to v1 values.

[Benjamin Hindman]

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

Ship it!


Ship It!

- Benjamin Hindman


On Sept. 24, 2015, 6:53 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38727/
> ---
> 
> (Updated Sept. 24, 2015, 6:53 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-3510
> https://issues.apache.org/jira/browse/MESOS-3510
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This aids in verifying the files are kept in sync.
> diff src/common/values.cpp src/v1/values.cpp should result
> in only include and namespace differences.
> 
> 
> Diffs
> -
> 
>   src/v1/values.cpp aaa9a7e19e70634890ff38aefa9817ed68682697 
> 
> Diff: https://reviews.apache.org/r/38727/diff/
> 
> 
> Testing
> ---
> 
> diff src/common/values.cpp src/v1/values.cpp
> ```
> 33,34c33,34
> < #include 
> < #include 
> ---
> > #include 
> > #include 
> 46a47
> > namespace v1 {
> 95d95
> < 
> 638a639
> > } // namespace v1 {
> ```
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38338: Enhanced option for Docker cli volume plugin.

[Vaibhav Khanduja]

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



src/docker/docker.cpp (line 422)


volumn - "volume"


- Vaibhav Khanduja


On Sept. 13, 2015, 5:34 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38338/
> ---
> 
> (Updated Sept. 13, 2015, 5:34 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3392
> https://issues.apache.org/jira/browse/MESOS-3392
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhanced option for Docker cli volume plugin.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   src/docker/docker.cpp 553e831029454d6d423842915b8bbfcaf19fa7f9 
> 
> Diff: https://reviews.apache.org/r/38338/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38645: Fixed Flaky Executor HTTP tests

[Anand Mazumdar]

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

(Updated Sept. 24, 2015, 11:24 p.m.)


Review request for mesos, Isabel Jimenez and Vinod Kone.


Changes
---

Review comments to bring pause/settle calls one after the other.


Repository: mesos


Description
---

This showed up on ASF CI. From the logs:

`I0922 17:31:49.819221 28463 slave.cpp:4104] Finished recovery”`
Then 
`../../src/tests/executor_http_api_tests.cpp:290: Failure`
`Failed to wait 15secs for __recover`

Instead of doing a `FUTURE_DISPATCH` after `StartSlave()` we should be doing it 
before starting the slave. In some cases, slave would have already recovered by 
the time we invoke `FUTURE_DISPATCH` leading to the flakiness.


Diffs (updated)
-

  src/tests/executor_http_api_tests.cpp 
9dbc5191b5950df2faa693720f3740e97c7df758 

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


Testing
---

Introduced a sleep before `AWAIT_READY`, the test failed. After this change 
with the sleep it still passed.

Ran in a loop 100 times.

ASF CI error log: 
https://builds.apache.org/job/Mesos/COMPILER=gcc,CONFIGURATION=--verbose,OS=ubuntu%3A14.04,label_exp=docker%7C%7CHadoop/839/changes


Thanks,

Anand Mazumdar

Re: Review Request 38646: Added WIP note for Executor endpoint in changelog

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Sept. 22, 2015, 9:18 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38646/
> ---
> 
> (Updated Sept. 22, 2015, 9:18 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Note for the 0.25 release that `api/v1/executor` endpoint is incomplete.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 885ab46ce90b60ae51c5ffbd3578c848c73321a7 
> 
> Diff: https://reviews.apache.org/r/38646/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 38645: Fixed Flaky Executor HTTP tests

[Anand Mazumdar]

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

(Updated Sept. 24, 2015, 11:09 p.m.)


Review request for mesos, Isabel Jimenez and Vinod Kone.


Changes
---

Address comments from Vinod


Repository: mesos


Description
---

This showed up on ASF CI. From the logs:

`I0922 17:31:49.819221 28463 slave.cpp:4104] Finished recovery”`
Then 
`../../src/tests/executor_http_api_tests.cpp:290: Failure`
`Failed to wait 15secs for __recover`

Instead of doing a `FUTURE_DISPATCH` after `StartSlave()` we should be doing it 
before starting the slave. In some cases, slave would have already recovered by 
the time we invoke `FUTURE_DISPATCH` leading to the flakiness.


Diffs (updated)
-

  src/tests/executor_http_api_tests.cpp 
9dbc5191b5950df2faa693720f3740e97c7df758 

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


Testing
---

Introduced a sleep before `AWAIT_READY`, the test failed. After this change 
with the sleep it still passed.

Ran in a loop 100 times.

ASF CI error log: 
https://builds.apache.org/job/Mesos/COMPILER=gcc,CONFIGURATION=--verbose,OS=ubuntu%3A14.04,label_exp=docker%7C%7CHadoop/839/changes


Thanks,

Anand Mazumdar

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Timothy Chen]

> On Sept. 24, 2015, 9 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/provisioner/docker/local_puller.hpp, line 40
> > 
> >
> > s/docker_discovery_local_dir/docker_local_archives_dir/
> > 
> > I suggested the flag name based on the suggestion about ArchivesPuller, 
> > I see that you are keeping the name LocalPuller (because we are not 
> > supporting remote archives right now) but changed the flag.
> > 
> > Maybe s/LocalPuller/LocalArchivesPuller/ below for now to be 
> > consistent? 
> > 
> > We can revisit other archives locations after 
> > https://issues.apache.org/jira/browse/MESOS-3511.

Hmm ok I'll rename to LocalArchivesPuller


- Timothy


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


On Sept. 24, 2015, 8:02 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38137/
> ---
> 
> (Updated Sept. 24, 2015, 8:02 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang 
> Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Joining all the commits around provisioner and local store into one review so 
> it's easier to review, as patches
> are changing code on top of each other.
> 
> All the commits are going to committed together.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 776483b 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.proto PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp 35d1199 
>   src/slave/flags.hpp 3f6601a 
>   src/slave/flags.cpp 8792162 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1b0c304 
> 
> Diff: https://reviews.apache.org/r/38137/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38137]

All tests passed.

- Mesos ReviewBot


On Sept. 24, 2015, 8:02 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38137/
> ---
> 
> (Updated Sept. 24, 2015, 8:02 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang 
> Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Joining all the commits around provisioner and local store into one review so 
> it's easier to review, as patches
> are changing code on top of each other.
> 
> All the commits are going to committed together.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 776483b 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.proto PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp 35d1199 
>   src/slave/flags.hpp 3f6601a 
>   src/slave/flags.cpp 8792162 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1b0c304 
> 
> Diff: https://reviews.apache.org/r/38137/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Review Request 38738: Make common public header type_utils symmetrical to v1 mesos.

[Joris Van Remoortere]

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

Review request for mesos, Benjamin Hindman and Ben Mahler.


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


Repository: mesos


Description
---

This aids in verifying the files are kept in sync.
diff include/mesos/type_utils.cpp include/mesos/v1/mesos.cpp should
result in only include and namespace differences.

Also included Modules as part of the V1 API to aid in symmetry.


Diffs
-

  include/mesos/module/module.proto 821fc0e72ece7c497595859fc5efc1c64ea49b9b 
  include/mesos/type_utils.hpp 6cedf079c710c7d6ab8f95c47c133e6a1efe9a82 
  include/mesos/v1/mesos.hpp 260e1125dea792ebcece404f94363e8c4bc36f28 
  include/mesos/v1/module/module.hpp PRE-CREATION 
  include/mesos/v1/module/module.proto PRE-CREATION 
  src/Makefile.am 776483bfe54255eb44dffed9fb43f3b26235fb40 

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


Testing
---

Most notable is the ACL streamer which I wasn't sure if we purposely moved into 
mesos as opposed to keeping the authorization directory in the previous api.
diff include/mesos/type_utils.hpp include/mesos/v1/mesos.hpp
```
19,20c19,20
< #ifndef __MESOS_TYPE_UTILS_H__
< #define __MESOS_TYPE_UTILS_H__
---
> #ifndef __MESOS_V1_HPP__
> #define __MESOS_V1_HPP__
26c26
< #include 
---
> #include  // ONLY USEFUL AFTER RUNNING PROTOC.
28c28
< #include 
---
> #include 
30c30
< #include 
---
> #include 
32,33d31
< #include 
< #include 
35,46d32
< #include 
< 
< // This file includes definitions for operators on public protobuf
< // classes (defined in mesos.proto, module.proto, etc.) that don't
< // have these operators generated by the protobuf compiler. The
< // corresponding definitions are in src/common/type_utils.cpp.
< //
< // Mesos modules need some of the protobuf classes defined in
< // mesos.proto, module.proto, etc., and require some of these
< // operators declared in type_utils.hpp. Exposing type_utils.hpp
< // allows us to build modules without having a dependency on mesos
< // source tree (src/*).
48a35
> namespace v1 {
61c48
< bool operator==(const SlaveInfo& left, const SlaveInfo& right);
---
> bool operator==(const AgentInfo& left, const AgentInfo& right);
104c91
< inline bool operator==(const SlaveID& left, const SlaveID& right)
---
> inline bool operator==(const AgentID& left, const AgentID& right)
152c139
< inline bool operator==(const SlaveID& left, const std::string& right)
---
> inline bool operator==(const AgentID& left, const std::string& right)
197c184
< inline bool operator!=(const SlaveID& left, const SlaveID& right)
---
> inline bool operator!=(const AgentID& left, const AgentID& right)
239c226
< inline bool operator<(const SlaveID& left, const SlaveID& right)
---
> inline bool operator<(const AgentID& left, const AgentID& right)
250a238,243
> inline std::ostream& operator<<(std::ostream& stream, const ACLs& acls)
> {
>   return stream << acls.DebugString();
> }
> 
> 
309c302
< inline std::ostream& operator<<(std::ostream& stream, const SlaveID& slaveId)
---
> inline std::ostream& operator<<(std::ostream& stream, const AgentID& agentId)
311c304
<   return stream << slaveId.value();
---
>   return stream << agentId.value();
315c308
< inline std::ostream& operator<<(std::ostream& stream, const SlaveInfo& slave)
---
> inline std::ostream& operator<<(std::ostream& stream, const AgentInfo& agent)
317c310
<   return stream << slave.DebugString();
---
>   return stream << agent.DebugString();
422c415
< const Modules& modules)
---
> const module::Modules& modules)
435a429
> } // namespace v1 {
441c435
< struct hash
---
> struct hash
445c439
<   typedef mesos::CommandInfo_URI argument_type;
---
>   typedef mesos::v1::CommandInfo::URI argument_type;
466c460
< struct hash
---
> struct hash
470c464
<   typedef mesos::ContainerID argument_type;
---
>   typedef mesos::v1::ContainerID argument_type;
482c476
< struct hash
---
> struct hash
486c480
<   typedef mesos::ExecutorID argument_type;
---
>   typedef mesos::v1::ExecutorID argument_type;
498c492
< struct hash
---
> struct hash
502c496
<   typedef mesos::FrameworkID argument_type;
---
>   typedef mesos::v1::FrameworkID argument_type;
514c508
< struct hash
---
> struct hash
518c512
<   typedef mesos::OfferID argument_type;
---
>   typedef mesos::v1::OfferID argument_type;
530c524
< struct hash
---
> struct hash
534c528
<   typedef mesos::SlaveID argument_type;
---
>   typedef mesos::v1::AgentID argument_type;
536c530
<   result_type operator()(const argument_type& slaveId) const
---
>   result_type operator()(const argument_type& agentId) const
539c533
< boost::hash_combine(seed, slaveId.value());
---
> boost::hash_combine(seed, agentId.value());
546c540
< struct hash
---
> struct hash
550c544
<   typedef mesos::TaskID argument_type;
---
>   typedef mesos::v1::

Re: Review Request 38731: Make common resources symmetrical to v1 resources.

[Ben Mahler]

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

Ship it!


Ship It!

- Ben Mahler


On Sept. 24, 2015, 8:20 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38731/
> ---
> 
> (Updated Sept. 24, 2015, 8:20 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-3510
> https://issues.apache.org/jira/browse/MESOS-3510
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This aids in verifying the files are kept in sync.
> diff src/common/resources.cpp src/v1/resources.cpp should result
> in only include and namespace differences.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp abfc6f34cd444cab7c95c2706a408ec9d3a66025 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/38731/diff/
> 
> 
> Testing
> ---
> 
> diff src/common/resources.cpp src/v1/resources.cpp
> ```
> 27,29c27,29
> < #include 
> < #include 
> < #include 
> ---
> > #include 
> > #include 
> > #include 
> 43a44
> > namespace v1 {
> 1263a1265
> > } // namespace v1 {
> ```
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38726: Make common attributes symmetrical to v1 attributes.

[Ben Mahler]

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

Ship it!


Ship It!

- Ben Mahler


On Sept. 24, 2015, 6:30 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38726/
> ---
> 
> (Updated Sept. 24, 2015, 6:30 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-3510
> https://issues.apache.org/jira/browse/MESOS-3510
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This aids in verifying the files are kept in sync.
> diff src/common/attributes.cpp src/v1/attributes.cpp should result
> in only include and namespace differences.
> 
> 
> Diffs
> -
> 
>   src/common/attributes.cpp 8a8d6245135aff0a6cfde8dee46640770c38ed0d 
> 
> Diff: https://reviews.apache.org/r/38726/diff/
> 
> 
> Testing
> ---
> 
> diff src/common/attributes.cpp src/v1/attributes.cpp 
> ```
> 24,25c24,25
> < #include 
> < #include 
> ---
> > #include 
> > #include 
> 34d33
> < 
> 35a35
> > namespace v1 {
> 227a228
> > } // namespace v1 {
> ```
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38729: Do not retry close on EINTR.

[Jie Yu]

> On Sept. 24, 2015, 8:55 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/src/subprocess.cpp, line 133
> > 
> >
> > Should we wrap it with #ifdef linux?
> 
> Ben Mahler wrote:
> Sadly, it is more involved than just an ifdef :(
> 
> For example, on OS X: 
> https://code.google.com/p/chromium/issues/detail?id=269623 and the change 
> here https://codereview.chromium.org/23455051/
> On Linux the no retry logic is correct, although it seems the glibc 
> wrapper makes it cancelable: 
> https://code.google.com/p/chromium/issues/detail?id=269623#c13

ok, looks like OSX is more complicated:(


- Jie


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


On Sept. 24, 2015, 7:41 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38729/
> ---
> 
> (Updated Sept. 24, 2015, 7:41 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3512
> https://issues.apache.org/jira/browse/MESOS-3512
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See: https://issues.apache.org/jira/browse/MESOS-3512
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> d6ea62ed1c914d34e0e189395831c86fff8aac22 
> 
> Diff: https://reviews.apache.org/r/38729/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 38645: Fixed Flaky Executor HTTP tests

[Vinod Kone]

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



src/tests/executor_http_api_tests.cpp (line 95)


You should do a Clock::Settle() here (and for that to work pause the clock 
before) because AFAICT AWAIT_READY(__recover) doesn't guarantee that 
Slave::__recover() has been executed. It only tells us that the event is about 
to be processed. See process::resume().


- Vinod Kone


On Sept. 22, 2015, 9:37 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38645/
> ---
> 
> (Updated Sept. 22, 2015, 9:37 p.m.)
> 
> 
> Review request for mesos, Isabel Jimenez and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This showed up on ASF CI. From the logs:
> 
> `I0922 17:31:49.819221 28463 slave.cpp:4104] Finished recovery”`
> Then 
> `../../src/tests/executor_http_api_tests.cpp:290: Failure`
> `Failed to wait 15secs for __recover`
> 
> Instead of doing a `FUTURE_DISPATCH` after `StartSlave()` we should be doing 
> it before starting the slave. In some cases, slave would have already 
> recovered by the time we invoke `FUTURE_DISPATCH` leading to the flakiness.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp 
> 9dbc5191b5950df2faa693720f3740e97c7df758 
> 
> Diff: https://reviews.apache.org/r/38645/diff/
> 
> 
> Testing
> ---
> 
> Introduced a sleep before `AWAIT_READY`, the test failed. After this change 
> with the sleep it still passed.
> 
> Ran in a loop 100 times.
> 
> ASF CI error log: 
> https://builds.apache.org/job/Mesos/COMPILER=gcc,CONFIGURATION=--verbose,OS=ubuntu%3A14.04,label_exp=docker%7C%7CHadoop/839/changes
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 38729: Do not retry close on EINTR.

[Ben Mahler]

> On Sept. 24, 2015, 8:55 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/src/subprocess.cpp, line 133
> > 
> >
> > Should we wrap it with #ifdef linux?

Sadly, it is more involved than just an ifdef :(

For example, on OS X: 
https://code.google.com/p/chromium/issues/detail?id=269623 and the change here 
https://codereview.chromium.org/23455051/
On Linux the no retry logic is correct, although it seems the glibc wrapper 
makes it cancelable: 
https://code.google.com/p/chromium/issues/detail?id=269623#c13


- Ben


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


On Sept. 24, 2015, 7:41 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38729/
> ---
> 
> (Updated Sept. 24, 2015, 7:41 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3512
> https://issues.apache.org/jira/browse/MESOS-3512
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See: https://issues.apache.org/jira/browse/MESOS-3512
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> d6ea62ed1c914d34e0e189395831c86fff8aac22 
> 
> Diff: https://reviews.apache.org/r/38729/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 38731: Make common resources symmetrical to v1 resources.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38726, 38727, 38731]

All tests passed.

- Mesos ReviewBot


On Sept. 24, 2015, 8:20 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38731/
> ---
> 
> (Updated Sept. 24, 2015, 8:20 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-3510
> https://issues.apache.org/jira/browse/MESOS-3510
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This aids in verifying the files are kept in sync.
> diff src/common/resources.cpp src/v1/resources.cpp should result
> in only include and namespace differences.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp abfc6f34cd444cab7c95c2706a408ec9d3a66025 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/38731/diff/
> 
> 
> Testing
> ---
> 
> diff src/common/resources.cpp src/v1/resources.cpp
> ```
> 27,29c27,29
> < #include 
> < #include 
> < #include 
> ---
> > #include 
> > #include 
> > #include 
> 43a44
> > namespace v1 {
> 1263a1265
> > } // namespace v1 {
> ```
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38729: Do not retry close on EINTR.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Sept. 24, 2015, 7:41 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38729/
> ---
> 
> (Updated Sept. 24, 2015, 7:41 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3512
> https://issues.apache.org/jira/browse/MESOS-3512
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See: https://issues.apache.org/jira/browse/MESOS-3512
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> d6ea62ed1c914d34e0e189395831c86fff8aac22 
> 
> Diff: https://reviews.apache.org/r/38729/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 38730: Do not retry close on EINTR.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Sept. 24, 2015, 7:41 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38730/
> ---
> 
> (Updated Sept. 24, 2015, 7:41 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3512
> https://issues.apache.org/jira/browse/MESOS-3512
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See: https://issues.apache.org/jira/browse/MESOS-3512
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> 6e266bd567d7182c66038822b2d77922d12fe8a1 
>   src/tests/containerizer/isolator_tests.cpp 
> a25ae97a519feb8ead6177da160df8a276ca15bf 
> 
> Diff: https://reviews.apache.org/r/38730/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 37903: stout: Fix bug in IPNetwork::create() with zero prefix.

[Neil Conway]

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

(Updated Sept. 24, 2015, 9:29 p.m.)


Review request for mesos.


Changes
---

Rebase.


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


Repository: mesos


Description
---

The previous coding would try to shift a uint32_t value 32 bits to the left; per
C++ spec, this yields undefined behavior. On my machine, this resulted in
treating a prefix of 0 as equivalent to a prefix of 32, which is obviously
wrong.

Spotted via ubsan: see MESOS-3328.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
1ad119d54820e97497b1773518875be25ddbf98a 
  3rdparty/libprocess/3rdparty/stout/tests/ip_tests.cpp 
b0cbcb38cfcb923ec7c185bacf139ceb0a28924f 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 38468: docs: Added discussion of finding a shepherd.

[Neil Conway]

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

(Updated Sept. 24, 2015, 9:29 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.


Changes
---

Rebase.


Repository: mesos


Description
---

Added suggestion that new contributors seek to find a shepherd for their work
before their begin writing code. Made a few other improvements.


Diffs (updated)
-

  docs/reporting-a-bug.md 6c7f74719a8586f0608eb0f0f77d15c8d534321d 
  docs/submitting-a-patch.md 754a16f9b43630880f0f6c4a8e8e2f5e081b0a87 

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


Testing
---


Thanks,

Neil Conway

Review Request 38734: Cleaned up function signatures to use Option.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Also make use of delegating constructors to reduce redundancy in the master
detector and master contender.


Diffs
-

  src/master/contender.hpp 927601c73f3ba56cafb366502a5f58a71c581837 
  src/master/contender.cpp 67562f19754bd57e293b62d0b7547ff2e6cf18ad 
  src/master/detector.hpp d31ce532e0b26569e521b21893ef91d771fc20bc 
  src/master/detector.cpp d0d10e58cc32f5ba9da1bd35a66dcbf5c204 
  src/master/main.cpp bafc605d6c20bd264b932e44ee80373a3f692734 

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


Testing
---

make check


Thanks,

Neil Conway

Review Request 38733: Make common type_utils symmetrical to v1 mesos.

[Joris Van Remoortere]

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

Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

This aids in verifying the files are kept in sync.
diff src/common/type_utils.cpp src/v1/mesos.cpp should result
in only include and namespace differences.

Moved some things out of type_utils.cpp that are not part of the
public API. They were moved into messages.cpp.


Diffs
-

  include/mesos/type_utils.hpp 6cedf079c710c7d6ab8f95c47c133e6a1efe9a82 
  include/mesos/v1/mesos.hpp 260e1125dea792ebcece404f94363e8c4bc36f28 
  src/Makefile.am 776483bfe54255eb44dffed9fb43f3b26235fb40 
  src/common/type_utils.cpp 5f74daba208bf22f1ab159a8e0c920e14c7a4614 
  src/messages/messages.cpp PRE-CREATION 
  src/v1/mesos.cpp 631d6e5290b27b6fcb8cbe56378dad620e55fbbf 

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


Testing
---

diff src/common/type_utils.cpp src/v1/mesos.cpp
```
19,24c19,21
< #include 
< #include 
< #include 
< #include 
< 
< #include "messages/messages.hpp"
---
> #include 
> #include 
> #include 
26a24
> namespace v1 {
331c329
< bool operator==(const SlaveInfo& left, const SlaveInfo& right)
---
> bool operator==(const AgentInfo& left, const AgentInfo& right)
348c346
< left.slave_id() == right.slave_id() &&
---
> left.agent_id() == right.agent_id() &&
362a361,362
> 
> } // namespace v1 {
```


Thanks,

Joris Van Remoortere

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Jiang Yan Xu]

> On Sept. 23, 2015, 12:50 a.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/provisioner/docker/puller.hpp, line 60
> > 
> >
> > If we use LinkedHashMap we don't need to create another struct right?
> 
> Timothy Chen wrote:
> True, but I actually like returning a list due to the fact we already 
> return a list when we collect all the futures, and thought converting it into 
> a LinkedHashMap in the end from a list was a bit unnecessary. What you think?

To me it's a bit contrived to explain this concept. Why is it LayerPath and not 
LayerInfo? Why is `id` a separate part of a "Path"? Why is the field inside be 
named `directory` instead of `path`? It's probably closer to a list of tuples 
of . I personally would spend a few more lines to do the conversion 
(which is an implementation detail) than exposing a concept that bears 
explanation to the caller.

That said, it's not a big deal. Feel free to drop this.


- Jiang Yan


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


On Sept. 24, 2015, 1:02 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38137/
> ---
> 
> (Updated Sept. 24, 2015, 1:02 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang 
> Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Joining all the commits around provisioner and local store into one review so 
> it's easier to review, as patches
> are changing code on top of each other.
> 
> All the commits are going to committed together.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 776483b 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.proto PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp 35d1199 
>   src/slave/flags.hpp 3f6601a 
>   src/slave/flags.cpp 8792162 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1b0c304 
> 
> Diff: https://reviews.apache.org/r/38137/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Jiang Yan Xu]

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

Ship it!


Thanks for being patient. Please commit this!


src/slave/containerizer/provisioner/docker/local_puller.hpp (line 40)


s/docker_discovery_local_dir/docker_local_archives_dir/

I suggested the flag name based on the suggestion about ArchivesPuller, I 
see that you are keeping the name LocalPuller (because we are not supporting 
remote archives right now) but changed the flag.

Maybe s/LocalPuller/LocalArchivesPuller/ below for now to be consistent? 

We can revisit other archives locations after 
https://issues.apache.org/jira/browse/MESOS-3511.



src/slave/containerizer/provisioner/docker/local_puller.hpp (line 41)


s/images saved as tar with the name as the image name with tag/images saved 
as tars with file names in the form of `:.tar`.

We better be consistent with repo vs. name in our code and documentation so 
it's less confusing than docker docs. :)



src/slave/containerizer/provisioner/docker/local_puller.cpp (line 279)


s/directory directory/directory/



src/slave/containerizer/provisioner/docker/paths.hpp (line 47)


Two blank lines here and elsewhere in this file beause they are outside a 
class.



src/slave/containerizer/provisioner/docker/paths.hpp (line 57)


s/staging/archivePath/ or s/staging/archiveDir/

This was suggested in my last review.

I think the easiest way to explain this bunch of methods is that: given an 
`archivePath` which is the base dir of the untarred archive, return me the path 
to the element inside the archive.



src/slave/containerizer/provisioner/docker/puller.hpp (line 42)


s/the directory the puller put its changeset to/the directory where the 
puller puts its changeset/



src/slave/containerizer/provisioner/docker/puller.hpp (line 57)


s/returns/return/ to be consistent with the use of first person verb form 
in this paragraph.



src/slave/containerizer/provisioner/docker/puller.hpp (line 74)


Kill line.


- Jiang Yan Xu


On Sept. 24, 2015, 1:02 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38137/
> ---
> 
> (Updated Sept. 24, 2015, 1:02 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang 
> Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Joining all the commits around provisioner and local store into one review so 
> it's easier to review, as patches
> are changing code on top of each other.
> 
> All the commits are going to committed together.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 776483b 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.proto PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp 35d1199 
>   src/slave/flags.hpp 3f6601a 
>   src/slave/flags.cpp 8792162 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1b0c304 
> 
> Diff: https://reviews.apache.org/r/38137/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38729: Do not retry close on EINTR.

[Jie Yu]

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



3rdparty/libprocess/src/subprocess.cpp (line 133)


Should we wrap it with #ifdef linux?


- Jie Yu


On Sept. 24, 2015, 7:41 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38729/
> ---
> 
> (Updated Sept. 24, 2015, 7:41 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3512
> https://issues.apache.org/jira/browse/MESOS-3512
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See: https://issues.apache.org/jira/browse/MESOS-3512
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> d6ea62ed1c914d34e0e189395831c86fff8aac22 
> 
> Diff: https://reviews.apache.org/r/38729/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 38730: Do not retry close on EINTR.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38729, 38730]

All tests passed.

- Mesos ReviewBot


On Sept. 24, 2015, 7:41 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38730/
> ---
> 
> (Updated Sept. 24, 2015, 7:41 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3512
> https://issues.apache.org/jira/browse/MESOS-3512
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See: https://issues.apache.org/jira/browse/MESOS-3512
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> 6e266bd567d7182c66038822b2d77922d12fe8a1 
>   src/tests/containerizer/isolator_tests.cpp 
> a25ae97a519feb8ead6177da160df8a276ca15bf 
> 
> Diff: https://reviews.apache.org/r/38730/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Review Request 38731: Make common resources symmetrical to v1 resources.

[Joris Van Remoortere]

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

Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

This aids in verifying the files are kept in sync.
diff src/common/resources.cpp src/v1/resources.cpp should result
in only include and namespace differences.


Diffs
-

  src/common/resources.cpp abfc6f34cd444cab7c95c2706a408ec9d3a66025 
  src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 

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


Testing
---

diff src/common/resources.cpp src/v1/resources.cpp
```
27,29c27,29
< #include 
< #include 
< #include 
---
> #include 
> #include 
> #include 
43a44
> namespace v1 {
1263a1265
> } // namespace v1 {
```


Thanks,

Joris Van Remoortere

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Timothy Chen]

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

(Updated Sept. 24, 2015, 8:02 p.m.)


Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang Yan 
Xu.


Repository: mesos


Description
---

Joining all the commits around provisioner and local store into one review so 
it's easier to review, as patches
are changing code on top of each other.

All the commits are going to committed together.


Diffs (updated)
-

  src/Makefile.am 776483b 
  src/slave/containerizer/provisioner/docker/local_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/local_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/message.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/message.proto PRE-CREATION 
  src/slave/containerizer/provisioner/docker/metadata_manager.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/metadata_manager.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/paths.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/paths.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/store.cpp 35d1199 
  src/slave/flags.hpp 3f6601a 
  src/slave/flags.cpp 8792162 
  src/tests/containerizer/provisioner_docker_tests.cpp 1b0c304 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 38727: Make common values symmetrical to v1 values.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38726, 38727]

All tests passed.

- Mesos ReviewBot


On Sept. 24, 2015, 6:53 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38727/
> ---
> 
> (Updated Sept. 24, 2015, 6:53 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-3510
> https://issues.apache.org/jira/browse/MESOS-3510
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This aids in verifying the files are kept in sync.
> diff src/common/values.cpp src/v1/values.cpp should result
> in only include and namespace differences.
> 
> 
> Diffs
> -
> 
>   src/v1/values.cpp aaa9a7e19e70634890ff38aefa9817ed68682697 
> 
> Diff: https://reviews.apache.org/r/38727/diff/
> 
> 
> Testing
> ---
> 
> diff src/common/values.cpp src/v1/values.cpp
> ```
> 33,34c33,34
> < #include 
> < #include 
> ---
> > #include 
> > #include 
> 46a47
> > namespace v1 {
> 95d95
> < 
> 638a639
> > } // namespace v1 {
> ```
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38728: Cgroups Test Filters aborts test.

[Ben Mahler]

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

Ship it!


Ship It!

- Ben Mahler


On Sept. 24, 2015, 7:35 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38728/
> ---
> 
> (Updated Sept. 24, 2015, 7:35 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Ian Downes.
> 
> 
> Bugs: MESOS-3513
> https://issues.apache.org/jira/browse/MESOS-3513
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cgroups Test Filters aborts test.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp 3c586ecd1e8afb80b3f262cb2aa89610141a7a4b 
> 
> Diff: https://reviews.apache.org/r/38728/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Review Request 38729: Do not retry close on EINTR.

[Ben Mahler]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

See: https://issues.apache.org/jira/browse/MESOS-3512


Diffs
-

  3rdparty/libprocess/src/subprocess.cpp 
d6ea62ed1c914d34e0e189395831c86fff8aac22 

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


Testing
---

make check


Thanks,

Ben Mahler

Review Request 38730: Do not retry close on EINTR.

[Ben Mahler]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

See: https://issues.apache.org/jira/browse/MESOS-3512


Diffs
-

  src/slave/containerizer/linux_launcher.cpp 
6e266bd567d7182c66038822b2d77922d12fe8a1 
  src/tests/containerizer/isolator_tests.cpp 
a25ae97a519feb8ead6177da160df8a276ca15bf 

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


Testing
---

make check


Thanks,

Ben Mahler

Review Request 38728: Cgroups Test Filters aborts test.

[Paul Brett]

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

Review request for mesos, Ben Mahler and Ian Downes.


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


Repository: mesos


Description
---

Cgroups Test Filters aborts test.


Diffs
-

  src/tests/environment.cpp 3c586ecd1e8afb80b3f262cb2aa89610141a7a4b 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Jie Yu]

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

Ship it!


LGTM! Let's get this committed!


src/slave/containerizer/provisioner/docker/local_puller.cpp (lines 126 - 127)


This is a little jagged, how about:
```
VLOG(1) << "Untarring image from '" << directory
<< "' to '" << tarPath << "'";
```



src/slave/containerizer/provisioner/docker/local_puller.cpp (line 167)


This is a local helper, right? Please make it a 'static' function.



src/slave/containerizer/provisioner/docker/local_puller.cpp (lines 260 - 269)


This can be done in parallel as well, right?



src/slave/containerizer/provisioner/docker/local_puller.cpp (line 287)


Insert a new line above.



src/slave/containerizer/provisioner/docker/local_puller.cpp (line 299)


Is this necessary?



src/slave/containerizer/provisioner/docker/local_puller.cpp (line 342)


insert a new line above.



src/slave/containerizer/provisioner/docker/metadata_manager.cpp (line 172)


No need for mesos::internal::slave?



src/slave/containerizer/provisioner/docker/metadata_manager.cpp (lines 192 - 
193)


This fits in one line?



src/tests/containerizer/provisioner_docker_tests.cpp (line 700)


2 blank lines above.


- Jie Yu


On Sept. 24, 2015, 9:21 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38137/
> ---
> 
> (Updated Sept. 24, 2015, 9:21 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang 
> Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Joining all the commits around provisioner and local store into one review so 
> it's easier to review, as patches
> are changing code on top of each other.
> 
> All the commits are going to committed together.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 776483b 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.proto PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp 35d1199 
>   src/slave/flags.hpp 3f6601a 
>   src/slave/flags.cpp 69976d7 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1b0c304 
> 
> Diff: https://reviews.apache.org/r/38137/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Jojy Varghese]

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

Ship it!


Ship It!

- Jojy Varghese


On Sept. 24, 2015, 9:21 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38137/
> ---
> 
> (Updated Sept. 24, 2015, 9:21 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang 
> Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Joining all the commits around provisioner and local store into one review so 
> it's easier to review, as patches
> are changing code on top of each other.
> 
> All the commits are going to committed together.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 776483b 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.proto PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp 35d1199 
>   src/slave/flags.hpp 3f6601a 
>   src/slave/flags.cpp 69976d7 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1b0c304 
> 
> Diff: https://reviews.apache.org/r/38137/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Review Request 38727: Make common values symmetrical to v1 values.

[Joris Van Remoortere]

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

Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

This aids in verifying the files are kept in sync.
diff src/common/values.cpp src/v1/values.cpp should result
in only include and namespace differences.


Diffs (updated)
-

  src/v1/values.cpp aaa9a7e19e70634890ff38aefa9817ed68682697 

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


Testing (updated)
---

diff src/common/values.cpp src/v1/values.cpp
```
33,34c33,34
< #include 
< #include 
---
> #include 
> #include 
46a47
> namespace v1 {
95d95
< 
638a639
> } // namespace v1 {
```


Thanks,

Joris Van Remoortere

Re: Review Request 38726: Make common attributes symmetrical to v1 attributes.

[Joris Van Remoortere]

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

(Updated Sept. 24, 2015, 6:30 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

This aids in verifying the files are kept in sync.
diff src/common/attributes.cpp src/v1/attributes.cpp should result
in only include and namespace differences.


Diffs
-

  src/common/attributes.cpp 8a8d6245135aff0a6cfde8dee46640770c38ed0d 

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


Testing
---

diff src/common/attributes.cpp src/v1/attributes.cpp 
```
24,25c24,25
< #include 
< #include 
---
> #include 
> #include 
34d33
< 
35a35
> namespace v1 {
227a228
> } // namespace v1 {
```


Thanks,

Joris Van Remoortere

Review Request 38726: Make common attributes symmetrical to v1 attributes.

[Joris Van Remoortere]

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

Review request for mesos and Benjamin Hindman.


Repository: mesos


Description
---

This aids in verifying the files are kept in sync.
diff src/common/attributes.cpp src/v1/attributes.cpp should result
in only include and namespace differences.


Diffs
-

  src/common/attributes.cpp 8a8d6245135aff0a6cfde8dee46640770c38ed0d 

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


Testing
---

diff src/common/attributes.cpp src/v1/attributes.cpp 
```
24,25c24,25
< #include 
< #include 
---
> #include 
> #include 
34d33
< 
35a35
> namespace v1 {
227a228
> } // namespace v1 {
```


Thanks,

Joris Van Remoortere

Review Request 38725: Fixed a typo for Agent help string for --launcher.

[Kapil Arya]

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

Review request for mesos, Jie Yu and Niklas Nielsen.


Repository: mesos


Description
---

Fixed a typo for Agent help string for --launcher.


Diffs
-

  src/slave/flags.cpp 10f68b80407c95f833de50299a940ed42a56c568 

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 38697: Updated documentation strings for --launcher flag.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Sept. 24, 2015, 6:44 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38697/
> ---
> 
> (Updated Sept. 24, 2015, 6:44 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3508
> https://issues.apache.org/jira/browse/MESOS-3508
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated documentation strings for --launcher flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/slave/flags.cpp 6164b4bae3f1b74da87f01a6db934f265e1a0117 
> 
> Diff: https://reviews.apache.org/r/38697/diff/
> 
> 
> Testing
> ---
> 
> Viewed in help message as well as markdown viewer.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alex Clemmer]

> On Sept. 23, 2015, 5:39 p.m., Alex Clemmer wrote:
> > Can we please add this file to the CMakeLists.txt file in 
> > 3rdparty/libprocess/src as well?
> 
> Alexander Rojas wrote:
> The included file is a header and so far cmake file doesn't seem to care 
> about the include directory (can you correct me if I'm wrong). Still, 
> shouln't the headers be there too? otherwise cmake wouldn't do proper 
> dependency analysis.

Oh, yes, my bad, those changes aren't committed yet. (I have too many reviews 
up and lose track of them all, sorry.)


- Alex


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


On Sept. 23, 2015, 8:57 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Sept. 23, 2015, 8:57 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the authenticator manager, which is a class which handles the 
> actual authentication procedure during the execution of 
> `ProcessManager::handle()` and it also takes care of the life cycle of 
> instances of http::Authenticator.
> 
> No tests are added at this point since no public API is generated, the goal 
> of this patch is to implement the manager and verify nothing breaks 
> afterwards. Authenticator manager tests proper come in a latter patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37996: Added property manager

[Alex Clemmer]

> On Sept. 23, 2015, 5:40 p.m., Alex Clemmer wrote:
> > Can we please add these tests to the CMakeLists.txt file in 
> > 3rdparty/libprocess/3rdparty/stout/tests?
> 
> Alexander Rojas wrote:
> It was done already since the first version of this patch.

Ah. I'm so sorry. I just kind of troll all the reviews looking for changes that 
touch a `Makefile.am` and sometimes I make mistakes like this. :(


- Alex


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


On Sept. 23, 2015, 11:17 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37996/
> ---
> 
> (Updated Sept. 23, 2015, 11:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the `PathInheritedProperties` 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/properties.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> baa648aacfd5204c33e102b58126862729fbb612 
>   3rdparty/libprocess/3rdparty/stout/tests/properties_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37993: Add explanatory comments for Allocator interface

[Alexander Rukletsov]

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


Looking almost perfect! Really appreciate that you diligently address all the 
comments I throw into you and don't give up.

While you're touching this file, I would say it makes sense to clean up 
everything we can in one patch. Here are some suggestions:
- Wrap all types and variables in comments in backticks;
- As I've mentioned in one of my previous reviews, let's use the same tense 
everywhere in the commens for consistency;
- Not a native speaker, but I think we should consolidate the use of articles 
when referencing allocator in comments. I think "an allocator" makes more sense 
since it is explicitly indicates that it's implementation dependent and not 
tied to a particular implementation;
- Again, not a native speaker, but let's refer to a framework as "which" or 
"that", not "who".


include/mesos/master/allocator.hpp (lines 45 - 51)


While you're touching this file, let's doxygenify this one as well.



include/mesos/master/allocator.hpp (line 54)


I think you killed that line in previous versions of the review, why do you 
want to bring it back?



include/mesos/master/allocator.hpp (lines 60 - 61)


Not yours, but let's wrap all types and variable names in backticks "`"



include/mesos/master/allocator.hpp (lines 74 - 75)


Mind switching to Present Simple here for consistency? s/will be/is



include/mesos/master/allocator.hpp (line 75)


I'm not sure `initialize()` can fail: it returns nothing and we do not 
throw exceptions. I see two possibilities here:
- an allocator uses an exception, which is not caught in the master => 
master fails over;
- an allocator uses `CHECK*` macros for error situations => master fails 
over.

Could you please rephrase that in order not to mislead the reader?



include/mesos/master/allocator.hpp (line 78)


s/perform the allocation/perform the batch allocation/

An allocator may also perform allocation based on events (a framework is 
added and so on). However, it's up to the implementation.



include/mesos/master/allocator.hpp (lines 151 - 152)


I'm a bit hesitant to put a reference to the built-in allocator here. The 
behaviour you describe is correct, but 
1) It's how the built-in allocator works
2) It's how the built-in allocator works now: when it the behaviour 
changes, my bet is that this comment won't be updated, hence it will become 
misleading.

What do you think?



include/mesos/master/allocator.hpp (line 159)


s/a/an



include/mesos/master/allocator.hpp (line 161)


s/will be/is



include/mesos/master/allocator.hpp (line 162)


s/new agent joins the Mesos cluster or in the case of a agent recovery./new 
agent joins the cluster or in case of agent recovery.

not a native speaker though



include/mesos/master/allocator.hpp (line 180)


s/a/an

here and below.



include/mesos/master/allocator.hpp (line 183)


s/should/are



include/mesos/master/allocator.hpp (line 191)


s/a/an



include/mesos/master/allocator.hpp (line 193)


What do you mean by "new" here? New feature or new update?



include/mesos/master/allocator.hpp (line 208)


s/a/an

here and below



include/mesos/master/allocator.hpp (line 210)


s/will be/is
s/when re-register a agent/when an agent reregisters



include/mesos/master/allocator.hpp (line 213)


s/going to be/being



include/mesos/master/allocator.hpp (line 219)


s/a/an

here and below



include/mesos/master/allocator.hpp (lines 221 - 223)


How about this:

This is triggered if an agent disconnects from the master. Outstanding 
offers on a deactivated agent are removed and resources are recovered in a 
separate call.



include/mesos/master/allocator.hpp (line 232)

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

[Jan Schlicht]

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



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 609)


s/protobuf(const T& message)/protobuf(const google::protobuf::Message& 
message)

The templatization in 608 isn't necessary here, as the static_assert in 611 
has the same effect as just using a google::protobuf::Message as function 
parameter. Please change the function signature and remove line 608 & 611,612.

Also, it'd make sense to return a `Try` as parsing `message` may 
fail. The current behavior is to call ABORT, one could as well return `None` 
here.



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 615)


This comment describes a behavior of the function and should be moved to 
608 (= above the function signature).



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 772)


Same as before: It'd make sense to return a `Try` here.


- Jan Schlicht


On Sept. 24, 2015, 5 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38342/
> ---
> 
> (Updated Sept. 24, 2015, 5 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 2285ce9 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 68328a2 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 8ebb798 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 34eb6d0 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 920f5c9 
> 
> Diff: https://reviews.apache.org/r/38342/diff/
> 
> 
> Testing
> ---
> 
> cd 3rdparty/libprocess/3rdparty/stout
> ./boostrap
> ./configure
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38335: Add JSON::protobuf for google::protobuf::RepeatedPtrField

[Klaus Ma]

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

(Updated Sept. 24, 2015, 2:43 p.m.)


Review request for mesos and Michael Park.


Changes
---

Resolved code conflict


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


Repository: mesos


Description
---

Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
converts a `google::protobuf::Message` into a `JSON::Object`.
We should add the support for `google::protobuf::RepeatedPtrField` by 
introducing overloaded functions.


Diffs (updated)
-

  src/common/http.cpp 99b843a 
  src/docker/executor.cpp 1e49013 
  src/examples/persistent_volume_framework.cpp 426d338 
  src/launcher/executor.cpp 50b3c6e 
  src/master/contender.cpp 67562f1 
  src/master/http.cpp cd37c91 
  src/master/maintenance.cpp 5fe9358 
  src/master/registrar.cpp d81560a 
  src/slave/containerizer/fetcher.cpp 2b2298c 
  src/slave/containerizer/mesos/containerizer.cpp b904b2d 
  src/slave/monitor.cpp 8d8b422 
  src/tests/containerizer/launch_tests.cpp de655ec 
  src/tests/fault_tolerance_tests.cpp c97bc46 
  src/tests/http_api_tests.cpp 7938bbb 
  src/tests/master_maintenance_tests.cpp 89ad138 
  src/tests/master_tests.cpp f26344d 
  src/tests/mesos.hpp 3e58b45 
  src/tests/mesos.cpp ab2d85b 
  src/tests/monitor_tests.cpp 583e711 
  src/tests/reservation_endpoints_tests.cpp 398a2e1 
  src/tests/resources_tests.cpp 6584fc6 
  src/tests/script.cpp bcc1fab 
  src/tests/slave_tests.cpp dccdbb0 
  src/usage/main.cpp 86fd796 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

[Klaus Ma]

> On Sept. 15, 2015, 12:59 p.m., Jan Schlicht wrote:
> > Please rebase your patch, latest commits in master lots of JSON related 
> > stuff.
> 
> Jan Schlicht wrote:
> Will review this RR after rebase.

The code diff was uploaded after resolving the conflict


- Klaus


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


On Sept. 24, 2015, 2:37 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38342/
> ---
> 
> (Updated Sept. 24, 2015, 2:37 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 2285ce9 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 68328a2 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 8ebb798 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 34eb6d0 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 920f5c9 
> 
> Diff: https://reviews.apache.org/r/38342/diff/
> 
> 
> Testing
> ---
> 
> cd 3rdparty/libprocess/3rdparty/stout
> ./boostrap
> ./configure
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

[Klaus Ma]

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

(Updated Sept. 24, 2015, 2:37 p.m.)


Review request for mesos, Alexander Rukletsov and Michael Park.


Changes
---

Resolved code conflict


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


Repository: mesos


Description
---

Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
converts a `google::protobuf::Message` into a `JSON::Object`.
We should add the support for `google::protobuf::RepeatedPtrField` by 
introducing overloaded functions.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 2285ce9 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 68328a2 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 8ebb798 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 34eb6d0 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 920f5c9 

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


Testing
---

cd 3rdparty/libprocess/3rdparty/stout
./boostrap
./configure
make


Thanks,

Klaus Ma

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37714, 37996, 37997, 37998, 37999, 38000, 38094]

All tests passed.

- Mesos ReviewBot


On Sept. 24, 2015, 10:33 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Sept. 24, 2015, 10:33 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/src/CMakeLists.txt 
> fb9bd04832779ac43151f2feb3dfbf58cb996434 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d0b9399d38fa284466a012a21080b1d9007af98b 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38051]

All tests passed.

- Mesos ReviewBot


On Sept. 24, 2015, 9:45 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 24, 2015, 9:45 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 6bee4f3 
>   src/tests/status_update_manager_tests.cpp 9970d71 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 1. Write a test for this change.
> 2. make successfully!
> 3. make check successfully!
> 4. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

[Alexander Rojas]

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

(Updated Sept. 24, 2015, 12:33 p.m.)


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


Changes
---

Added new file to CMakeFiles.txt


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/src/CMakeLists.txt 
fb9bd04832779ac43151f2feb3dfbf58cb996434 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d0b9399d38fa284466a012a21080b1d9007af98b 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

> On Sept. 23, 2015, 7:39 p.m., Alex Clemmer wrote:
> > Can we please add this file to the CMakeLists.txt file in 
> > 3rdparty/libprocess/src as well?

The included file is a header and so far cmake file doesn't seem to care about 
the include directory (can you correct me if I'm wrong). Still, shouln't the 
headers be there too? otherwise cmake wouldn't do proper dependency analysis.


- Alexander


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


On Sept. 23, 2015, 10:57 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Sept. 23, 2015, 10:57 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the authenticator manager, which is a class which handles the 
> actual authentication procedure during the execution of 
> `ProcessManager::handle()` and it also takes care of the life cycle of 
> instances of http::Authenticator.
> 
> No tests are added at this point since no public API is generated, the goal 
> of this patch is to implement the manager and verify nothing breaks 
> afterwards. Authenticator manager tests proper come in a latter patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38158: Refactored Value::Ranges coalesce().

[Alexander Rukletsov]

> On Sept. 23, 2015, 6:29 p.m., Ben Mahler wrote:
> > Where is the benchmark for this change? :(
> 
> Joerg Schad wrote:
> We benchmarked the code with a proprietary benchmark. I added 
> https://issues.apache.org/jira/browse/MESOS-3502 aiming at adding an 
> open-source benchmark for this.

Ben, I'm with you regarding benchmarking this change. But we thougt about this 
patch as a hotfix improvement, which makes the behavior not worse than the 
status quo. Basically, Jörg removed extra memory allocations and cleaned up 
deletions from a linear array.

I think we still need a deeper refactoring here (similar to `state.json`), with 
caching intermediate results or other ideas that can drastically speed up the 
performance. This will be a bigger project, which will definitely require a 
thorough benchmark.


- Alexander


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


On Sept. 23, 2015, 12:37 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38158/
> ---
> 
> (Updated Sept. 23, 2015, 12:37 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joris Van Remoortere, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3051
> https://issues.apache.org/jira/browse/MESOS-3051
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The goal of this refactoring was to reuse the Ranges objects as much as 
> possible, as prior there was substantial time spend in allocation/destruction 
> (MESOS-3051).
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 750264e603b4cde2011f07f4434a4b34fe3e512f 
>   src/tests/resources_tests.cpp 0318885336409f7cc9dbd4a3daa9b52db197bbd1 
>   src/tests/values_tests.cpp fc35d97894a2de6207b9337180e2160e6f2cb1f5 
> 
> Diff: https://reviews.apache.org/r/38158/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38137]

All tests passed.

- Mesos ReviewBot


On Sept. 24, 2015, 9:21 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38137/
> ---
> 
> (Updated Sept. 24, 2015, 9:21 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang 
> Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Joining all the commits around provisioner and local store into one review so 
> it's easier to review, as patches
> are changing code on top of each other.
> 
> All the commits are going to committed together.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 776483b 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.proto PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp 35d1199 
>   src/slave/flags.hpp 3f6601a 
>   src/slave/flags.cpp 69976d7 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1b0c304 
> 
> Diff: https://reviews.apache.org/r/38137/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 37996: Added property manager

[Alexander Rojas]

> On Sept. 23, 2015, 7:40 p.m., Alex Clemmer wrote:
> > Can we please add these tests to the CMakeLists.txt file in 
> > 3rdparty/libprocess/3rdparty/stout/tests?

It was done already since the first version of this patch.


- Alexander


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


On Sept. 23, 2015, 1:17 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37996/
> ---
> 
> (Updated Sept. 23, 2015, 1:17 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the `PathInheritedProperties` 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/properties.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> baa648aacfd5204c33e102b58126862729fbb612 
>   3rdparty/libprocess/3rdparty/stout/tests/properties_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37714: Updated Multimap and multihashmap so their signatures resemble that of hashmap and hashset.

[Alexander Rojas]

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

(Updated Sept. 24, 2015, 12:05 p.m.)


Review request for mesos, Joerg Schad, Michael Park, and Jan Schlicht.


Changes
---

`multihashmap` constructed from `std::multimap` instead than from `Multimap`. 
Initializer list constructors now delegate to parent constructor.


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


Repository: mesos


Description
---

Adds extra template parameters to `multihashmap` which offer control over the 
hash function to use as well as the equality operator.

Implements initializer_list, copy and move constructors for both, 
`multihashmap` and `Multimap` in a similar way as it was done for `hashmap` and 
`hashset`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/multihashmap.hpp 
d9e4031cee64e48ad50541c04ca11e7861d0a17c 
  3rdparty/libprocess/3rdparty/stout/include/stout/multimap.hpp 
fb3e7a1d0377001389980302342813217f49cf5f 
  3rdparty/libprocess/3rdparty/stout/tests/multimap_tests.cpp 
535cd2d10e3074c86c149ce85b205e73ca42ddd3 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37997: Running PathInheritedProperties tests from libprocess Makefile

[Alexander Rojas]

> On Sept. 23, 2015, 7:38 p.m., Alex Clemmer wrote:
> > Can we add these tests to the CMakeLists.txt file in 
> > 3rdparty/libprocess/3rdparty/stout/tests/ as well?

As I mentioned it was done in a previous patch. But the stout test with 
autotool are ran from the libprocess Makefile which forces the commit to be 
split in two.


- Alexander


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


On Sept. 23, 2015, 10:55 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37997/
> ---
> 
> (Updated Sept. 23, 2015, 10:55 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout tests are actually ran from libprocess makefile, which requires new 
> tests to be split in two commits.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> e64e3d9e958fab3c7c25fad17dcc2b279d255500 
> 
> Diff: https://reviews.apache.org/r/37997/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Yong Qiao Wang]

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

(Updated Sept. 24, 2015, 9:45 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Only update the task status when its old status is not terminal.


Diffs
-

  src/master/master.cpp 6bee4f3 
  src/tests/status_update_manager_tests.cpp 9970d71 

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


Testing (updated)
---

UT:
1. Write a test for this change.
2. make successfully!
3. make check successfully!
4. Run test framework successfully!


Thanks,

Yong Qiao Wang