Re: Review Request 39202: CMake: Moved libevent, gmock, http-parser to CMake on Windows.

[Alex Clemmer]

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


Ah. Sorry about the reviewbot problem. I thought the dependency chain of 
reviews was updated when I pushed, but it wasn't.

- Alex Clemmer


On Oct. 10, 2015, 4:36 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39202/
> ---
> 
> (Updated Oct. 10, 2015, 4:36 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Moved libevent, gmock, http-parser to CMake on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> f0de224b0d7924344fb1945b387b728a7241df87 
>   3rdparty/libprocess/3rdparty/http-parser/CMakeLists.txt PRE-CREATION 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> ee1c74d31e28136bf289f4100d79a8ce568cd3af 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> 60108ff598fb075584196aa3d8e8e66e726c9f2a 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> ea6db09e1a1aa01450aee93814e07f09feae7ac9 
> 
> Diff: https://reviews.apache.org/r/39202/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39182: Windows: Enable ip_tests.

[Alex Clemmer]

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

(Updated Oct. 10, 2015, 5:58 a.m.)


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


Bugs: MESOS-3439 and MESOS-3629
https://issues.apache.org/jira/browse/MESOS-3439
https://issues.apache.org/jira/browse/MESOS-3629


Repository: mesos


Description
---

Windows: Enable ip_tests.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/duration.hpp 
90551541f59889e96b21dbe1b65d3904850464c2 
  3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
d1e2df6151149e03ffb4a76e2c24ff78b891e016 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
50e35f43d87c69a83a9e7d039d1881404ea8be38 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
94292f8a46ec31bbaf6e52f48109322bbe123f70 

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


Testing
---

Ran stout_tests from VS.


Thanks,

Alex Clemmer

Re: Review Request 39201: Included `stout/check.hpp` in `future.hpp`.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39019, 39076, 39091, 39092, 39093, 39096, 39097, 39180, 
39181, 39201]

All tests passed.

- Mesos ReviewBot


On Oct. 10, 2015, 4:31 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39201/
> ---
> 
> (Updated Oct. 10, 2015, 4:31 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Windows integration work will require us to support `stout/net.hpp`,
> but this file includes `stout/os.hpp`, which would be onerous to port.
> 
> To avoid having to port that entire file, we simply roped in what we
> needed from `stout/net.hpp`. A consequence of unincluding os.hpp is that
> `process/future.hpp` now requires a direct inclusion of
> `stout/check.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 9006b8a83d03eab6e67de12a954110029b7d150e 
> 
> Diff: https://reviews.apache.org/r/39201/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39194: Added documentation and scripts for building mesos.apache.org website locally in a Docker container.

[Artem Harutyunyan]

> On Oct. 9, 2015, 3:52 p.m., Jonathon Rossi wrote:
> > support/website/files/run_site.sh, line 10
> > 
> >
> > With the changes we made you shouldn't pull from subversion anymore, 
> > the site files are in /site. Dave is yet to remove all the files from 
> > subversion other than the /publish directory.
> > 
> > The plan is to get everything going with the /publish directory 
> > manually committed to subversion and then work on removing subversion from 
> > the equation all together so pushing to git will cause the site to be built 
> > and deployed.

Thanks, I noticed 
https://git1-us-west.apache.org/repos/asf?p=mesos.git;a=commit;h=145dc90b127a76dbc0ced914e9c46a537aae903c
 just now. I'll make the changes (including the ones you proposed in the email 
on dev@ list).


- Artem


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


On Oct. 9, 2015, 3:43 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39194/
> ---
> 
> (Updated Oct. 9, 2015, 3:43 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3694
> https://issues.apache.org/jira/browse/MESOS-3694
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/website/Dockerfile PRE-CREATION 
>   support/website/README.md PRE-CREATION 
>   support/website/files/run_site.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39194/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Review Request 39202: CMake: Moved libevent, gmock, http-parser to CMake on Windows.

[Alex Clemmer]

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

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


Repository: mesos


Description
---

CMake: Moved libevent, gmock, http-parser to CMake on Windows.


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
f0de224b0d7924344fb1945b387b728a7241df87 
  3rdparty/libprocess/3rdparty/http-parser/CMakeLists.txt PRE-CREATION 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
ee1c74d31e28136bf289f4100d79a8ce568cd3af 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake 
60108ff598fb075584196aa3d8e8e66e726c9f2a 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
ea6db09e1a1aa01450aee93814e07f09feae7ac9 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 39201: Included `stout/check.hpp` in `future.hpp`.

[Alex Clemmer]

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

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


Repository: mesos


Description
---

The Windows integration work will require us to support `stout/net.hpp`,
but this file includes `stout/os.hpp`, which would be onerous to port.

To avoid having to port that entire file, we simply roped in what we
needed from `stout/net.hpp`. A consequence of unincluding os.hpp is that
`process/future.hpp` now requires a direct inclusion of
`stout/check.hpp`.


Diffs
-

  3rdparty/libprocess/include/process/future.hpp 
9006b8a83d03eab6e67de12a954110029b7d150e 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39181: Windows: Added support for `stout/net.hpp`.

[Alex Clemmer]

> On Oct. 9, 2015, 7:02 p.m., Alex Clemmer wrote:
> > 3rdparty/libprocess/include/process/future.hpp, line 39
> > 
> >
> > FYI: `future.hpp` actually requires `check.hpp`, but this requirement 
> > was actually hidden because `posix/windows.hpp` included `os.hpp`, which 
> > itself includes `check.hpp`.
> > 
> > But, since we don't want or need `os.hpp` (including it would block the 
> > Windows port of this file), we have to place this `#include` here.

ThiSorry, I must have thought future.hpp was in stout or something stupid like 
this. I've moved this change to #39201.


- Alex


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


On Oct. 10, 2015, 4:29 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39181/
> ---
> 
> (Updated Oct. 10, 2015, 4:29 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3630 and MESOS-3631
> https://issues.apache.org/jira/browse/MESOS-3630
> https://issues.apache.org/jira/browse/MESOS-3631
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added support for `stout/net.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
> 3f829bafe96526bc2263c9f228f85de38c435f60 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/net.hpp 
> 11e3895ee46e36faca0d2e1b436b61576826e472 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/net.hpp 
> 4f82796c2b9ef6a9198be145d969c5fce933be49 
> 
> Diff: https://reviews.apache.org/r/39181/diff/
> 
> 
> Testing
> ---
> 
> Ran stout tests from VS.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39181: Windows: Added support for `stout/net.hpp`.

[Alex Clemmer]

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

(Updated Oct. 10, 2015, 4:29 a.m.)


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


Bugs: MESOS-3630 and MESOS-3631
https://issues.apache.org/jira/browse/MESOS-3630
https://issues.apache.org/jira/browse/MESOS-3631


Repository: mesos


Description
---

Windows: Added support for `stout/net.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
3f829bafe96526bc2263c9f228f85de38c435f60 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/net.hpp 
11e3895ee46e36faca0d2e1b436b61576826e472 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/net.hpp 
4f82796c2b9ef6a9198be145d969c5fce933be49 

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


Testing
---

Ran stout tests from VS.


Thanks,

Alex Clemmer

Re: Review Request 39180: Windows: Added support for `stout/os/open.hpp`.

[Alex Clemmer]

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

(Updated Oct. 10, 2015, 4:28 a.m.)


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


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


Repository: mesos


Description
---

Windows: Added support for `stout/os/open.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/open.hpp 
43f261fd7a60b534f642f826ebf6ab18d72180c2 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/open.hpp 
023993d859e3a101ca387c1a514cd75de0d2beb1 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/open.hpp 
14fa11765c222cb4e80f5e45360d0f05facb578e 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
50e35f43d87c69a83a9e7d039d1881404ea8be38 

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


Testing
---

Ran both stout_tests from VS.


Thanks,

Alex Clemmer

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

[Alex Clemmer]

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

(Updated Oct. 10, 2015, 4:28 a.m.)


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


Repository: mesos


Description
---

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


Diffs (updated)
-

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

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


Testing
---


Thanks,

Alex Clemmer

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39018]

All tests passed.

- Mesos ReviewBot


On Oct. 10, 2015, 12:43 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 10, 2015, 12:43 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> New code was added to `ResourcesTest.ParsingFromJSON`, and two new tests were 
> added: `ResourcesTest.ParsingFromJSONWithRoles` and 
> `ResourcesTest.ParsingFromJSONError`. These attempt to test common error 
> scenarios that might show up in user-supplied JSON.
> 
> `make check` was used to confirm that all tests pass.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39096: CMake:[1/2] Moved `__WINDOWS__` flag definition to CompilationConfigure.

[Alex Clemmer]

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

(Updated Oct. 10, 2015, 4:27 a.m.)


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


Repository: mesos


Description
---

CMake:[1/2] Moved `__WINDOWS__` flag definition to CompilationConfigure.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake d1d75098d0ad655a7dcd74628bffd7ba8547b454 

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


Testing
---


Thanks,

Alex Clemmer

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

[Alex Clemmer]

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

(Updated Oct. 10, 2015, 4:27 a.m.)


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


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


Repository: mesos


Description
---

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


Diffs (updated)
-

  CMakeLists.txt a5a66c13496a95bb7095504a4645909d887fb49f 

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


Testing
---


Thanks,

Alex Clemmer

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

[Alex Clemmer]

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

(Updated Oct. 10, 2015, 4:25 a.m.)


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


Repository: mesos


Description
---

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


Diffs (updated)
-

  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
ee1c74d31e28136bf289f4100d79a8ce568cd3af 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39197: Provider tests: minor style fixes.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39196, 39197]

All tests passed.

- Mesos ReviewBot


On Oct. 9, 2015, 11:49 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39197/
> ---
> 
> (Updated Oct. 9, 2015, 11:49 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provider tests: minor style fixes.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/39197/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38919: Validation of Docker Image Manifests

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38901, 38919]

All tests passed.

- Mesos ReviewBot


On Oct. 9, 2015, 11:39 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38919/
> ---
> 
> (Updated Oct. 9, 2015, 11:39 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-3099
> https://issues.apache.org/jira/browse/MESOS-3099
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Validation of Docker Image Manifests
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38919/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

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

[Alex Clemmer]

> On Oct. 7, 2015, 5:05 p.m., Joseph Wu wrote:
> > CMakeLists.txt, line 97
> > 
> >
> > This only runs the libprocess tests.  Did you mean to do this?

Oh, man, somehow they must have gotten obliterated when I was moving this code 
around. Oops!


- Alex


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


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

Re: Review Request 39194: Added documentation and scripts for building mesos.apache.org website locally in a Docker container.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39194]

All tests passed.

- Mesos ReviewBot


On Oct. 9, 2015, 10:43 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39194/
> ---
> 
> (Updated Oct. 9, 2015, 10:43 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3694
> https://issues.apache.org/jira/browse/MESOS-3694
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/website/Dockerfile PRE-CREATION 
>   support/website/README.md PRE-CREATION 
>   support/website/files/run_site.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39194/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

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

[Joseph Wu]

> On Oct. 9, 2015, 4:14 p.m., Adam B wrote:
> > src/common/resources.cpp, lines 368-384
> > 
> >
> > This smells like a hack to workaround a bug in picojson. Can you link 
> > to something that indicates this as best practice or a recommended 
> > workaround? Is there a picojson issue we can track to know if this ever 
> > gets fixed?
> 
> Greg Mann wrote:
> I'll have to do some research on this one; there are no existing picojson 
> issues addressing this (only 27 issues have ever been raised in the project 
> :-), but I can certainly file one. I'll get back to you on this.

TLDR: Our bug, not a PicoJson bug.  (Good catch though!)

We should have a check after parsing: 
https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp#L740

`picojson::parse` will return the current position of stream iterator.  We 
should be saving this value and checking to see if it matches the end (minus 
whitespace).
See: 
https://github.com/kazuho/picojson/blob/25fc213cca61ea22b3c2e4db8def9927562ba5f7/picojson.h#L925

Note that the reason PicoJson does not barf on that input is because PicoJson 
supports "streaming" JSON objects.  i.e. You can give it a "stream" like `{} {} 
{}`, and expect three objects if you parse three times (passing the iterator 
along).


- Joseph


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


On Oct. 9, 2015, 5:43 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 9, 2015, 5:43 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> New code was added to `ResourcesTest.ParsingFromJSON`, and two new tests were 
> added: `ResourcesTest.ParsingFromJSONWithRoles` and 
> `ResourcesTest.ParsingFromJSONError`. These attempt to test common error 
> scenarios that might show up in user-supplied JSON.
> 
> `make check` was used to confirm that all tests pass.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

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

[Greg Mann]

> On Oct. 9, 2015, 11:14 p.m., Adam B wrote:
> > include/mesos/resources.hpp, lines 83-98
> > 
> >
> > Do these all need to be public?

Indeed they do not. In addition to making these private, I rearranged the 
member functions in resources.cpp so that private members are now located in an 
appropriately-labeled section.


> On Oct. 9, 2015, 11:14 p.m., Adam B wrote:
> > src/common/resources.cpp, lines 368-384
> > 
> >
> > This smells like a hack to workaround a bug in picojson. Can you link 
> > to something that indicates this as best practice or a recommended 
> > workaround? Is there a picojson issue we can track to know if this ever 
> > gets fixed?

I'll have to do some research on this one; there are no existing picojson 
issues addressing this (only 27 issues have ever been raised in the project 
:-), but I can certainly file one. I'll get back to you on this.


- Greg


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


On Oct. 10, 2015, 12:43 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 10, 2015, 12:43 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> New code was added to `ResourcesTest.ParsingFromJSON`, and two new tests were 
> added: `ResourcesTest.ParsingFromJSONWithRoles` and 
> `ResourcesTest.ParsingFromJSONError`. These attempt to test common error 
> scenarios that might show up in user-supplied JSON.
> 
> `make check` was used to confirm that all tests pass.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

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

[Greg Mann]

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

(Updated Oct. 10, 2015, 12:43 a.m.)


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


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

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


Diffs (updated)
-

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

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


Testing (updated)
---

New code was added to `ResourcesTest.ParsingFromJSON`, and two new tests were 
added: `ResourcesTest.ParsingFromJSONWithRoles` and 
`ResourcesTest.ParsingFromJSONError`. These attempt to test common error 
scenarios that might show up in user-supplied JSON.

`make check` was used to confirm that all tests pass.


Thanks,

Greg Mann

Re: Review Request 39182: Windows: Enable ip_tests.

[Joseph Wu]

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

Ship it!


Ship It!

- Joseph Wu


On Oct. 9, 2015, 12:49 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39182/
> ---
> 
> (Updated Oct. 9, 2015, 12:49 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3439 and MESOS-3629
> https://issues.apache.org/jira/browse/MESOS-3439
> https://issues.apache.org/jira/browse/MESOS-3629
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Enable ip_tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/duration.hpp 
> 90551541f59889e96b21dbe1b65d3904850464c2 
>   3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
> d1e2df6151149e03ffb4a76e2c24ff78b891e016 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 50e35f43d87c69a83a9e7d039d1881404ea8be38 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
> 
> Diff: https://reviews.apache.org/r/39182/diff/
> 
> 
> Testing
> ---
> 
> Ran stout_tests from VS.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39193: Fixed leakage of fts_open.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39192, 39193]

All tests passed.

- Mesos ReviewBot


On Oct. 9, 2015, 10:12 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39193/
> ---
> 
> (Updated Oct. 9, 2015, 10:12 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed leakage of fts_open.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp 53c568bd9559bbe46adb90265445f466245a0722 
> 
> Diff: https://reviews.apache.org/r/39193/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 39181: Windows: Added support for `stout/net.hpp`.

[Joseph Wu]

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

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 


Need to remove those files from stout/include/Makefile.am


- Joseph Wu


On Oct. 9, 2015, noon, Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39181/
> ---
> 
> (Updated Oct. 9, 2015, noon)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3630 and MESOS-3631
> https://issues.apache.org/jira/browse/MESOS-3630
> https://issues.apache.org/jira/browse/MESOS-3631
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added support for `stout/net.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
> 3f829bafe96526bc2263c9f228f85de38c435f60 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/net.hpp 
> 11e3895ee46e36faca0d2e1b436b61576826e472 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/net.hpp 
> 4f82796c2b9ef6a9198be145d969c5fce933be49 
>   3rdparty/libprocess/include/process/future.hpp 
> 9006b8a83d03eab6e67de12a954110029b7d150e 
> 
> Diff: https://reviews.apache.org/r/39181/diff/
> 
> 
> Testing
> ---
> 
> Ran stout tests from VS.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39180: Windows: Added support for `stout/os/open.hpp`.

[Joseph Wu]

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

Ship it!


Looks good (identical to the original open.hpp, except for the one new #ifdef).


3rdparty/libprocess/3rdparty/stout/include/stout/os/open.hpp (line 20)


You need to remove these files from 
3rdparty/libprocess/3rdparty/stout/include/Makefile.am



3rdparty/libprocess/3rdparty/stout/include/stout/os/open.hpp (lines 58 - 61)


One newline too many here.


- Joseph Wu


On Oct. 9, 2015, 11:30 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39180/
> ---
> 
> (Updated Oct. 9, 2015, 11:30 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3693
> https://issues.apache.org/jira/browse/MESOS-3693
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added support for `stout/os/open.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/open.hpp 
> 43f261fd7a60b534f642f826ebf6ab18d72180c2 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/open.hpp 
> 023993d859e3a101ca387c1a514cd75de0d2beb1 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/open.hpp 
> 14fa11765c222cb4e80f5e45360d0f05facb578e 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 50e35f43d87c69a83a9e7d039d1881404ea8be38 
> 
> Diff: https://reviews.apache.org/r/39180/diff/
> 
> 
> Testing
> ---
> 
> Ran both stout_tests from VS.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Review Request 39197: Provider tests: minor style fixes.

[Jojy Varghese]

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

Review request for mesos and Timothy Chen.


Repository: mesos


Description
---

Provider tests: minor style fixes.


Diffs
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Review Request 39196: Puller tests: removed extraneous sandbox directory

[Jojy Varghese]

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

Review request for mesos and Timothy Chen.


Repository: mesos


Description
---

Puller tests: removed extraneous sandbox directory


Diffs
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38919: Validation of Docker Image Manifests

[Anand Mazumdar]

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

Ship it!


Ship It!

- Anand Mazumdar


On Oct. 9, 2015, 11:39 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38919/
> ---
> 
> (Updated Oct. 9, 2015, 11:39 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-3099
> https://issues.apache.org/jira/browse/MESOS-3099
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Validation of Docker Image Manifests
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38919/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 38919: Validation of Docker Image Manifests

[Gilbert Song]

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

(Updated Oct. 9, 2015, 4:39 p.m.)


Review request for mesos, Jojy Varghese and Timothy Chen.


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


Repository: mesos


Description
---

Validation of Docker Image Manifests


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check (Ubuntu14.04 + clang++-3.6)


Thanks,

Gilbert Song

Re: Review Request 39152: Pass LIBPROCESS_IP even when executor environment is specified.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39152]

All tests passed.

- Mesos ReviewBot


On Oct. 9, 2015, 8:34 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39152/
> ---
> 
> (Updated Oct. 9, 2015, 8:34 p.m.)
> 
> 
> Review request for mesos, Adam B and Kapil Arya.
> 
> 
> Bugs: MESOS-3553
> https://issues.apache.org/jira/browse/MESOS-3553
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If DNS is not available on the agent node and a task is launched which 
> explicitly specifies the executor's environment, LIBPROCESS_IP will not be 
> passed through and the default hostname lookup after spawning the executor 
> process will throw an error. This patch alters the agent to always pass 
> LIBPROCESS_IP, even when the executor environment is specified.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> 25c87e9f948b7efe8b9a853c403bee69982d6c4c 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 5bc7d408bda0c249e1b66747d8bd87e688362e6c 
> 
> Diff: https://reviews.apache.org/r/39152/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39194: Added documentation and scripts for building mesos.apache.org website locally in a Docker container.

[Greg Mann]

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



support/website/README.md (line 36)


Since this will live within the Mesos repo, this step can probably be 
omitted?


- Greg Mann


On Oct. 9, 2015, 10:43 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39194/
> ---
> 
> (Updated Oct. 9, 2015, 10:43 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3694
> https://issues.apache.org/jira/browse/MESOS-3694
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/website/Dockerfile PRE-CREATION 
>   support/website/README.md PRE-CREATION 
>   support/website/files/run_site.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39194/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

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

[Adam B]

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


Looking good, but I haven't made it to the tests yet (coming soon), and I 
assume the v1 resources are identical?

Can you elaborate on the "Testing Done"? Call out any new/modified unit tests, 
or any existing tests that verify your changes did not affect functionality?


include/mesos/resources.hpp (line 80)


"a Resources object containing a single resource"? Looks like it's actually 
"a single Resource object". Maybe just s/Resources/Resource/?



include/mesos/resources.hpp (lines 83 - 98)


Do these all need to be public?



include/mesos/resources.hpp (line 98)


s/checkParsedResource/validateParsedResource/?



src/common/resources.cpp (line 362)


You call `resourcesTry.get()` 4 times in this block. Maybe worth a 
`JSON::Value resourcesValue = resourcesTry.get();` above?



src/common/resources.cpp (lines 368 - 384)


This smells like a hack to workaround a bug in picojson. Can you link to 
something that indicates this as best practice or a recommended workaround? Is 
there a picojson issue we can track to know if this ever gets fixed?



src/common/resources.cpp (line 482)


Only needed just before the `foreach`. Might be unnecessary if 
protobuf::parse errors, so let's move it down to where it's used.



src/common/resources.cpp (lines 501 - 508)


Duplicated code. Can you factor this out?



src/common/resources.cpp (line 522)


s/validation/validationError/ (or just `error`) because 
"validation.isSome()" sounds like a good thing.



src/common/resources.cpp (line 535)


Disk resources are fine. Just not a disk with a 'persistence' field.



src/common/resources.cpp (line 632)


No longer needs explicit namespace, since we're `using RepeatedPtrField`. 
Ditto for the 2 other uses in this file.


- Adam B


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

Re: Review Request 38919: Validation of Docker Image Manifests

[Gilbert Song]

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

(Updated Oct. 9, 2015, 4:08 p.m.)


Review request for mesos, Jojy Varghese and Timothy Chen.


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


Repository: mesos


Description
---

Validation of Docker Image Manifests


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check (Ubuntu14.04 + clang++-3.6)


Thanks,

Gilbert Song

Re: Review Request 39193: Fixed leakage of fts_open.

[Ben Mahler]

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



src/linux/cgroups.cpp (line 925)


Ditto here, can you preserve the intended errno for ErrnoError?


- Ben Mahler


On Oct. 9, 2015, 10:12 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39193/
> ---
> 
> (Updated Oct. 9, 2015, 10:12 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed leakage of fts_open.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp 53c568bd9559bbe46adb90265445f466245a0722 
> 
> Diff: https://reviews.apache.org/r/39193/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 39192: Fixed leakage of fts_open.

[Ben Mahler]

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



3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp (line 243)


Can you preserve errno here and in the other cases you've touched?


- Ben Mahler


On Oct. 9, 2015, 10:10 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39192/
> ---
> 
> (Updated Oct. 9, 2015, 10:10 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed leakage of fts_open.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> 5d2f39d9a9d963225bf463572cb8fe99dd9aa6f5 
> 
> Diff: https://reviews.apache.org/r/39192/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

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

[Ben Mahler]

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


Overall looks good, some of the comments I made can be addressed in separate 
reviews.


include/mesos/containerizer/containerizer.proto (lines 88 - 92)


Why do you say "executor" here but termination is about "container" above?

Also, can you:

s/task_state/state/
s/task_status_reasons/reasons/

s/'reason'/'reasons'/
s/of a status update for those pending/unterminated tasks/of status updates 
for non-terminal tasks/



include/mesos/mesos.proto (line 1100)


0 should have been UNKNOWN, ugh :(



include/mesos/mesos.proto (lines 1119 - 1122)


Can you keep these alphabetical?

It's a bit unfortunate that we don't have a common 
"REASON_CONTAINER_LIMITATION" prefix for the limitations. Lesson learned for 
the next time we decide to name enums to use use prefixing to group related 
enums...

Do you want to add a generic limitation? I believe it's ok to rename these 
from a conversation with vinod, given they were intended for metrics rather 
than control flow in schedulers:

REASON_CONTAINER_LIMITATION
REASON_CONTAINER_LIMITATION_DISK
REASON_CONTAINER_LIMITATION_MEMORY

Note the presence of a general limitation that custom isolators can use.



include/mesos/slave/isolator.proto (line 42)


How about: s/for those tasks that are in non-terminal status when the 
container is terminated/for any remaining non-terminal tasks/ ?



include/mesos/slave/isolator.proto (line 44)


Why not s/task_status_reason/reason/ ?



src/slave/containerizer/mesos/containerizer.cpp (lines 1235 - 1237)


Can you use the arrow operator here?

status->isSome
status->get



src/slave/containerizer/mesos/containerizer.cpp (line 1243)


! empty() ?



src/slave/containerizer/mesos/containerizer.cpp (line 1257)


Why did you need the trim here?



src/slave/slave.hpp (lines 306 - 307)


Could you wrap on the next line, as is done with getExecutorInfo?



src/slave/slave.hpp (lines 629 - 636)


Hm.. I think people mind get a bit confused when they encounter this, how 
about we add a comment to guide them and rename it like the following:

```
// When the agent initiates a destroyal of the container,
// we expect a termination to occur. The 'pendingTermation'
// indicates why the agent initiated the destruction and
// will influence the information sent in the status updates
// for any remaining non-terminal tasks.
Option pendingTermination;
```



src/slave/slave.cpp (line 1661)


Shall we rename this to 'update' to be a bit clearer?



src/slave/slave.cpp (line 2656)


Ditto here.



src/slave/slave.cpp (lines 2668 - 2678)


Weird that some of the existing code paths don't set the executor state to 
TERMINATING, do we need a function to make the destroyal code consistent?

Shall we follow up in a separate patch?



src/slave/slave.cpp (line 2716)


Is it easy to add the value of the timeout here? E.g.

```
Executor did not re-register within 10secs
```



src/slave/slave.cpp (line 2931)


Ditto here.



src/slave/slave.cpp (line 2955)


Please use the arrow operator going forward :)

future->isFailed
future->failure



src/slave/slave.cpp (line 3248)


Mind avoiding the ternary just for clarity? Up to you.



src/slave/slave.cpp (line 3372)


Ditto here, this is 'launched'?



src/slave/slave.cpp (line 3398)


Odd that we don't go set the state to TERMINATING here as well, for example.



src/slave/slave.cpp (lines 3954 - 3955)


Ditto here about adding the value if possible



src/slave/slave.cpp (line 4398)


Odd that th

Re: Review Request 39194: Added documentation and scripts for building mesos.apache.org website locally in a Docker container.

[Jonathon Rossi]

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



support/website/files/run_site.sh (line 10)


With the changes we made you shouldn't pull from subversion anymore, the 
site files are in /site. Dave is yet to remove all the files from subversion 
other than the /publish directory.

The plan is to get everything going with the /publish directory manually 
committed to subversion and then work on removing subversion from the equation 
all together so pushing to git will cause the site to be built and deployed.


- Jonathon Rossi


On Oct. 9, 2015, 10:43 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39194/
> ---
> 
> (Updated Oct. 9, 2015, 10:43 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3694
> https://issues.apache.org/jira/browse/MESOS-3694
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/website/Dockerfile PRE-CREATION 
>   support/website/README.md PRE-CREATION 
>   support/website/files/run_site.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39194/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Review Request 39194: Added documentation and scripts for building mesos.apache.org website locally in a Docker container.

[Artem Harutyunyan]

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

Review request for mesos and Adam B.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  support/website/Dockerfile PRE-CREATION 
  support/website/README.md PRE-CREATION 
  support/website/files/run_site.sh PRE-CREATION 

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


Testing
---


Thanks,

Artem Harutyunyan

Re: Review Request 39182: Windows: Enable ip_tests.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [39019, 39076, 39091, 39092, 39093, 39096, 39097, 39180, 39181]

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

Error:
 2015-10-09 22:29:55 URL:https://reviews.apache.org/r/39181/diff/raw/ 
[11913/11913] -> "39181.patch" [1]
Successfully applied: Windows: Added support for `stout/net.hpp`.

Windows: Added support for `stout/net.hpp`.


Review: https://reviews.apache.org/r/39181
Checking 2 files using filter 
--filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/operators,+whitespace/semicolon,+whitespace/tab,+whitespace/todo
Total errors found: 0
ERROR: Commit spanning multiple projects.

Please use separate commits for mesos, libprocess and stout.

Paths grouped by project:
libprocess:
  3rdparty/libprocess/include/process/future.hpp
stout:
  3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/net.hpp
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/net.hpp
Failed to commit patch

- Mesos ReviewBot


On Oct. 9, 2015, 7:49 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39182/
> ---
> 
> (Updated Oct. 9, 2015, 7:49 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3439 and MESOS-3629
> https://issues.apache.org/jira/browse/MESOS-3439
> https://issues.apache.org/jira/browse/MESOS-3629
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Enable ip_tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/duration.hpp 
> 90551541f59889e96b21dbe1b65d3904850464c2 
>   3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
> d1e2df6151149e03ffb4a76e2c24ff78b891e016 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 50e35f43d87c69a83a9e7d039d1881404ea8be38 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
> 
> Diff: https://reviews.apache.org/r/39182/diff/
> 
> 
> Testing
> ---
> 
> Ran stout_tests from VS.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38919: Validation of Docker Image Manifests

[Gilbert Song]

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

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


Review request for mesos, Jojy Varghese and Timothy Chen.


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


Repository: mesos


Description
---

Validation of Docker Image Manifests


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check (Ubuntu14.04 + clang++-3.6)


Thanks,

Gilbert Song

Re: Review Request 38919: Validation of Docker Image Manifests

[Anand Mazumdar]

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


Mainly style nits otherwise looks pretty good.


src/slave/containerizer/provisioner/docker/spec.hpp (line 38)


Nit: s/validateManifest/validate

We tend to use non-redundant names. This is clear from the 
signature/comment that this function validates a manifest.

`Option validate(const docker::DockerImageManifest& manifest);`



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


I am a bit confused here. Doesn't the `validateManifest` function validates 
this 4 items ?

If so, can we remove this TODO ?



src/slave/containerizer/provisioner/docker/spec.cpp (line 38)


I am assuming .size() returns an `std::size_t` ?

Hence just checking for `==0` should suffice ?

Ditto for the other 2 cases



src/slave/containerizer/provisioner/docker/spec.cpp (line 39)


We tend to avoid periods at the end of log/error statements. Can you remove 
them. Ditto for the other 3 occurences



src/slave/containerizer/provisioner/docker/spec.cpp (line 50)


s/va/v1



src/slave/containerizer/provisioner/docker/spec.cpp (line 52)


s/Number of/Size of



src/slave/containerizer/provisioner/docker/spec.cpp (line 59)


const string& blobSum



src/slave/containerizer/provisioner/docker/spec.cpp (line 64)


Unused variable. Remove it.



src/slave/containerizer/provisioner/docker/spec.cpp (line 65)


Unused variable, remove it.



src/slave/containerizer/provisioner/docker/spec.cpp (line 83)


s/Manisfests/Manifest



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


Did you miss adding this block in the last review of this chain ?

Usually we tend to make each review in a chain self-complete by itself that 
can be ideally submitted on it's own. 

There might be some exceptions to this in some circumstances though when 
this is not possible.



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


How about:

// Test Manifest Validation with empty repeated 'fsLayers' field.

Ditto for the following test-case.


- Anand Mazumdar


On Oct. 6, 2015, 10:35 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38919/
> ---
> 
> (Updated Oct. 6, 2015, 10:35 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-3099
> https://issues.apache.org/jira/browse/MESOS-3099
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Validation of Docker Image Manifests
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38919/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 39193: Fixed leakage of fts_open.

[Chi Zhang]

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

Review request for mesos and Ben Mahler.


Bugs: mesos-3519
https://issues.apache.org/jira/browse/mesos-3519


Repository: mesos


Description
---

Fixed leakage of fts_open.


Diffs
-

  src/linux/cgroups.cpp 53c568bd9559bbe46adb90265445f466245a0722 

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


Testing
---


Thanks,

Chi Zhang

Review Request 39192: Fixed leakage of fts_open.

[Chi Zhang]

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

Review request for mesos and Ben Mahler.


Bugs: mesos-3519
https://issues.apache.org/jira/browse/mesos-3519


Repository: mesos


Description
---

Fixed leakage of fts_open.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
5d2f39d9a9d963225bf463572cb8fe99dd9aa6f5 

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


Testing
---


Thanks,

Chi Zhang

Re: Review Request 39187: added FileDescriptor tests in libprocess Makefile.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39186, 39187]

All tests passed.

- Mesos ReviewBot


On Oct. 9, 2015, 7:05 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39187/
> ---
> 
> (Updated Oct. 9, 2015, 7:05 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: mesos-3520
> https://issues.apache.org/jira/browse/mesos-3520
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> added FileDescriptor tests in libprocess Makefile.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> e64e3d9e958fab3c7c25fad17dcc2b279d255500 
> 
> Diff: https://reviews.apache.org/r/39187/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 39152: Pass LIBPROCESS_IP even when executor environment is specified.

[Adam B]

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

Ship it!


Thanks! I'll fix the indent and commit this.

Also, you did more than just run `make check`, you modified 
MesosContainerizerIsolatorPreparationTest.ExecutorEnvironmentVariable to test 
your env-variable setting. Call that out in the "Testing Done" section.


src/tests/containerizer/mesos_containerizer_tests.cpp (lines 334 - 336)


We only indent 2 spaces after a trailing '='.
Try a `grep -rn -B1 "^[^a-zA-Z0-9]*\".*\"$" src/ |grep -A2 "=$"` and you'll 
see.


- Adam B


On Oct. 9, 2015, 1:34 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39152/
> ---
> 
> (Updated Oct. 9, 2015, 1:34 p.m.)
> 
> 
> Review request for mesos, Adam B and Kapil Arya.
> 
> 
> Bugs: MESOS-3553
> https://issues.apache.org/jira/browse/MESOS-3553
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If DNS is not available on the agent node and a task is launched which 
> explicitly specifies the executor's environment, LIBPROCESS_IP will not be 
> passed through and the default hostname lookup after spawning the executor 
> process will throw an error. This patch alters the agent to always pass 
> LIBPROCESS_IP, even when the executor environment is specified.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> 25c87e9f948b7efe8b9a853c403bee69982d6c4c 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 5bc7d408bda0c249e1b66747d8bd87e688362e6c 
> 
> Diff: https://reviews.apache.org/r/39152/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

[Gilbert Song]

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

(Updated Oct. 9, 2015, 2:39 p.m.)


Review request for mesos, Jojy Varghese and Timothy Chen.


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


Repository: mesos


Description
---

Serialize Docker Image Spec as Protobuf


Diffs (updated)
-

  src/CMakeLists.txt 828dbb690841c561816811dfbb044aa3afead89d 
  src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
  src/slave/containerizer/provisioner/docker/message.proto 
bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
  src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check (ubuntu 14.04 + clang++-3.6)


Thanks,

Gilbert Song

Re: Review Request 39152: Pass LIBPROCESS_IP even when executor environment is specified.

[Anand Mazumdar]

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

Ship it!


LGTM


src/tests/containerizer/mesos_containerizer_tests.cpp (line 366)


Not a huge fan of this hackery but this approach is also used by other 
similar tests. I wonder if we should create a separate test fixture for this 
test/such tests in general ?

Also , wondering what do other projects do or what is the recommended way ?


- Anand Mazumdar


On Oct. 9, 2015, 8:34 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39152/
> ---
> 
> (Updated Oct. 9, 2015, 8:34 p.m.)
> 
> 
> Review request for mesos, Adam B and Kapil Arya.
> 
> 
> Bugs: MESOS-3553
> https://issues.apache.org/jira/browse/MESOS-3553
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If DNS is not available on the agent node and a task is launched which 
> explicitly specifies the executor's environment, LIBPROCESS_IP will not be 
> passed through and the default hostname lookup after spawning the executor 
> process will throw an error. This patch alters the agent to always pass 
> LIBPROCESS_IP, even when the executor environment is specified.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> 25c87e9f948b7efe8b9a853c403bee69982d6c4c 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 5bc7d408bda0c249e1b66747d8bd87e688362e6c 
> 
> Diff: https://reviews.apache.org/r/39152/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39152: Pass LIBPROCESS_IP even when executor environment is specified.

[Greg Mann]

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

(Updated Oct. 9, 2015, 8:33 p.m.)


Review request for mesos and Kapil Arya.


Changes
---

Changed conditional in test.


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


Repository: mesos


Description
---

If DNS is not available on the agent node and a task is launched which 
explicitly specifies the executor's environment, LIBPROCESS_IP will not be 
passed through and the default hostname lookup after spawning the executor 
process will throw an error. This patch alters the agent to always pass 
LIBPROCESS_IP, even when the executor environment is specified.


Diffs (updated)
-

  src/slave/containerizer/containerizer.cpp 
25c87e9f948b7efe8b9a853c403bee69982d6c4c 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
5bc7d408bda0c249e1b66747d8bd87e688362e6c 

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


Testing
---

`make check`


Thanks,

Greg Mann

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

[Anand Mazumdar]

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


LGTM, Just some minor comments/concerns about including `headers` that we 
need/style issues. Can you do a sweep of the `includes` in case I missed 
pointing some of them here.


src/Makefile.am (line 852)


Nit: Can we align the trailing `` ?



src/slave/containerizer/provisioner/docker/spec.hpp (line 18)


We generally prefer header files that are `complete` i.e. they compile on 
their own. Can you ensure that we include `stout/json.hpp` here since it has 
`JSON::Object`. Also , do a sweep of the other includes.



src/slave/containerizer/provisioner/docker/spec.hpp (line 22)


Do we need this ? If not, remove this include.



src/slave/containerizer/provisioner/docker/spec.cpp (line 20)


Do you need this ? If not remove it



src/slave/containerizer/provisioner/docker/spec.cpp (line 24)


Do you need this ? If not remove it



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


Remove this comment. It's Redundant/self-explanatory here.



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


s/EXPECT_SOME/ASSERT_SOME , since it makes no sense to continue this test 
if this fails.



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


Nit: Does it make sense to change the ASSERT_EQ to EXPECT_EQ ? 

Might help the user to see a list of all the things that failed instead of 
just aborting at the first failure.



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


How about :

`// This is an invalid manifest. The required fields 'signatures' and 
'schemaVersion' are not set.`

While testing, we only make assumptions about the things we are testing as 
part of our test. The `validateManifest` call is not part of this test, so its 
better to just ignore/not mention it.


- Anand Mazumdar


On Oct. 7, 2015, 6:57 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38901/
> ---
> 
> (Updated Oct. 7, 2015, 6:57 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-2972
> https://issues.apache.org/jira/browse/MESOS-2972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Serialize Docker Image Spec as Protobuf
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 828dbb690841c561816811dfbb044aa3afead89d 
>   src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38901/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu 14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 39182: Windows: Enable ip_tests.

[Alex Clemmer]

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

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


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


Changes
---

This is solid evidence that stout/ip.hpp works on Windows.


Bugs: MESOS-3439 and MESOS-3629
https://issues.apache.org/jira/browse/MESOS-3439
https://issues.apache.org/jira/browse/MESOS-3629


Repository: mesos


Description
---

Windows: Enable ip_tests.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/duration.hpp 
90551541f59889e96b21dbe1b65d3904850464c2 
  3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
d1e2df6151149e03ffb4a76e2c24ff78b891e016 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
50e35f43d87c69a83a9e7d039d1881404ea8be38 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
94292f8a46ec31bbaf6e52f48109322bbe123f70 

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


Testing
---

Ran stout_tests from VS.


Thanks,

Alex Clemmer

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

[Timothy Chen]

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

Ship it!


Ship It!


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


You don't need to use TemporaryDirectoryTest since you're not writing any 
files.

You probably can just use ::testing::Test as base


- Timothy Chen


On Oct. 7, 2015, 6:57 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38901/
> ---
> 
> (Updated Oct. 7, 2015, 6:57 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-2972
> https://issues.apache.org/jira/browse/MESOS-2972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Serialize Docker Image Spec as Protobuf
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 828dbb690841c561816811dfbb044aa3afead89d 
>   src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38901/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu 14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39102]

All tests passed.

- Mesos ReviewBot


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

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

[Jie Yu]

> On Oct. 9, 2015, 2:06 a.m., Kapil Arya wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, lines 1109-1115
> > 
> >
> > I am slightly confused here. If one or more `Isolator::prepare` calls 
> > returned a failure, where are we accumulating the failure messages?

No, the prepare of each isolator is serialized. So the error message will be 
from the first failed isolator->prepare.


- Jie


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


On Oct. 9, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38746/
> ---
> 
> (Updated Oct. 9, 2015, 1:28 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.
> 
> The following doc summarized the problem and proposed a solution:
> https://docs.google.com/document/d/1klGDAu5yBVf-CGWLqvELLIfxLfRaisGkhi6Gn7952-4/edit?usp=sharing
> 
> 
> 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 6c975f904178e01797b67628a2d471ec7b3b1fbf 
>   src/slave/containerizer/external_containerizer.cpp 
> 211649201777f0d2ce802a865090129eacdd53be 
>   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.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
>   src/tests/containerizer.cpp 1f743155526192569dd61a47ab67d8e58aab205a 
>   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
> 
> tested with Docker as well.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 39181: Windows: Added support for `stout/net.hpp`.

[Alex Clemmer]

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

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


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


Bugs: MESOS-3630 and MESOS-3631
https://issues.apache.org/jira/browse/MESOS-3630
https://issues.apache.org/jira/browse/MESOS-3631


Repository: mesos


Description
---

Windows: Added support for `stout/net.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
3f829bafe96526bc2263c9f228f85de38c435f60 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/net.hpp 
11e3895ee46e36faca0d2e1b436b61576826e472 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/net.hpp 
4f82796c2b9ef6a9198be145d969c5fce933be49 
  3rdparty/libprocess/include/process/future.hpp 
9006b8a83d03eab6e67de12a954110029b7d150e 

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


Testing
---

Ran stout tests from VS.


Thanks,

Alex Clemmer

Review Request 39187: added FileDescriptor tests in libprocess Makefile.

[Chi Zhang]

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

Review request for mesos and Ben Mahler.


Bugs: mesos-3520
https://issues.apache.org/jira/browse/mesos-3520


Repository: mesos


Description
---

added FileDescriptor tests in libprocess Makefile.


Diffs
-

  3rdparty/libprocess/3rdparty/Makefile.am 
e64e3d9e958fab3c7c25fad17dcc2b279d255500 

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


Testing
---


Thanks,

Chi Zhang

Re: Review Request 39152: Pass LIBPROCESS_IP even when executor environment is specified.

[Kapil Arya]

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

Ship it!


Ship It!

- Kapil Arya


On Oct. 8, 2015, 8:57 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39152/
> ---
> 
> (Updated Oct. 8, 2015, 8:57 p.m.)
> 
> 
> Review request for mesos and Kapil Arya.
> 
> 
> Bugs: MESOS-3553
> https://issues.apache.org/jira/browse/MESOS-3553
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If DNS is not available on the agent node and a task is launched which 
> explicitly specifies the executor's environment, LIBPROCESS_IP will not be 
> passed through and the default hostname lookup after spawning the executor 
> process will throw an error. This patch alters the agent to always pass 
> LIBPROCESS_IP, even when the executor environment is specified.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> 25c87e9f948b7efe8b9a853c403bee69982d6c4c 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 5bc7d408bda0c249e1b66747d8bd87e688362e6c 
> 
> Diff: https://reviews.apache.org/r/39152/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 39186: added a FileDescriptor to manage the life cycle of fds.

[Chi Zhang]

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

Review request for mesos and Ben Mahler.


Bugs: mesos-3520
https://issues.apache.org/jira/browse/mesos-3520


Repository: mesos


Description
---

added a FileDescriptor to manage the life cycle of fds.


Diffs
-

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

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


Testing
---


Thanks,

Chi Zhang

Re: Review Request 39181: Windows: Added support for `stout/net.hpp`.

[Alex Clemmer]

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



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


FYI: `future.hpp` actually requires `check.hpp`, but this requirement was 
actually hidden because `posix/windows.hpp` included `os.hpp`, which itself 
includes `check.hpp`.

But, since we don't want or need `os.hpp` (including it would block the 
Windows port of this file), we have to place this `#include` here.


- Alex Clemmer


On Oct. 9, 2015, 7 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39181/
> ---
> 
> (Updated Oct. 9, 2015, 7 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3630 and MESOS-3631
> https://issues.apache.org/jira/browse/MESOS-3630
> https://issues.apache.org/jira/browse/MESOS-3631
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added support for `stout/net.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
> 3f829bafe96526bc2263c9f228f85de38c435f60 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/net.hpp 
> 11e3895ee46e36faca0d2e1b436b61576826e472 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/net.hpp 
> 4f82796c2b9ef6a9198be145d969c5fce933be49 
>   3rdparty/libprocess/include/process/future.hpp 
> 9006b8a83d03eab6e67de12a954110029b7d150e 
> 
> Diff: https://reviews.apache.org/r/39181/diff/
> 
> 
> Testing
> ---
> 
> Ran stout tests from VS.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39152: Pass LIBPROCESS_IP even when executor environment is specified.

[Cody Maloney]

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


LGTM!

- Cody Maloney


On Oct. 9, 2015, 12:57 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39152/
> ---
> 
> (Updated Oct. 9, 2015, 12:57 a.m.)
> 
> 
> Review request for mesos and Kapil Arya.
> 
> 
> Bugs: MESOS-3553
> https://issues.apache.org/jira/browse/MESOS-3553
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If DNS is not available on the agent node and a task is launched which 
> explicitly specifies the executor's environment, LIBPROCESS_IP will not be 
> passed through and the default hostname lookup after spawning the executor 
> process will throw an error. This patch alters the agent to always pass 
> LIBPROCESS_IP, even when the executor environment is specified.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> 25c87e9f948b7efe8b9a853c403bee69982d6c4c 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 5bc7d408bda0c249e1b66747d8bd87e688362e6c 
> 
> Diff: https://reviews.apache.org/r/39152/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39014: RegistryClient refactor: renamed ManifestResponse

[Jojy Varghese]

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

(Updated Oct. 9, 2015, 6:37 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased.


Repository: mesos


Description
---

RegistryClient refactor: renamed ManifestResponse as per review comments.


Diffs (updated)
-

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

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39156: RegistryClient refactor: changed getManifest interface

[Jojy Varghese]

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

(Updated Oct. 9, 2015, 6:37 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased.


Repository: mesos


Description
---

RegistryClient refactor: changed getManifest interface


Diffs (updated)
-

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

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38579: Refactored registry client

[Jojy Varghese]

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

(Updated Oct. 9, 2015, 6:36 p.m.)


Review request for mesos, Ben Mahler and Timothy Chen.


Changes
---

rebased.


Repository: mesos


Description
---

- Cleanup and broke big methods into smaller chunks.


Diffs (updated)
-

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

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39184: RegistryClient refactor: reordered ctor parameters

[Jojy Varghese]

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

(Updated Oct. 9, 2015, 6:35 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased after reordering patches.


Repository: mesos


Description
---

RegistryClient refactor: reordered ctor parameters


Diffs (updated)
-

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

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38443: Added layerid information to ManifestResponse

[Jojy Varghese]

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

(Updated Oct. 9, 2015, 6:34 p.m.)


Review request for mesos, Ben Mahler and Timothy Chen.


Changes
---

Split change into small patches


Repository: mesos


Description
---

Added layerid information to ManifestResponse


Diffs (updated)
-

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

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39155: RegistryClient refactor: removed nested namespace references

[Jojy Varghese]

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

(Updated Oct. 9, 2015, 6:32 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased after patch reorder


Repository: mesos


Description
---

RegistryClient refactor: removed nested namespace references


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39182: Windows: Enable ip_tests.

[Alex Clemmer]

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

(Updated Oct. 9, 2015, 6:30 p.m.)


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


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


Repository: mesos


Description
---

Windows: Enable ip_tests.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/duration.hpp 
90551541f59889e96b21dbe1b65d3904850464c2 
  3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
d1e2df6151149e03ffb4a76e2c24ff78b891e016 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
50e35f43d87c69a83a9e7d039d1881404ea8be38 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
94292f8a46ec31bbaf6e52f48109322bbe123f70 

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


Testing (updated)
---

Ran stout_tests from VS.


Thanks,

Alex Clemmer

Re: Review Request 39180: Windows: Added support for `stout/os/open.hpp`.

[Alex Clemmer]

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

(Updated Oct. 9, 2015, 6:30 p.m.)


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


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


Repository: mesos


Description
---

Windows: Added support for `stout/os/open.hpp`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/open.hpp 
43f261fd7a60b534f642f826ebf6ab18d72180c2 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/open.hpp 
023993d859e3a101ca387c1a514cd75de0d2beb1 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/open.hpp 
14fa11765c222cb4e80f5e45360d0f05facb578e 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
50e35f43d87c69a83a9e7d039d1881404ea8be38 

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


Testing (updated)
---

Ran both stout_tests from VS.


Thanks,

Alex Clemmer

Review Request 39184: RegistryClient refactor: reordered ctor parameters

[Jojy Varghese]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

RegistryClient refactor: reordered ctor parameters


Diffs
-

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

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39181: Windows: Added support for `stout/net.hpp`.

[Alex Clemmer]

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

(Updated Oct. 9, 2015, 6:30 p.m.)


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


Bugs: MESOS-3630 and MESOS-3631
https://issues.apache.org/jira/browse/MESOS-3630
https://issues.apache.org/jira/browse/MESOS-3631


Repository: mesos


Description
---

Windows: Added support for `stout/net.hpp`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
3f829bafe96526bc2263c9f228f85de38c435f60 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/net.hpp 
11e3895ee46e36faca0d2e1b436b61576826e472 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/net.hpp 
4f82796c2b9ef6a9198be145d969c5fce933be49 

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


Testing (updated)
---

Ran stout tests from VS.


Thanks,

Alex Clemmer

Review Request 39180: Windows: Added support for `stout/os/open.hpp`.

[Alex Clemmer]

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

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


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


Repository: mesos


Description
---

Windows: Added support for `stout/os/open.hpp`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/open.hpp 
43f261fd7a60b534f642f826ebf6ab18d72180c2 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/open.hpp 
023993d859e3a101ca387c1a514cd75de0d2beb1 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/open.hpp 
14fa11765c222cb4e80f5e45360d0f05facb578e 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
50e35f43d87c69a83a9e7d039d1881404ea8be38 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 39181: Windows: Added support for `stout/net.hpp`.

[Alex Clemmer]

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

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


Bugs: MESOS-3630 and MESOS-3631
https://issues.apache.org/jira/browse/MESOS-3630
https://issues.apache.org/jira/browse/MESOS-3631


Repository: mesos


Description
---

Windows: Added support for `stout/net.hpp`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
3f829bafe96526bc2263c9f228f85de38c435f60 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/net.hpp 
11e3895ee46e36faca0d2e1b436b61576826e472 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/net.hpp 
4f82796c2b9ef6a9198be145d969c5fce933be49 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 39182: Windows: Enable ip_tests.

[Alex Clemmer]

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

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


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


Repository: mesos


Description
---

Windows: Enable ip_tests.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/duration.hpp 
90551541f59889e96b21dbe1b65d3904850464c2 
  3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
d1e2df6151149e03ffb4a76e2c24ff78b891e016 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
50e35f43d87c69a83a9e7d039d1881404ea8be38 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
94292f8a46ec31bbaf6e52f48109322bbe123f70 

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


Testing
---


Thanks,

Alex Clemmer

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

[Greg Mann]

> On Oct. 9, 2015, 9:35 a.m., Guangya Liu wrote:
> > docs/attributes-resources.md, line 174
> > 
> >
> > Do you mind to update three to five here?

Nice catch!


- Greg


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


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

Re: Review Request 38627: Adds an overload of ModuleManager::create() allowing overriding parameters programatically

[Qian Zhang]

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

Ship it!


Ship It!

- Qian Zhang


On Oct. 3, 2015, 4:11 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38627/
> ---
> 
> (Updated Oct. 3, 2015, 4:11 a.m.)
> 
> 
> Review request for mesos, Adam B, Bernd Mathiske, Niklas Nielsen, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allows developers to provide their own parameters when loading modules 
> instead of using the ones provided by the user when loading Mesos. This helps 
> to deal with default modules (those used when the user doesn't provide any), 
> and for testing of the modules.
> 
> 
> Diffs
> -
> 
>   src/module/manager.hpp 302eb409fb8ef53b9cef8d2ecbe7b7f452b095ef 
>   src/tests/module.hpp 0820978441aede18dae6d1701433bff705b8c3c2 
> 
> Diff: https://reviews.apache.org/r/38627/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

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

[Greg Mann]

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

(Updated Oct. 9, 2015, 4:13 p.m.)


Review request for mesos, Adam B and Neil Conway.


Changes
---

Addressed comment.


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


Repository: mesos


Description
---

Added documentation for JSON resources.


Diffs (updated)
-

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

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


Testing
---

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


Thanks,

Greg Mann

Re: Review Request 39172: Add Blue Yonder to the powered by Mesos list.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39172]

All tests passed.

- Mesos ReviewBot


On Oct. 9, 2015, 1:42 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39172/
> ---
> 
> (Updated Oct. 9, 2015, 1:42 p.m.)
> 
> 
> Review request for mesos and Dave Lester.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add Blue Yonder to the powered by Mesos list.
> 
> 
> Diffs
> -
> 
>   docs/powered-by-mesos.md bbcac44570506b7e2bdd06ac0ab2204b0417314e 
> 
> Diff: https://reviews.apache.org/r/39172/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

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

[Benjamin Bannier]

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

(Updated Oct. 9, 2015, 2:24 p.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Changes
---

Widdened supported glibc versions.

strerror_r is broken on <=glibc-2.15 only if certain defines are set, so we can 
support more setups (the current patch compiles and checks e.g. under 
CentOS-5.11).


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


Repository: mesos


Description
---

This adds a thread-safe wrapper around strerror_r which has semantics similar 
to strerror. We plan to use this at call sites currently relying on strerror.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/strerror.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
e6d36ec1bf414b52d0899f0edf83e0ad8910dd0e 

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


Testing
---

make check


Thanks,

Benjamin Bannier

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

[Benjamin Bannier]

> On Oct. 8, 2015, 9:07 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/strerror.hpp, line 54
> > 
> >
> > We have to support CentOS 5 which has glibc 2.5 FWICT.

I widdened the requirements on the libc by more granular rejection of the buggy 
glibc setup. This should work now under e.g. CentOS-5.11.


- Benjamin


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


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

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

[Benjamin Bannier]

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

(Updated Oct. 9, 2015, 2:25 p.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Changes
---

Added another missing header.


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


Repository: mesos


Description
---

Switch call sites to using safe strerror_r wrapper.


Diffs (updated)
-

  src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
  src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
  src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
  src/linux/routing/link/internal.hpp 015c0ef5be516d7786c96a96437cced1ae8487fa 
  src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
  src/slave/containerizer/docker.cpp 6c975f904178e01797b67628a2d471ec7b3b1fbf 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
8823b7850a1ac17fc4f4868aadf1b04428d2381b 
  src/slave/containerizer/isolators/filesystem/posix.cpp 
eec510c4f7655d67b33ad90210eeb57fcc910684 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
  src/slave/containerizer/mesos/containerizer.cpp 
b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
  src/slave/containerizer/mesos/launch.cpp 
09d4d8f4d6837e93a82deef76ca07e2167d6a405 
  src/slave/containerizer/provisioner/backends/bind.cpp 
1fe1746c0bc1c9c12e1378e6438122a91b58316b 
  src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
  src/tests/containerizer/memory_test_helper.cpp 
8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
  src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 

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


Testing
---

make check


Thanks,

Benjamin Bannier

Review Request 39172: Add Blue Yonder to the powered by Mesos list.

[Stephan Erb]

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

Review request for mesos and Dave Lester.


Repository: mesos


Description
---

Add Blue Yonder to the powered by Mesos list.


Diffs
-

  docs/powered-by-mesos.md bbcac44570506b7e2bdd06ac0ab2204b0417314e 

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


Testing
---


Thanks,

Stephan Erb

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Oct. 9, 2015, 12:53 p.m.)


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


Changes
---

Fixes big bug of dlangling reference. Addresses till's review issues.


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 (updated)
-

  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 
591c1a959057155e1bf0f5bd73352e78d1c15cb3 
  3rdparty/libprocess/src/process.cpp d1c81f1d244f02bf42cab97198587ce1b8c7c407 

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


Testing
---

make check


Thanks,

Alexander Rojas

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

[Till Toenshoff]

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

Ship it!


Ship It!

- Till Toenshoff


On Oct. 8, 2015, 1 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39098/
> ---
> 
> (Updated Oct. 8, 2015, 1 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3560
> https://issues.apache.org/jira/browse/MESOS-3560
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When decoding the JSON credential file into the Credential protobuf object, 
> the 'secret' which is in 'bytes' is mapped into base64 string automatically 
> by protobuf from JSON. This creates an unintended behavior, forcing users to 
> encode in base64 their secret when wanting to pass a JSON file to the 
> --credentials flag.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 4a16be1 
>   src/examples/java/TestExceptionFramework.java 78720b0 
>   src/examples/java/TestFramework.java aad94c0 
> 
> Diff: https://reviews.apache.org/r/39098/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

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

[Guangya Liu]

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



docs/attributes-resources.md (line 174)


Do you mind to update three to five here?


- Guangya Liu


On 十月 8, 2015, 3:32 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39102/
> ---
> 
> (Updated 十月 8, 2015, 3:32 p.m.)
> 
> 
> Review request for mesos, Adam B and Neil Conway.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for JSON resources.
> 
> 
> Diffs
> -
> 
>   docs/attributes-resources.md f712d094f14426515dabde45f98d6c1ae36c3447 
>   docs/configuration.md 2ab476a2d2c43e309b570d73ecac80e27b296e7e 
> 
> Diff: https://reviews.apache.org/r/39102/diff/
> 
> 
> Testing
> ---
> 
> Viewed the relevant documentation sections ('Attributes and Resources' & 
> 'Configuration') using the mesos-website-container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 38883: Removed calls to apply-review.sh script. Added support for amending commit messages.

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 十月 8, 2015, 6:28 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38883/
> ---
> 
> (Updated 十月 8, 2015, 6:28 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, 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/38883/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

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/#review102036
---


Patch looks great!

Reviews applied: [38746]

All tests passed.

- Mesos ReviewBot


On Oct. 9, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38746/
> ---
> 
> (Updated Oct. 9, 2015, 1:28 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.
> 
> The following doc summarized the problem and proposed a solution:
> https://docs.google.com/document/d/1klGDAu5yBVf-CGWLqvELLIfxLfRaisGkhi6Gn7952-4/edit?usp=sharing
> 
> 
> 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 6c975f904178e01797b67628a2d471ec7b3b1fbf 
>   src/slave/containerizer/external_containerizer.cpp 
> 211649201777f0d2ce802a865090129eacdd53be 
>   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.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
>   src/tests/containerizer.cpp 1f743155526192569dd61a47ab67d8e58aab205a 
>   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
> 
> tested with Docker as well.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38943: Fixed misleading information in the Mesos framework writing guide.

[Adam B]

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

Ship it!


Minor nits/tweaks, but I'll fix those myself before I commit this.
Thanks for updating our docs!


docs/app-framework-development-guide.md (line 135)


s/shell commands/shell commands and Docker containers/



docs/app-framework-development-guide.md (line 137)


Unnecessary space (' ') before period.



docs/app-framework-development-guide.md (line 231)


or FTP!


- Adam B


On Oct. 1, 2015, 5:16 p.m., Connor Doyle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38943/
> ---
> 
> (Updated Oct. 1, 2015, 5:16 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The framework author's guide contained some misleading and incomplete 
> information.
> 
> Added description of the Mesos command executor.
> Fixed outdated information about how to use ExecutorInfo.
> Added a link to the Mesos fetcher user documentation.
> 
> 
> Diffs
> -
> 
>   docs/app-framework-development-guide.md c7aa08d 
> 
> Diff: https://reviews.apache.org/r/38943/diff/
> 
> 
> Testing
> ---
> 
> Visually verified the generated output in Chrome, using 
> http://github.com/mesosphere/mesos-website-container.
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>

Re: Review Request 39154: Log IP addresses together with failure messages.

[Mesos ReviewBot]

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


Bad review!

Reviews applied: []

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

- Mesos ReviewBot


On Oct. 9, 2015, 1:42 a.m., Brice Arnould wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39154/
> ---
> 
> (Updated Oct. 9, 2015, 1:42 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-1661
> https://issues.apache.org/jira/browse/MESOS-1661
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Log IP addresses together with failure messages.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/poll_socket.cpp 
> 28ed102972a9d8f88048aea4046ed837b6a25b35 
> 
> Diff: https://reviews.apache.org/r/39154/diff/
> 
> 
> Testing
> ---
> 
> Ran "make check"
> 
> 
> Thanks,
> 
> Brice Arnould
> 
>

Re: Review Request 39152: Pass LIBPROCESS_IP even when executor environment is specified.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39152]

All tests passed.

- Mesos ReviewBot


On Oct. 9, 2015, 12:57 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39152/
> ---
> 
> (Updated Oct. 9, 2015, 12:57 a.m.)
> 
> 
> Review request for mesos and Kapil Arya.
> 
> 
> Bugs: MESOS-3553
> https://issues.apache.org/jira/browse/MESOS-3553
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If DNS is not available on the agent node and a task is launched which 
> explicitly specifies the executor's environment, LIBPROCESS_IP will not be 
> passed through and the default hostname lookup after spawning the executor 
> process will throw an error. This patch alters the agent to always pass 
> LIBPROCESS_IP, even when the executor environment is specified.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> 25c87e9f948b7efe8b9a853c403bee69982d6c4c 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 5bc7d408bda0c249e1b66747d8bd87e688362e6c 
> 
> Diff: https://reviews.apache.org/r/39152/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>