Re: Review Request 61597: Fixed linking to `IPHlpAPI` library.

[Mesos Reviewbot Windows]

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



Bad review!

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

- Mesos Reviewbot Windows


On Aug. 11, 2017, 11:27 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61597/
> ---
> 
> (Updated Aug. 11, 2017, 11:27 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler and Joseph Wu.
> 
> 
> Bugs: MESOS-7704
> https://issues.apache.org/jira/browse/MESOS-7704
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the `#pragma comment(lib...)` method works with MSVC, it is
> inconsistent with how we link to the rest of the Windows libraries. So
> it was deleted and `IPHlpAPI` was added to stout's Windows dependencies.
> 
> Furthermore, the `` header was removed from the individual
> files and placed in `windows.hpp` to ensure it is included in the
> correct order with respect to the other Windows headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 76b2a67419cb6836b598a0070f26632d4ca048ff 
>   3rdparty/stout/include/stout/windows.hpp 
> 92db7278aea5cc9b94bec77071b9803f58042624 
>   3rdparty/stout/include/stout/windows/ip.hpp 
> 76f23c823662a54162e64160980512b191bb88e8 
>   3rdparty/stout/include/stout/windows/mac.hpp 
> 09c4c9626d135705a502b6d148f5b6ba64b688cd 
>   3rdparty/stout/include/stout/windows/net.hpp 
> 1418b5c981a2286c9ae390d801a8021e3a442f5b 
> 
> 
> Diff: https://reviews.apache.org/r/61597/diff/1/
> 
> 
> Testing
> ---
> 
> stout-tests on Windows
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 61597: Fixed linking to `IPHlpAPI` library.

[Mesos Reviewbot Windows]

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



Bad review!

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

- Mesos Reviewbot Windows


On Aug. 11, 2017, 11:27 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61597/
> ---
> 
> (Updated Aug. 11, 2017, 11:27 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler and Joseph Wu.
> 
> 
> Bugs: MESOS-7704
> https://issues.apache.org/jira/browse/MESOS-7704
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the `#pragma comment(lib...)` method works with MSVC, it is
> inconsistent with how we link to the rest of the Windows libraries. So
> it was deleted and `IPHlpAPI` was added to stout's Windows dependencies.
> 
> Furthermore, the `` header was removed from the individual
> files and placed in `windows.hpp` to ensure it is included in the
> correct order with respect to the other Windows headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 76b2a67419cb6836b598a0070f26632d4ca048ff 
>   3rdparty/stout/include/stout/windows.hpp 
> 92db7278aea5cc9b94bec77071b9803f58042624 
>   3rdparty/stout/include/stout/windows/ip.hpp 
> 76f23c823662a54162e64160980512b191bb88e8 
>   3rdparty/stout/include/stout/windows/mac.hpp 
> 09c4c9626d135705a502b6d148f5b6ba64b688cd 
>   3rdparty/stout/include/stout/windows/net.hpp 
> 1418b5c981a2286c9ae390d801a8021e3a442f5b 
> 
> 
> Diff: https://reviews.apache.org/r/61597/diff/1/
> 
> 
> Testing
> ---
> 
> stout-tests on Windows
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 61597: Fixed linking to `IPHlpAPI` library.

[Mesos Reviewbot Windows]

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



Bad review!

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

- Mesos Reviewbot Windows


On Aug. 11, 2017, 11:27 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61597/
> ---
> 
> (Updated Aug. 11, 2017, 11:27 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler and Joseph Wu.
> 
> 
> Bugs: MESOS-7704
> https://issues.apache.org/jira/browse/MESOS-7704
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the `#pragma comment(lib...)` method works with MSVC, it is
> inconsistent with how we link to the rest of the Windows libraries. So
> it was deleted and `IPHlpAPI` was added to stout's Windows dependencies.
> 
> Furthermore, the `` header was removed from the individual
> files and placed in `windows.hpp` to ensure it is included in the
> correct order with respect to the other Windows headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 76b2a67419cb6836b598a0070f26632d4ca048ff 
>   3rdparty/stout/include/stout/windows.hpp 
> 92db7278aea5cc9b94bec77071b9803f58042624 
>   3rdparty/stout/include/stout/windows/ip.hpp 
> 76f23c823662a54162e64160980512b191bb88e8 
>   3rdparty/stout/include/stout/windows/mac.hpp 
> 09c4c9626d135705a502b6d148f5b6ba64b688cd 
>   3rdparty/stout/include/stout/windows/net.hpp 
> 1418b5c981a2286c9ae390d801a8021e3a442f5b 
> 
> 
> Diff: https://reviews.apache.org/r/61597/diff/1/
> 
> 
> Testing
> ---
> 
> stout-tests on Windows
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 61597: Fixed linking to `IPHlpAPI` library.

[Mesos Reviewbot Windows]

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



Bad review!

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

- Mesos Reviewbot Windows


On Aug. 11, 2017, 11:27 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61597/
> ---
> 
> (Updated Aug. 11, 2017, 11:27 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler and Joseph Wu.
> 
> 
> Bugs: MESOS-7704
> https://issues.apache.org/jira/browse/MESOS-7704
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the `#pragma comment(lib...)` method works with MSVC, it is
> inconsistent with how we link to the rest of the Windows libraries. So
> it was deleted and `IPHlpAPI` was added to stout's Windows dependencies.
> 
> Furthermore, the `` header was removed from the individual
> files and placed in `windows.hpp` to ensure it is included in the
> correct order with respect to the other Windows headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 76b2a67419cb6836b598a0070f26632d4ca048ff 
>   3rdparty/stout/include/stout/windows.hpp 
> 92db7278aea5cc9b94bec77071b9803f58042624 
>   3rdparty/stout/include/stout/windows/ip.hpp 
> 76f23c823662a54162e64160980512b191bb88e8 
>   3rdparty/stout/include/stout/windows/mac.hpp 
> 09c4c9626d135705a502b6d148f5b6ba64b688cd 
>   3rdparty/stout/include/stout/windows/net.hpp 
> 1418b5c981a2286c9ae390d801a8021e3a442f5b 
> 
> 
> Diff: https://reviews.apache.org/r/61597/diff/1/
> 
> 
> Testing
> ---
> 
> stout-tests on Windows
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 61602: Fixed mesos containerizer to support docker image WORKDIR missing.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61602]

Logs available here: http://104.210.40.105/logs/master/61602

- Mesos Reviewbot Windows


On Aug. 12, 2017, 1:04 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61602/
> ---
> 
> (Updated Aug. 12, 2017, 1:04 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, and Qian Zhang.
> 
> 
> Bugs: MESOS-7652
> https://issues.apache.org/jira/browse/MESOS-7652
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some docker image may have 'WORKDIR' set in its manifest but that
> 'WORKDIR' does not exist in the image rootfs (e.g., the workdir
> is removed in the following dockerfile).
> 
> From the reference of dockerfile, "If the WORKDIR doesn’t exist,
> it will be created even if it’s not used in any subsequent
> Dockerfile instruction". So we should create the working directory
> if it does not exist in the image's rootfs.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 8e662931697a2f20e0ac1e80a2911b96f646b5af 
> 
> 
> Diff: https://reviews.apache.org/r/61602/diff/1/
> 
> 
> Testing
> ---
> 
> make
> 
> Manually tested using 'quay.io/spinnaker/front50:master' docker image.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 61493: Added a test `DefaultExecutorTest.MultiTaskgroupSharePidNamespace`.

[Gastón Kleiman]

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




src/tests/default_executor_tests.cpp
Lines 1436 (patched)


Please use:

`EXPECT_FALSE(offers1->offers().empty());`



src/tests/default_executor_tests.cpp
Lines 1451-1452 (patched)


What do you think about the following alternative?

`v1::TaskGroupInfo taskGroup = v1::createTaskGroupInfo({taskInfo})`



src/tests/default_executor_tests.cpp
Lines 1465-1481 (patched)


I think that the following is shorter and more readable:

```
v1::Offer::Operation launchGroup = v1::LAUNCH_GROUP(
executorInfo,
v1::createTaskGroupInfo({taskInfo}));

mesos.send(v1::createCallAccept(
frameworkId,
offer1,
{launchGroup}));
```



src/tests/default_executor_tests.cpp
Lines 1491 (patched)


Add

`EXPECT_FALSE(offers2->offers().empty());`



src/tests/default_executor_tests.cpp
Lines 1503-1504 (patched)


Ditto.



src/tests/default_executor_tests.cpp
Lines 1511-1529 (patched)


Ditto.



src/tests/default_executor_tests.cpp
Lines 1547 (patched)


Remove this extra line.


- Gastón Kleiman


On Aug. 8, 2017, 9:44 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61493/
> ---
> 
> (Updated Aug. 8, 2017, 9:44 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `DefaultExecutorTest.MultiTaskgroupSharePidNamespace`.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> b9776314a8781963b92ba9ac297654f61a443bc8 
> 
> 
> Diff: https://reviews.apache.org/r/61493/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61597: Fixed linking to `IPHlpAPI` library.

[Mesos Reviewbot Windows]

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



Bad review!

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

- Mesos Reviewbot Windows


On Aug. 11, 2017, 11:27 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61597/
> ---
> 
> (Updated Aug. 11, 2017, 11:27 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler and Joseph Wu.
> 
> 
> Bugs: MESOS-7704
> https://issues.apache.org/jira/browse/MESOS-7704
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the `#pragma comment(lib...)` method works with MSVC, it is
> inconsistent with how we link to the rest of the Windows libraries. So
> it was deleted and `IPHlpAPI` was added to stout's Windows dependencies.
> 
> Furthermore, the `` header was removed from the individual
> files and placed in `windows.hpp` to ensure it is included in the
> correct order with respect to the other Windows headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 76b2a67419cb6836b598a0070f26632d4ca048ff 
>   3rdparty/stout/include/stout/windows.hpp 
> 92db7278aea5cc9b94bec77071b9803f58042624 
>   3rdparty/stout/include/stout/windows/ip.hpp 
> 76f23c823662a54162e64160980512b191bb88e8 
>   3rdparty/stout/include/stout/windows/mac.hpp 
> 09c4c9626d135705a502b6d148f5b6ba64b688cd 
>   3rdparty/stout/include/stout/windows/net.hpp 
> 1418b5c981a2286c9ae390d801a8021e3a442f5b 
> 
> 
> Diff: https://reviews.apache.org/r/61597/diff/1/
> 
> 
> Testing
> ---
> 
> stout-tests on Windows
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 61546: Created staging dir only when needed.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61546]

Logs available here: http://104.210.40.105/logs/master/61546

- Mesos Reviewbot Windows


On Aug. 10, 2017, 9:28 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61546/
> ---
> 
> (Updated Aug. 10, 2017, 9:28 a.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-6950
> https://issues.apache.org/jira/browse/MESOS-6950
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created staging dir only when needed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 8058dcb31b9de59fa8d3638b553632235542 
> 
> 
> Diff: https://reviews.apache.org/r/61546/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60525: Fixed the default filter used by the allocator.

[Gastón Kleiman]

> On Aug. 11, 2017, 7:47 p.m., Greg Mann wrote:
> > include/mesos/mesos.proto
> > Line 2257 (original), 2258-2262 (patched)
> > 
> >
> > Is there a suitable place in the documentation for us to mention this?
> > 
> > Also, could you add an update for the changelog which calls out this 
> > change?

I added something to `docs/scheduler-http-api.md`, most other places that 
mention Filters tell users to look at the protos.


> On Aug. 11, 2017, 7:47 p.m., Greg Mann wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Line 1019 (original), 1019 (patched)
> > 
> >
> > s/timeout = timeout =/timeout =/

:scream: ;-)


> On Aug. 11, 2017, 7:47 p.m., Greg Mann wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Line 1170 (original), 1180 (patched)
> > 
> >
> > Ditto

Ohhh no, not again!


- Gastón


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


On Aug. 12, 2017, 1:07 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60525/
> ---
> 
> (Updated Aug. 12, 2017, 1:07 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7660
> https://issues.apache.org/jira/browse/MESOS-7660
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a framework accepts/refuses an offer using a very long filter, the
> `HierarchicalAllocator` will use the default filter instead. Meaning
> that it will filter the resources for only 5 seconds. This can happen
> when a framework sets `Filter::refuse_seconds` to a number of seconds
> larger than what fits in `Duration`.
> 
> This patch makes the hierarchical allocator cap the filter duration to
> at most 365 days.
> 
> 
> Diffs
> -
> 
>   docs/scheduler-http-api.md 4a5d77b88ae7cf0a0d8d39fe2579eb68bf33059a 
>   include/mesos/mesos.proto 50a5caf2623fff7d371753cbc6e842779fbf9769 
>   include/mesos/v1/mesos.proto 9d47a8a3a2f10fa493b4db35a73f33e6ee1ac727 
>   src/master/allocator/mesos/hierarchical.cpp 
> f021c34ef11aac42026ba39c5a1b775794982035 
> 
> 
> Diff: https://reviews.apache.org/r/60525/diff/7/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 60525: Fixed the default filter used by the allocator.

[Gastón Kleiman]

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

(Updated Aug. 12, 2017, 1:07 a.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, and 
Vinod Kone.


Changes
---

Fixed all the things!


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


Repository: mesos


Description
---

If a framework accepts/refuses an offer using a very long filter, the
`HierarchicalAllocator` will use the default filter instead. Meaning
that it will filter the resources for only 5 seconds. This can happen
when a framework sets `Filter::refuse_seconds` to a number of seconds
larger than what fits in `Duration`.

This patch makes the hierarchical allocator cap the filter duration to
at most 365 days.


Diffs (updated)
-

  docs/scheduler-http-api.md 4a5d77b88ae7cf0a0d8d39fe2579eb68bf33059a 
  include/mesos/mesos.proto 50a5caf2623fff7d371753cbc6e842779fbf9769 
  include/mesos/v1/mesos.proto 9d47a8a3a2f10fa493b4db35a73f33e6ee1ac727 
  src/master/allocator/mesos/hierarchical.cpp 
f021c34ef11aac42026ba39c5a1b775794982035 


Diff: https://reviews.apache.org/r/60525/diff/7/

Changes: https://reviews.apache.org/r/60525/diff/6-7/


Testing
---

`make check`


Thanks,

Gastón Kleiman

Review Request 61601: Stout: make boundary checking in Duration consistent.

[Gastón Kleiman]

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

Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, and 
Vinod Kone.


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


Repository: mesos


Description
---

Stout: make boundary checking in Duration consistent.


Diffs
-

  3rdparty/stout/include/stout/duration.hpp 
b0cd77b833f6fbf752b4db820fd43b87e1d1e476 


Diff: https://reviews.apache.org/r/61601/diff/1/


Testing
---

Stout tests still pass.


Thanks,

Gastón Kleiman

Review Request 61602: Fixed mesos containerizer to support docker image WORKDIR missing.

[Gilbert Song]

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

Review request for mesos, Jie Yu, Kevin Klues, and Qian Zhang.


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


Repository: mesos


Description
---

Some docker image may have 'WORKDIR' set in its manifest but that
'WORKDIR' does not exist in the image rootfs (e.g., the workdir
is removed in the following dockerfile).

>From the reference of dockerfile, "If the WORKDIR doesn’t exist,
it will be created even if it’s not used in any subsequent
Dockerfile instruction". So we should create the working directory
if it does not exist in the image's rootfs.


Diffs
-

  src/slave/containerizer/mesos/launch.cpp 
8e662931697a2f20e0ac1e80a2911b96f646b5af 


Diff: https://reviews.apache.org/r/61602/diff/1/


Testing
---

make

Manually tested using 'quay.io/spinnaker/front50:master' docker image.


Thanks,

Gilbert Song

Re: Review Request 60721: Stout: Made `Duration::parse()` handle durations out of range.

[Gastón Kleiman]

> On Aug. 11, 2017, 6:32 p.m., Greg Mann wrote:
> > 3rdparty/stout/tests/duration_tests.cpp
> > Lines 65 (patched)
> > 
> >
> > Why the newline?

woops, I did it again.


- Gastón


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


On Aug. 12, 2017, 1:02 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60721/
> ---
> 
> (Updated Aug. 12, 2017, 1:02 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7661
> https://issues.apache.org/jira/browse/MESOS-7661
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made `Duration:parse()` return an error if the argument is out of the
> range that a `Duration` can represent.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/duration.hpp 
> b0cd77b833f6fbf752b4db820fd43b87e1d1e476 
>   3rdparty/stout/tests/duration_tests.cpp 
> 59b08f14849a8db31f11fbd0b2e1248c99afd9dd 
> 
> 
> Diff: https://reviews.apache.org/r/60721/diff/5/
> 
> 
> Testing
> ---
> 
> Added a new expectation to an existing test and confirmed that tests still 
> pass.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 60721: Stout: Made `Duration::parse()` handle durations out of range.

[Gastón Kleiman]

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

(Updated Aug. 12, 2017, 1:02 a.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, and 
Vinod Kone.


Changes
---

Fixed whitespace.


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


Repository: mesos


Description
---

Made `Duration:parse()` return an error if the argument is out of the
range that a `Duration` can represent.


Diffs (updated)
-

  3rdparty/stout/include/stout/duration.hpp 
b0cd77b833f6fbf752b4db820fd43b87e1d1e476 
  3rdparty/stout/tests/duration_tests.cpp 
59b08f14849a8db31f11fbd0b2e1248c99afd9dd 


Diff: https://reviews.apache.org/r/60721/diff/5/

Changes: https://reviews.apache.org/r/60721/diff/4-5/


Testing
---

Added a new expectation to an existing test and confirmed that tests still pass.


Thanks,

Gastón Kleiman

Re: Review Request 61493: Added a test `DefaultExecutorTest.MultiTaskgroupSharePidNamespace`.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [61270, 61406, 61428, 61463, 61464, 61465, 61466, 61493]

Logs available here: http://104.210.40.105/logs/master/61493

- Mesos Reviewbot Windows


On Aug. 8, 2017, 9:44 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61493/
> ---
> 
> (Updated Aug. 8, 2017, 9:44 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `DefaultExecutorTest.MultiTaskgroupSharePidNamespace`.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> b9776314a8781963b92ba9ac297654f61a443bc8 
> 
> 
> Diff: https://reviews.apache.org/r/61493/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61511: Improved the readability of some assertions/expectations.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Aug. 10, 2017, 9:10 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61511/
> ---
> 
> (Updated Aug. 10, 2017, 9:10 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prefer checking whether a container is empty instead of checking its
> size.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 75f7a585ec75f1d8b78453dd203bb78822ee7bc9 
>   src/tests/check_tests.cpp 0810851cf1811f2b4f511f5ca49fe372fd0bac82 
>   src/tests/container_logger_tests.cpp 
> 2ebff809636fbf68c9e3fb87b98f168338f80d6d 
>   src/tests/containerizer/appc_spec_tests.cpp 
> 9bc82531dbb5f10b3d17f26609867c909d86816d 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> 67242119f2d10f6f3010c374ea58138e40e4a33e 
>   src/tests/containerizer/cgroups_tests.cpp 
> 506fc7f00dc1ac48476334de930b720b85a691dc 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 60c85adab11af06be474661544957ca20b7de8c3 
>   src/tests/containerizer/cpu_isolator_tests.cpp 
> 0e86318bb7e261ac00be19f6405557f29a2e92af 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> fa91bcf8d4fdcda44fb5607d99b86a6323096244 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> 866af61f8669163ff3ddd10ed7ecb655568f8250 
>   src/tests/containerizer/environment_secret_isolator_tests.cpp 
> b034ceec7d1bf92db8a1d344835ad48ea2d24952 
>   src/tests/containerizer/io_switchboard_tests.cpp 
> 742143a0f093e415fe98235bbd25342fd65e0483 
>   src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
> 6d95d607b81358953a4afcec03b60e87e7192edd 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> f9cab2fd123e48a5b4387fb2b609d70c7bf535cd 
>   src/tests/containerizer/memory_isolator_tests.cpp 
> b7b7acd37f69832e9ba79c9fe7ed0bc473b53e02 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> 8a43c4fada8bdcedc6e1e24833d0d6c94cf927d2 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 1fc56c4a8ca95bb47af968ad72dfef69455e7d46 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 84b39b15880c7b5d9b7967f1e686baa59f52a015 
>   src/tests/containerizer/posix_rlimits_isolator_tests.cpp 
> bf25049515e8b8d0c085be24c4de22b3ade261b2 
>   src/tests/containerizer/routing_tests.cpp 
> d05b3b11dc5fcd54d956990f252509865168e6b6 
>   src/tests/containerizer/runtime_isolator_tests.cpp 
> fbca31a4da1c83574cce7414fe5e03b1f86591cb 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> c220a6ba5b274892bee195b26a5069d0750b4cb2 
>   src/tests/default_executor_tests.cpp 
> b9776314a8781963b92ba9ac297654f61a443bc8 
>   src/tests/dynamic_weights_tests.cpp 
> 3c86325a0749112606683bffdf305661170493e5 
>   src/tests/exception_tests.cpp b5b657c2d1b31b59ab1554bf854ef1a69b1517ed 
>   src/tests/fault_tolerance_tests.cpp 
> 5b8213531f5688e94063937af19e674508f0dd8c 
>   src/tests/fetcher_cache_tests.cpp 6d212cd460322ad6e97f4cf6ef537323275b7da0 
>   src/tests/group_tests.cpp f2e37cc7184dd328466b554a16da3173cfbca4a7 
>   src/tests/health_check_tests.cpp e824d1b2c2b17423394da0b089ae8940450378b7 
>   src/tests/hook_tests.cpp 04edbf809b7289a7d5336e4f00f23dfb55d3dbf8 
>   src/tests/http_fault_tolerance_tests.cpp 
> 8fcd56d86dcbdd181864756187beb4ff2ac1ff2a 
>   src/tests/master_authorization_tests.cpp 
> 8d54472d8a31956e6e8bb5fe7ffbf47dc793c0bc 
>   src/tests/master_maintenance_tests.cpp 
> e7a80ff4199927df8bf0fb54458d357ae444260d 
>   src/tests/master_quota_tests.cpp 9d52f767c112abd55ab3f49d172eb6e3caea511b 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 9c31eeae1a1b67af142a01e6c548b509ba06740c 
>   src/tests/master_tests.cpp e6ed02e07a9860ca3a56bba9c502a09444c5b26e 
>   src/tests/master_validation_tests.cpp 
> 813fb25007973f3499b94dcd0e9d2184ba08634c 
>   src/tests/oversubscription_tests.cpp 
> 54535a35e33dde0db3b547b9e31b4545d2900b67 
>   src/tests/partition_tests.cpp 62a84f797201ccd18b71490949e3130d2b9c3668 
>   src/tests/persistent_volume_tests.cpp 
> 7b0f436aa270f64cde93adc58833eef97c73dc26 
>   src/tests/reconciliation_tests.cpp f1635fd627d9a7944cb9d4b0117fcded7b6693e1 
>   src/tests/registrar_tests.cpp e2c38d32bc5882b95bbcf1da2e849e4d5ec6a9af 
>   src/tests/registrar_zookeeper_tests.cpp 
> cc082d320b4b150717728ec48edd0b26169cecd7 
>   src/tests/resource_offers_tests.cpp 
> e1fcab4b8fbb625876cf246505db2d05ac5d5710 
>   src/tests/scheduler_driver_tests.cpp 
> 47ad1dc9a4e5e0c5444fcc3a90d4645c3b6013ca 
>   src/tests/scheduler_tests.cpp 5c42dc00f177a61d6ee595c26255492aa07aaad9 
>   src/tests/slave_authorization_tests.cpp 
> 

Re: Review Request 61597: Fixed linking to `IPHlpAPI` library.

[Mesos Reviewbot Windows]

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



Bad review!

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

- Mesos Reviewbot Windows


On Aug. 11, 2017, 11:27 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61597/
> ---
> 
> (Updated Aug. 11, 2017, 11:27 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler and Joseph Wu.
> 
> 
> Bugs: MESOS-7704
> https://issues.apache.org/jira/browse/MESOS-7704
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the `#pragma comment(lib...)` method works with MSVC, it is
> inconsistent with how we link to the rest of the Windows libraries. So
> it was deleted and `IPHlpAPI` was added to stout's Windows dependencies.
> 
> Furthermore, the `` header was removed from the individual
> files and placed in `windows.hpp` to ensure it is included in the
> correct order with respect to the other Windows headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 76b2a67419cb6836b598a0070f26632d4ca048ff 
>   3rdparty/stout/include/stout/windows.hpp 
> 92db7278aea5cc9b94bec77071b9803f58042624 
>   3rdparty/stout/include/stout/windows/ip.hpp 
> 76f23c823662a54162e64160980512b191bb88e8 
>   3rdparty/stout/include/stout/windows/mac.hpp 
> 09c4c9626d135705a502b6d148f5b6ba64b688cd 
>   3rdparty/stout/include/stout/windows/net.hpp 
> 1418b5c981a2286c9ae390d801a8021e3a442f5b 
> 
> 
> Diff: https://reviews.apache.org/r/61597/diff/1/
> 
> 
> Testing
> ---
> 
> stout-tests on Windows
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 61588: Added a `[-s|--skip-hooks]` option when applying reviews.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61588]

Logs available here: http://104.210.40.105/logs/master/61588

- Mesos Reviewbot Windows


On Aug. 11, 2017, 2:03 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61588/
> ---
> 
> (Updated Aug. 11, 2017, 2:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Kevin Klues.
> 
> 
> Bugs: MESOS-7880
> https://issues.apache.org/jira/browse/MESOS-7880
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a `[-s|--skip-hooks]` option when applying reviews.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py aa2eb8d23bed52a4f33510c53ace2c63bd66ec49 
> 
> 
> Diff: https://reviews.apache.org/r/61588/diff/3/
> 
> 
> Testing
> ---
> 
> Applied a review chain.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61597: Fixed linking to `IPHlpAPI` library.

[Andrew Schwartzmeyer]

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

(Updated Aug. 11, 2017, 4:27 p.m.)


Review request for mesos, Jeff Coffler and Joseph Wu.


Summary (updated)
-

Fixed linking to `IPHlpAPI` library.


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


Repository: mesos


Description
---

While the `#pragma comment(lib...)` method works with MSVC, it is
inconsistent with how we link to the rest of the Windows libraries. So
it was deleted and `IPHlpAPI` was added to stout's Windows dependencies.

Furthermore, the `` header was removed from the individual
files and placed in `windows.hpp` to ensure it is included in the
correct order with respect to the other Windows headers.


Diffs
-

  3rdparty/stout/CMakeLists.txt 76b2a67419cb6836b598a0070f26632d4ca048ff 
  3rdparty/stout/include/stout/windows.hpp 
92db7278aea5cc9b94bec77071b9803f58042624 
  3rdparty/stout/include/stout/windows/ip.hpp 
76f23c823662a54162e64160980512b191bb88e8 
  3rdparty/stout/include/stout/windows/mac.hpp 
09c4c9626d135705a502b6d148f5b6ba64b688cd 
  3rdparty/stout/include/stout/windows/net.hpp 
1418b5c981a2286c9ae390d801a8021e3a442f5b 


Diff: https://reviews.apache.org/r/61597/diff/1/


Testing
---

stout-tests on Windows


Thanks,

Andrew Schwartzmeyer

Re: Review Request 61597: Windows: Fixed linking to `IPHlpAPI` library.

[Andrew Schwartzmeyer]

> On Aug. 11, 2017, 2:45 p.m., Mesos Reviewbot Windows wrote:
> > Bad review!
> > 
> > Error:
> > No reviewers specified. Please find a reviewer by asking on JIRA or the 
> > mailing list.

Oh, I know why. There are some dependent reviews without a reviewer (as I used 
`support/post-reviews.py` which doesn't let you add reviewers when posting, and 
didn't particularly want to hand-edit 80+ reviews when Joe can find them all 
anyway).


- Andrew


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


On Aug. 11, 2017, 11:31 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61597/
> ---
> 
> (Updated Aug. 11, 2017, 11:31 a.m.)
> 
> 
> Review request for mesos, Jeff Coffler and Joseph Wu.
> 
> 
> Bugs: MESOS-7704
> https://issues.apache.org/jira/browse/MESOS-7704
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the `#pragma comment(lib...)` method works with MSVC, it is
> inconsistent with how we link to the rest of the Windows libraries. So
> it was deleted and `IPHlpAPI` was added to stout's Windows dependencies.
> 
> Furthermore, the `` header was removed from the individual
> files and placed in `windows.hpp` to ensure it is included in the
> correct order with respect to the other Windows headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 76b2a67419cb6836b598a0070f26632d4ca048ff 
>   3rdparty/stout/include/stout/windows.hpp 
> 92db7278aea5cc9b94bec77071b9803f58042624 
>   3rdparty/stout/include/stout/windows/ip.hpp 
> 76f23c823662a54162e64160980512b191bb88e8 
>   3rdparty/stout/include/stout/windows/mac.hpp 
> 09c4c9626d135705a502b6d148f5b6ba64b688cd 
>   3rdparty/stout/include/stout/windows/net.hpp 
> 1418b5c981a2286c9ae390d801a8021e3a442f5b 
> 
> 
> Diff: https://reviews.apache.org/r/61597/diff/1/
> 
> 
> Testing
> ---
> 
> stout-tests on Windows
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 61291: Imported `glog` library.

[Andrew Schwartzmeyer]

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

(Updated Aug. 11, 2017, 4:20 p.m.)


Review request for mesos.


Changes
---

Disallow linking to static library. Also brought over `GTEST_CONFIG=no` from 
Autotools.

Note that when we update Glog, and build with CMake, we'll be able to link to 
shared or static as needed.


Repository: mesos


Description
---

Review: https://reviews.apache.org/r/61291


Diffs (updated)
-

  3rdparty/CMakeLists.txt f4feaf90b4750338cfa2127524a5e6556a86d063 


Diff: https://reviews.apache.org/r/61291/diff/3/

Changes: https://reviews.apache.org/r/61291/diff/2-3/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 61565: Stout: Improved the readability of some assertions/expectations.

[Greg Mann]

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


Ship it!




Woohoo!! Thx Gaston!

- Greg Mann


On Aug. 10, 2017, 5:52 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61565/
> ---
> 
> (Updated Aug. 10, 2017, 5:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prefer checking whether a container is empty instead of checking its
> size.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/flags_tests.cpp 
> d78ab55b983ff6918733617ae8db5be29f817c69 
>   3rdparty/stout/tests/multimap_tests.cpp 
> 36860e43efab04cf8a2276523e6937c1918d5a90 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> 841f655a19e87657de77cb57c30b8310bb5898a4 
>   3rdparty/stout/tests/os/process_tests.cpp 
> 4b5c25782dd67624bf139ad826142183a6d6f1a2 
>   3rdparty/stout/tests/os_tests.cpp e8ecece1d5361e8d0e1ad42682db51b48ab6be1f 
>   3rdparty/stout/tests/proc_tests.cpp 
> c6d1d442df3e38d1d89716dc036b4a57ab0941b1 
>   3rdparty/stout/tests/strings_tests.cpp 
> a51144df652c5d456d8dab49ca8b2cbec69ea4b6 
> 
> 
> Diff: https://reviews.apache.org/r/61565/diff/2/
> 
> 
> Testing
> ---
> 
> The stout tests still pass.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61564: Libprocess: Improved the readability of some assertions/expectations.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Aug. 10, 2017, 5:51 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61564/
> ---
> 
> (Updated Aug. 10, 2017, 5:51 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prefer checking whether a container is empty instead of checking its
> size.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/decoder_tests.cpp 
> 5742c83c632a2f03b4935738c3e78f39edc33e6d 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> d71fa4b9619a5fb5b8b8cae3310c36aaefc878ae 
> 
> 
> Diff: https://reviews.apache.org/r/61564/diff/1/
> 
> 
> Testing
> ---
> 
> The libprocess tests still pass.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61565: Stout: Improved the readability of some assertions/expectations.

[Greg Mann]

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


Ship it!




Thanks for this, Gaston!!

- Greg Mann


On Aug. 10, 2017, 5:52 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61565/
> ---
> 
> (Updated Aug. 10, 2017, 5:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prefer checking whether a container is empty instead of checking its
> size.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/flags_tests.cpp 
> d78ab55b983ff6918733617ae8db5be29f817c69 
>   3rdparty/stout/tests/multimap_tests.cpp 
> 36860e43efab04cf8a2276523e6937c1918d5a90 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> 841f655a19e87657de77cb57c30b8310bb5898a4 
>   3rdparty/stout/tests/os/process_tests.cpp 
> 4b5c25782dd67624bf139ad826142183a6d6f1a2 
>   3rdparty/stout/tests/os_tests.cpp e8ecece1d5361e8d0e1ad42682db51b48ab6be1f 
>   3rdparty/stout/tests/proc_tests.cpp 
> c6d1d442df3e38d1d89716dc036b4a57ab0941b1 
>   3rdparty/stout/tests/strings_tests.cpp 
> a51144df652c5d456d8dab49ca8b2cbec69ea4b6 
> 
> 
> Diff: https://reviews.apache.org/r/61565/diff/2/
> 
> 
> Testing
> ---
> 
> The stout tests still pass.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61546: Created staging dir only when needed.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 10, 2017, 2:28 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61546/
> ---
> 
> (Updated Aug. 10, 2017, 2:28 a.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-6950
> https://issues.apache.org/jira/browse/MESOS-6950
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created staging dir only when needed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 8058dcb31b9de59fa8d3638b553632235542 
> 
> 
> Diff: https://reviews.apache.org/r/61546/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61493: Added a test `DefaultExecutorTest.MultiTaskgroupSharePidNamespace`.

[Gilbert Song]

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




src/tests/default_executor_tests.cpp
Lines 1580 (patched)


you may want to `strings::trim()`.


- Gilbert Song


On Aug. 8, 2017, 2:44 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61493/
> ---
> 
> (Updated Aug. 8, 2017, 2:44 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `DefaultExecutorTest.MultiTaskgroupSharePidNamespace`.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> b9776314a8781963b92ba9ac297654f61a443bc8 
> 
> 
> Diff: https://reviews.apache.org/r/61493/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61493: Added a test `DefaultExecutorTest.MultiTaskgroupSharePidNamespace`.

[Gilbert Song]

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




src/tests/default_executor_tests.cpp
Lines 1538-1542 (patched)


Could we save the executor sandbox to a var and call `getSandboxPath()`?



src/tests/default_executor_tests.cpp
Lines 1554 (patched)


we should not rely on the symlink to get the nested sandbox.



src/tests/default_executor_tests.cpp
Lines 1558-1568 (patched)


why do we need these? Could we do `AWAIT_TRUE()`?


- Gilbert Song


On Aug. 8, 2017, 2:44 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61493/
> ---
> 
> (Updated Aug. 8, 2017, 2:44 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `DefaultExecutorTest.MultiTaskgroupSharePidNamespace`.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> b9776314a8781963b92ba9ac297654f61a443bc8 
> 
> 
> Diff: https://reviews.apache.org/r/61493/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61466: Added the test `ROOT_CGROUPS_LaunchNestedSharePidNamespace`.

[Gilbert Song]

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


Fix it, then Ship it!





src/tests/containerizer/nested_mesos_containerizer_tests.cpp
Lines 879 (patched)


Could we follow the semantics in this file and use `containerid1`, 
`containerid2` etc.?



src/tests/containerizer/nested_mesos_containerizer_tests.cpp
Lines 934 (patched)


ditto to sandboxPath1, sandboxPath2..


- Gilbert Song


On Aug. 7, 2017, 8:14 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61466/
> ---
> 
> (Updated Aug. 7, 2017, 8:14 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the test `ROOT_CGROUPS_LaunchNestedSharePidNamespace`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> e46643434dc85d766bd549a037f36a89a6738678 
> 
> 
> Diff: https://reviews.apache.org/r/61466/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61465: Added test `NamespacesIsolatorTest.ROOT_SharePidNamespaceWhenDisallow`.

[Gilbert Song]

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


Fix it, then Ship it!





src/tests/containerizer/isolator_tests.cpp
Lines 73 (patched)


s/disallow/disallowSharingAgentPidNs/g ?


- Gilbert Song


On Aug. 7, 2017, 8:13 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61465/
> ---
> 
> (Updated Aug. 7, 2017, 8:13 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test `NamespacesIsolatorTest.ROOT_SharePidNamespaceWhenDisallow`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> f3c541c92f8ecc5c12356363bbbf3561d3b75230 
> 
> 
> Diff: https://reviews.apache.org/r/61465/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61464: Added a test `NamespacesIsolatorTest.ROOT_SharePidNamespace`.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 7, 2017, 8:13 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61464/
> ---
> 
> (Updated Aug. 7, 2017, 8:13 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `NamespacesIsolatorTest.ROOT_SharePidNamespace`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> f3c541c92f8ecc5c12356363bbbf3561d3b75230 
> 
> 
> Diff: https://reviews.apache.org/r/61464/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61464: Added a test `NamespacesIsolatorTest.ROOT_SharePidNamespace`.

[Gilbert Song]

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




src/tests/containerizer/isolator_tests.cpp
Lines 160-165 (patched)


I rephased a little bit:
```
// This test verifies a top-level container can share pid namespace
// with the agent when the field `share_pid_namespace` is set as
// 'true' in LinuxInfo. Please note that the agent flag
// `--disallow_sharing_agent_pid_namespace` is set to
// false by default, that means top-level container is allowed to share
// pid namespace with agent.
```


- Gilbert Song


On Aug. 7, 2017, 8:13 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61464/
> ---
> 
> (Updated Aug. 7, 2017, 8:13 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `NamespacesIsolatorTest.ROOT_SharePidNamespace`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> f3c541c92f8ecc5c12356363bbbf3561d3b75230 
> 
> 
> Diff: https://reviews.apache.org/r/61464/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61597: Windows: Fixed linking to `IPHlpAPI` library.

[Mesos Reviewbot Windows]

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



Bad review!

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

- Mesos Reviewbot Windows


On Aug. 11, 2017, 11:31 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61597/
> ---
> 
> (Updated Aug. 11, 2017, 11:31 a.m.)
> 
> 
> Review request for mesos, Jeff Coffler and Joseph Wu.
> 
> 
> Bugs: MESOS-7704
> https://issues.apache.org/jira/browse/MESOS-7704
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the `#pragma comment(lib...)` method works with MSVC, it is
> inconsistent with how we link to the rest of the Windows libraries. So
> it was deleted and `IPHlpAPI` was added to stout's Windows dependencies.
> 
> Furthermore, the `` header was removed from the individual
> files and placed in `windows.hpp` to ensure it is included in the
> correct order with respect to the other Windows headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 76b2a67419cb6836b598a0070f26632d4ca048ff 
>   3rdparty/stout/include/stout/windows.hpp 
> 92db7278aea5cc9b94bec77071b9803f58042624 
>   3rdparty/stout/include/stout/windows/ip.hpp 
> 76f23c823662a54162e64160980512b191bb88e8 
>   3rdparty/stout/include/stout/windows/mac.hpp 
> 09c4c9626d135705a502b6d148f5b6ba64b688cd 
>   3rdparty/stout/include/stout/windows/net.hpp 
> 1418b5c981a2286c9ae390d801a8021e3a442f5b 
> 
> 
> Diff: https://reviews.apache.org/r/61597/diff/1/
> 
> 
> Testing
> ---
> 
> stout-tests on Windows
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 61588: Added a `[-s|--skip-hooks]` option when applying reviews.

[Chun-Hung Hsiao]

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

(Updated Aug. 11, 2017, 9:03 p.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, and Kevin Klues.


Changes
---

Addressed @bbannier's comments and made the program pass the style check.


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


Repository: mesos


Description
---

Added a `[-s|--skip-hooks]` option when applying reviews.


Diffs (updated)
-

  support/apply-reviews.py aa2eb8d23bed52a4f33510c53ace2c63bd66ec49 


Diff: https://reviews.apache.org/r/61588/diff/3/

Changes: https://reviews.apache.org/r/61588/diff/2-3/


Testing (updated)
---

Applied a review chain.


Thanks,

Chun-Hung Hsiao

Re: Review Request 61597: Windows: Fixed linking to `IPHlpAPI` library.

[Mesos Reviewbot Windows]

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



Bad review!

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

- Mesos Reviewbot Windows


On Aug. 11, 2017, 11:31 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61597/
> ---
> 
> (Updated Aug. 11, 2017, 11:31 a.m.)
> 
> 
> Review request for mesos, Jeff Coffler and Joseph Wu.
> 
> 
> Bugs: MESOS-7704
> https://issues.apache.org/jira/browse/MESOS-7704
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the `#pragma comment(lib...)` method works with MSVC, it is
> inconsistent with how we link to the rest of the Windows libraries. So
> it was deleted and `IPHlpAPI` was added to stout's Windows dependencies.
> 
> Furthermore, the `` header was removed from the individual
> files and placed in `windows.hpp` to ensure it is included in the
> correct order with respect to the other Windows headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 76b2a67419cb6836b598a0070f26632d4ca048ff 
>   3rdparty/stout/include/stout/windows.hpp 
> 92db7278aea5cc9b94bec77071b9803f58042624 
>   3rdparty/stout/include/stout/windows/ip.hpp 
> 76f23c823662a54162e64160980512b191bb88e8 
>   3rdparty/stout/include/stout/windows/mac.hpp 
> 09c4c9626d135705a502b6d148f5b6ba64b688cd 
>   3rdparty/stout/include/stout/windows/net.hpp 
> 1418b5c981a2286c9ae390d801a8021e3a442f5b 
> 
> 
> Diff: https://reviews.apache.org/r/61597/diff/1/
> 
> 
> Testing
> ---
> 
> stout-tests on Windows
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 61262: Added 'heartbeat' event for the operator API.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61262]

Logs available here: http://104.210.40.105/logs/master/61262

- Mesos Reviewbot Windows


On July 31, 2017, 10:30 a.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61262/
> ---
> 
> (Updated July 31, 2017, 10:30 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Greg Mann.
> 
> 
> Bugs: MESOS-7695
> https://issues.apache.org/jira/browse/MESOS-7695
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the 'HEARTBEAT' event for the operator API, modified other
> related test cases to accept heartbeats.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 0e4c30e3df704a929a3bd2e1787a76421d14a983 
>   include/mesos/v1/master/master.proto 
> c04fd1602396a58086331f5fa56518c5dee9af89 
>   src/master/http.cpp 9df086c417a9392f62d600c7a6486be0a1cf7e70 
>   src/master/master.hpp 84465af782d4024f22463d981ef9d0ef7827d043 
>   src/master/master.cpp 7bb3adf48431cd97b3c404b8a1eba4ac6a01efcc 
>   src/tests/api_tests.cpp 1d5b080c809248bdf4c76ddad382d714692c804b 
> 
> 
> Diff: https://reviews.apache.org/r/61262/diff/3/
> 
> 
> Testing
> ---
> 
> make check -j48
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61588: Added a `[-s|--skip-hooks]` option when applying reviews.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61588]

Logs available here: http://104.210.40.105/logs/master/61588

- Mesos Reviewbot Windows


On Aug. 11, 2017, 6:45 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61588/
> ---
> 
> (Updated Aug. 11, 2017, 6:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Kevin Klues.
> 
> 
> Bugs: MESOS-7880
> https://issues.apache.org/jira/browse/MESOS-7880
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a `[-s|--skip-hooks]` option when applying reviews.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py aa2eb8d23bed52a4f33510c53ace2c63bd66ec49 
> 
> 
> Diff: https://reviews.apache.org/r/61588/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 60525: Fixed the default filter used by the allocator.

[Greg Mann]

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




include/mesos/mesos.proto
Line 2257 (original), 2258-2262 (patched)


Is there a suitable place in the documentation for us to mention this?

Also, could you add an update for the changelog which calls out this change?



src/master/allocator/mesos/hierarchical.cpp
Line 1019 (original), 1019 (patched)


s/timeout = timeout =/timeout =/



src/master/allocator/mesos/hierarchical.cpp
Lines 1022 (patched)


s/filters.get()./filters->/

here and elsewhere



src/master/allocator/mesos/hierarchical.cpp
Line 1021 (original), 1026 (patched)


Can you just do

```
timeout = Days(365);
```



src/master/allocator/mesos/hierarchical.cpp
Line 1170 (original), 1180 (patched)


Ditto



src/master/allocator/mesos/hierarchical.cpp
Lines 1187 (patched)


Ditto



src/master/allocator/mesos/hierarchical.cpp
Lines 1190 (patched)


Could you make this language consistent here and below? This line uses 
"refused resources offer filter", while the next one uses "refused resources 
filter".


- Greg Mann


On July 26, 2017, 11:30 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60525/
> ---
> 
> (Updated July 26, 2017, 11:30 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7660
> https://issues.apache.org/jira/browse/MESOS-7660
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a framework accepts/refuses an offer using a very long filter, the
> `HierarchicalAllocator` will use the default filter instead. Meaning
> that it will filter the resources for only 5 seconds. This can happen
> when a framework sets `Filter::refuse_seconds` to a number of seconds
> larger than what fits in `Duration`.
> 
> This patch makes the hierarchical allocator cap the filter duration to
> at most 365 days.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
>   include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
>   src/master/allocator/mesos/hierarchical.cpp 
> f021c34ef11aac42026ba39c5a1b775794982035 
> 
> 
> Diff: https://reviews.apache.org/r/60525/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61588: Added a `[-s|--skip-hooks]` option when applying reviews.

[Benjamin Bannier]

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


Fix it, then Ship it!




Thanks for the patch. I am sure this will be very useful for e.g., reviewing 
patches currently not passing the our linters.


support/apply-reviews.py
Lines 263-266 (original), 269-273 (patched)


This line extends beyond 80 chars. Let's wrap it to not undo already done 
work for MESOS-6390, e.g.,

cmd = u"git commit "\
"--author \"{author}\" "\
"{amend} -aF \"{message}\" {verify}".format(
author=quote(data['author']),
amend=amend,
message=message_file,
verify=verify)

(We seem to prefer C-style continuations to the more usual triple quotes).

Note how the args to `format` are indented by 4 spaces relative to the 
wrapped string.

To check if the format passes the still not enabled linting of support 
Python scripts, add `support` to `PyLinter`'s `source_dirs` in 
`support/mesos-style.py`, and run it on the file.


- Benjamin Bannier


On Aug. 11, 2017, 8:45 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61588/
> ---
> 
> (Updated Aug. 11, 2017, 8:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Kevin Klues.
> 
> 
> Bugs: MESOS-7880
> https://issues.apache.org/jira/browse/MESOS-7880
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a `[-s|--skip-hooks]` option when applying reviews.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py aa2eb8d23bed52a4f33510c53ace2c63bd66ec49 
> 
> 
> Diff: https://reviews.apache.org/r/61588/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61463: Fixed a bug in the test `NamespacesIsolatorTest`.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 8, 2017, 2:43 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61463/
> ---
> 
> (Updated Aug. 8, 2017, 2:43 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this test, we used the command `stat -c %i` to get the current
> process's pid/ipc namespace inode, that is not correct since
> `/proc/self/ns/pid` and `/proc/self/ns/ipc` are symbol links, we
> should get the inode that the link references rather than the link
> itself. So we need to add a `-L` option in the `stat` command.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> f3c541c92f8ecc5c12356363bbbf3561d3b75230 
> 
> 
> Diff: https://reviews.apache.org/r/61463/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61428: Added pid ns sharing based on agent flag and protobuf message field.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 8, 2017, 2:40 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61428/
> ---
> 
> (Updated Aug. 8, 2017, 2:40 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added pid ns sharing based on agent flag and protobuf message field.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
> 2b316dbdf4a3735771af5bed80c6251d0d1cbd50 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
> f1dfc9f7398ffc029d7180d7f014a515338cb3f4 
> 
> 
> Diff: https://reviews.apache.org/r/61428/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61597: Windows: Fixed linking to `IPHlpAPI` library.

[Andrew Schwartzmeyer]

> On Aug. 11, 2017, 12:09 p.m., Mesos Reviewbot Windows wrote:
> > Bad review!
> > 
> > Error:
> > No reviewers specified. Please find a reviewer by asking on JIRA or the 
> > mailing list.

Weird, it had reviewers before it was posted.


- Andrew


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


On Aug. 11, 2017, 11:31 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61597/
> ---
> 
> (Updated Aug. 11, 2017, 11:31 a.m.)
> 
> 
> Review request for mesos, Jeff Coffler and Joseph Wu.
> 
> 
> Bugs: MESOS-7704
> https://issues.apache.org/jira/browse/MESOS-7704
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the `#pragma comment(lib...)` method works with MSVC, it is
> inconsistent with how we link to the rest of the Windows libraries. So
> it was deleted and `IPHlpAPI` was added to stout's Windows dependencies.
> 
> Furthermore, the `` header was removed from the individual
> files and placed in `windows.hpp` to ensure it is included in the
> correct order with respect to the other Windows headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 76b2a67419cb6836b598a0070f26632d4ca048ff 
>   3rdparty/stout/include/stout/windows.hpp 
> 92db7278aea5cc9b94bec77071b9803f58042624 
>   3rdparty/stout/include/stout/windows/ip.hpp 
> 76f23c823662a54162e64160980512b191bb88e8 
>   3rdparty/stout/include/stout/windows/mac.hpp 
> 09c4c9626d135705a502b6d148f5b6ba64b688cd 
>   3rdparty/stout/include/stout/windows/net.hpp 
> 1418b5c981a2286c9ae390d801a8021e3a442f5b 
> 
> 
> Diff: https://reviews.apache.org/r/61597/diff/1/
> 
> 
> Testing
> ---
> 
> stout-tests on Windows
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 61262: Added 'heartbeat' event for the operator API.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61262]

Logs available here: http://104.210.40.105/logs/master/61262

- Mesos Reviewbot Windows


On July 31, 2017, 5:30 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61262/
> ---
> 
> (Updated July 31, 2017, 5:30 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Greg Mann.
> 
> 
> Bugs: MESOS-7695
> https://issues.apache.org/jira/browse/MESOS-7695
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the 'HEARTBEAT' event for the operator API, modified other
> related test cases to accept heartbeats.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 0e4c30e3df704a929a3bd2e1787a76421d14a983 
>   include/mesos/v1/master/master.proto 
> c04fd1602396a58086331f5fa56518c5dee9af89 
>   src/master/http.cpp 9df086c417a9392f62d600c7a6486be0a1cf7e70 
>   src/master/master.hpp 84465af782d4024f22463d981ef9d0ef7827d043 
>   src/master/master.cpp 7bb3adf48431cd97b3c404b8a1eba4ac6a01efcc 
>   src/tests/api_tests.cpp 1d5b080c809248bdf4c76ddad382d714692c804b 
> 
> 
> Diff: https://reviews.apache.org/r/61262/diff/3/
> 
> 
> Testing
> ---
> 
> make check -j48
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61597: Windows: Fixed linking to `IPHlpAPI` library.

[Mesos Reviewbot Windows]

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



Bad review!

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

- Mesos Reviewbot Windows


On Aug. 11, 2017, 6:31 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61597/
> ---
> 
> (Updated Aug. 11, 2017, 6:31 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler and Joseph Wu.
> 
> 
> Bugs: MESOS-7704
> https://issues.apache.org/jira/browse/MESOS-7704
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the `#pragma comment(lib...)` method works with MSVC, it is
> inconsistent with how we link to the rest of the Windows libraries. So
> it was deleted and `IPHlpAPI` was added to stout's Windows dependencies.
> 
> Furthermore, the `` header was removed from the individual
> files and placed in `windows.hpp` to ensure it is included in the
> correct order with respect to the other Windows headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 76b2a67419cb6836b598a0070f26632d4ca048ff 
>   3rdparty/stout/include/stout/windows.hpp 
> 92db7278aea5cc9b94bec77071b9803f58042624 
>   3rdparty/stout/include/stout/windows/ip.hpp 
> 76f23c823662a54162e64160980512b191bb88e8 
>   3rdparty/stout/include/stout/windows/mac.hpp 
> 09c4c9626d135705a502b6d148f5b6ba64b688cd 
>   3rdparty/stout/include/stout/windows/net.hpp 
> 1418b5c981a2286c9ae390d801a8021e3a442f5b 
> 
> 
> Diff: https://reviews.apache.org/r/61597/diff/1/
> 
> 
> Testing
> ---
> 
> stout-tests on Windows
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 61270: Added container PID namespace control protobuf field in LinuxInfo.

[Gilbert Song]

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

(Updated Aug. 11, 2017, 11:52 a.m.)


Review request for mesos, Avinash sridharan, Gastón Kleiman, Jie Yu, Kevin 
Klues, Qian Zhang, and Vinod Kone.


Repository: mesos


Description
---

User facing interface for container configurable PID namespace.


Diffs (updated)
-

  include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
  include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 


Diff: https://reviews.apache.org/r/61270/diff/4/

Changes: https://reviews.apache.org/r/61270/diff/3-4/


Testing
---

make


Thanks,

Gilbert Song

Re: Review Request 61588: Added a `[-s|--skip-hooks]` option when applying reviews.

[Chun-Hung Hsiao]

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

(Updated Aug. 11, 2017, 6:45 p.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, and Kevin Klues.


Changes
---

Addressed @bbannier's comments.


Summary (updated)
-

Added a `[-s|--skip-hooks]` option when applying reviews.


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


Repository: mesos


Description (updated)
---

Added a `[-s|--skip-hooks]` option when applying reviews.


Diffs (updated)
-

  support/apply-reviews.py aa2eb8d23bed52a4f33510c53ace2c63bd66ec49 


Diff: https://reviews.apache.org/r/61588/diff/2/

Changes: https://reviews.apache.org/r/61588/diff/1-2/


Testing
---


Thanks,

Chun-Hung Hsiao

Re: Review Request 60721: Stout: Made `Duration::parse()` handle durations out of range.

[Greg Mann]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/duration.hpp
Lines 402-403 (original), 413-414 (patched)


Could you follow up with a patch to make our boundary checking consistent? 
i.e., we could use `min().nanos` and `max().nanos` here.



3rdparty/stout/tests/duration_tests.cpp
Lines 65 (patched)


Why the newline?


- Greg Mann


On July 26, 2017, 11:12 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60721/
> ---
> 
> (Updated July 26, 2017, 11:12 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7661
> https://issues.apache.org/jira/browse/MESOS-7661
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made `Duration:parse()` return an error if the argument is out of the
> range that a `Duration` can represent.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/duration.hpp 
> b0cd77b833f6fbf752b4db820fd43b87e1d1e476 
>   3rdparty/stout/tests/duration_tests.cpp 
> 59b08f14849a8db31f11fbd0b2e1248c99afd9dd 
> 
> 
> Diff: https://reviews.apache.org/r/60721/diff/4/
> 
> 
> Testing
> ---
> 
> Added a new expectation to an existing test and confirmed that tests still 
> pass.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Review Request 61597: Windows: Fixed linking to `IPHlpAPI` library.

[Andrew Schwartzmeyer]

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

Review request for mesos, Jeff Coffler and Joseph Wu.


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


Repository: mesos


Description
---

While the `#pragma comment(lib...)` method works with MSVC, it is
inconsistent with how we link to the rest of the Windows libraries. So
it was deleted and `IPHlpAPI` was added to stout's Windows dependencies.

Furthermore, the `` header was removed from the individual
files and placed in `windows.hpp` to ensure it is included in the
correct order with respect to the other Windows headers.


Diffs
-

  3rdparty/stout/CMakeLists.txt 76b2a67419cb6836b598a0070f26632d4ca048ff 
  3rdparty/stout/include/stout/windows.hpp 
92db7278aea5cc9b94bec77071b9803f58042624 
  3rdparty/stout/include/stout/windows/ip.hpp 
76f23c823662a54162e64160980512b191bb88e8 
  3rdparty/stout/include/stout/windows/mac.hpp 
09c4c9626d135705a502b6d148f5b6ba64b688cd 
  3rdparty/stout/include/stout/windows/net.hpp 
1418b5c981a2286c9ae390d801a8021e3a442f5b 


Diff: https://reviews.apache.org/r/61597/diff/1/


Testing
---

stout-tests on Windows


Thanks,

Andrew Schwartzmeyer

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Greg Mann]

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




docs/health-checks.md
Line 29 (original), 29 (patched)


s/defines/which defines a/



docs/health-checks.md
Lines 36 (patched)


s/task's/tasks'/



docs/health-checks.md
Lines 74 (patched)


s/certain condition/a certain condition/



docs/health-checks.md
Lines 75 (patched)


s/of specific/of a specific/



docs/health-checks.md
Lines 80-83 (patched)


I can't figure out precisely what is being said here; could you try to 
re-word?



docs/health-checks.md
Lines 85-86 (patched)


What do you mean by "only the executor knows how to interpret `TaskInfo`"?

I might replace with something like:

"It is the responsibility of the executor to interpret `CheckInfo` and 
`HealthCheckInfo` and perform checks appropriately."



docs/health-checks.md
Lines 86 (patched)


s/only executor/only the executor/



docs/health-checks.md
Lines 97 (patched)


s/checking all/checking that all/



docs/health-checks.md
Lines 102 (patched)


s/it/them/



docs/health-checks.md
Lines 109-112 (patched)


Could you make the punctuation at the ends of these lines consistent?



docs/health-checks.md
Lines 110-112 (patched)


Forgot to increment numbers: 2, 3



docs/health-checks.md
Lines 118 (patched)


s/internal task's/the task's internal/



docs/health-checks.md
Lines 135 (patched)


s/given HTTP/given that HTTP/



docs/health-checks.md
Lines 143 (patched)


s/in the future/in the future the/



docs/health-checks.md
Lines 157 (patched)


s/As/As the/



docs/health-checks.md
Lines 159 (patched)


s/as few as possible updates/as few updates as possible/



docs/health-checks.md
Lines 272-278 (patched)


Should we also call out here that setting interval_seconds to zero is a 
really bad idea?



docs/health-checks.md
Line 219 (original), 459 (patched)


s/during first/during the first/

s/response time/response times/



docs/health-checks.md
Lines 280-281 (original), 522-523 (patched)


I might recommend:

"Regardless of executor, all checks and health checks consume resources 
from the task's resource allocation."



docs/health-checks.md
Line 281 (original), 523 (patched)


s/towards task's/towards the task's/



docs/health-checks.md
Lines 539-541 (patched)


I might recommend:

"Due to its short-polling nature, a check whose state oscillates repeatedly 
may lead to scalability issues due to a high volume of task status updates."


- Greg Mann


On Aug. 11, 2017, 12:12 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 11, 2017, 12:12 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/2/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61261: Documented agent garbage collection metrics.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [61259, 61260, 61261]

Logs available here: http://104.210.40.105/logs/master/61261

- Mesos Reviewbot Windows


On July 31, 2017, 7:31 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61261/
> ---
> 
> (Updated July 31, 2017, 7:31 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7842
> https://issues.apache.org/jira/browse/MESOS-7842
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented the following agent sandbox garbage collection metrics:
>   - slave/gc_path_removals_failed
>   - slave/gc_path_removals_pending
>   - slave/gc_path_removals_succeeded
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md d1f64d46b22e79c16a680a2e7fff9a01202c5457 
> 
> 
> Diff: https://reviews.apache.org/r/61261/diff/2/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61434]

Logs available here: http://104.210.40.105/logs/master/61434

- Mesos Reviewbot Windows


On Aug. 11, 2017, 12:12 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 11, 2017, 12:12 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/2/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61408: Added test cases for V1 teardown Call.

[Greg Mann]

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




src/tests/api_tests.cpp
Lines 2905-2906 (patched)


How about:

"This test verifies that when the operator API TEARDOWN call is made, the 
framework is shutdown and removed. It also confirms that authorization of this 
call is performed correctly."



src/tests/api_tests.cpp
Lines 2911-2931 (patched)


If you set the `permissive` bit to `true`, then I think we can get rid of 
these first 3 ACLs, which would improve readability a little. Then you could 
explicitly set an ACL for DEFAULT_CREDENTIAL_2.



src/tests/api_tests.cpp
Lines 2949-2953 (patched)


Since the containerizer is created to be injected into the agent, let's 
have these two blocks switch places:

```
  Future registerSlaveMessage =
FUTURE_PROTOBUF(RegisterSlaveMessage(), _, _);

  ExecutorID executorId = DEFAULT_EXECUTOR_ID;
  TestContainerizer containerizer(executorId, executor);
```



src/tests/api_tests.cpp
Lines 2964 (patched)


Is this necessary?



src/tests/api_tests.cpp
Lines 3024-3025 (patched)


Fits on one line.



src/tests/api_tests.cpp
Lines 3070-3071 (patched)


Fits on one line.


- Greg Mann


On Aug. 10, 2017, 10:47 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61408/
> ---
> 
> (Updated Aug. 10, 2017, 10:47 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Greg Mann.
> 
> 
> Bugs: MESOS-6846
> https://issues.apache.org/jira/browse/MESOS-6846
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for teardown operation in V1 operator API.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 75f7a585ec75f1d8b78453dd203bb78822ee7bc9 
> 
> 
> Diff: https://reviews.apache.org/r/61408/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61588: Added a `--skip-style-check` option when applying reviews.

[Benjamin Bannier]

> On Aug. 11, 2017, 11:02 a.m., Benjamin Bannier wrote:
> > support/apply-reviews.py
> > Line 364 (original), 371-373 (patched)
> > 
> >
> > Since this prevents running any (pre-)commit hooks, I wonder if we 
> > should give this a more general name, e.g., simply expose 
> > `[-n|--no-verify]`.
> 
> Chun-Hung Hsiao wrote:
> If we use `--no-verify` a the long option, then the short option `-n` 
> would conflict with `--no-amend`, and it seems to me that we should not 
> change what's already there. And `-v` looks like `--verbose` and also not 
> obvious to see it's *not* doing a verification. Keviv and I looked through 
> the keyboard and came up with this one.

Good point about collisions. What about e.g., `[-s|--skip-commit-hooks]` or 
even `[-s|--skip-hooks]`? If I had a precommit hook invoking `clang-format` 
this flag would also disable it.


- Benjamin


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


On Aug. 11, 2017, 1:41 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61588/
> ---
> 
> (Updated Aug. 11, 2017, 1:41 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Kevin Klues.
> 
> 
> Bugs: MESOS-7880
> https://issues.apache.org/jira/browse/MESOS-7880
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a `--skip-style-check` option when applying reviews.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py aa2eb8d23bed52a4f33510c53ace2c63bd66ec49 
> 
> 
> Diff: https://reviews.apache.org/r/61588/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61588: Added a `--skip-style-check` option when applying reviews.

[Chun-Hung Hsiao]

> On Aug. 11, 2017, 9:02 a.m., Benjamin Bannier wrote:
> > support/apply-reviews.py
> > Line 364 (original), 371-373 (patched)
> > 
> >
> > Since this prevents running any (pre-)commit hooks, I wonder if we 
> > should give this a more general name, e.g., simply expose 
> > `[-n|--no-verify]`.

If we use `--no-verify` a the long option, then the short option `-n` would 
conflict with `--no-amend`, and it seems to me that we should not change what's 
already there. And `-v` looks like `--verbose` and also not obvious to see it's 
*not* doing a verification. Keviv and I looked through the keyboard and came up 
with this one.


- Chun-Hung


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


On Aug. 10, 2017, 11:41 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61588/
> ---
> 
> (Updated Aug. 10, 2017, 11:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Kevin Klues.
> 
> 
> Bugs: MESOS-7880
> https://issues.apache.org/jira/browse/MESOS-7880
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a `--skip-style-check` option when applying reviews.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py aa2eb8d23bed52a4f33510c53ace2c63bd66ec49 
> 
> 
> Diff: https://reviews.apache.org/r/61588/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61580: Extracted JNI code into a protected function for clarity.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61579, 61580]

Logs available here: http://104.210.40.105/logs/master/61580

- Mesos Reviewbot Windows


On Aug. 10, 2017, 8:41 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61580/
> ---
> 
> (Updated Aug. 10, 2017, 8:41 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-7872
> https://issues.apache.org/jira/browse/MESOS-7872
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 
> 1f58fbff4e8414e4d2ae4c8f69b637ee3315e411 
> 
> 
> Diff: https://reviews.apache.org/r/61580/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61118: WIP: Building gRPC support in libprocess with CMake.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61488, 61489, 61508, 61509, 61095, 61576, 61097, 61098, 
61433, 61517, 61159, 61160, 61096, 61118]

Logs available here: http://104.210.40.105/logs/master/61118

- Mesos Reviewbot Windows


On Aug. 4, 2017, 11:21 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61118/
> ---
> 
> (Updated Aug. 4, 2017, 11:21 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-7810
> https://issues.apache.org/jira/browse/MESOS-7810
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch enables CMake to build code for gRPC support in libprocess
> along with tests under Linux.
> 
> TODO(chhsiao): gRPC support for Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/CMakeLists.txt 
> f97291bd7cadcb440231619a8a2d1029a447ec27 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> 27451c275b1f5a2eb7ada78f8cbe4b7c2c63ca92 
> 
> 
> Diff: https://reviews.apache.org/r/61118/diff/3/
> 
> 
> Testing
> ---
> 
> make
> sudo make check
> cd 3rdparty/libprocess/src/tests && make libprocess-tests && 
> ./libprocess-tests
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61531: Fixed the device number proto 'major' and 'minor' to avoid MACROs.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61531]

Logs available here: http://104.210.40.105/logs/master/61531

- Mesos Reviewbot Windows


On Aug. 9, 2017, 6:24 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61531/
> ---
> 
> (Updated Aug. 9, 2017, 6:24 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, Jie Yu, James Peach, Qian Zhang, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the device number proto 'major' and 'minor' to avoid MACROs.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
>   include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
>   src/linux/cgroups.hpp e708747f16fa4579bbd7d0e9e4bf969c05625568 
>   src/linux/cgroups.cpp 21f9983cb34a7b37d65b802dc56bc4804077664b 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 
> 96014b560377b5c716e30781de18d34c2ea8e17b 
> 
> 
> Diff: https://reviews.apache.org/r/61531/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 61565: Stout: Improved the readability of some assertions/expectations.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61511, 61564, 61565]

Logs available here: http://104.210.40.105/logs/master/61565

- Mesos Reviewbot Windows


On Aug. 10, 2017, 5:52 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61565/
> ---
> 
> (Updated Aug. 10, 2017, 5:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prefer checking whether a container is empty instead of checking its
> size.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/flags_tests.cpp 
> d78ab55b983ff6918733617ae8db5be29f817c69 
>   3rdparty/stout/tests/multimap_tests.cpp 
> 36860e43efab04cf8a2276523e6937c1918d5a90 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> 841f655a19e87657de77cb57c30b8310bb5898a4 
>   3rdparty/stout/tests/os/process_tests.cpp 
> 4b5c25782dd67624bf139ad826142183a6d6f1a2 
>   3rdparty/stout/tests/os_tests.cpp e8ecece1d5361e8d0e1ad42682db51b48ab6be1f 
>   3rdparty/stout/tests/proc_tests.cpp 
> c6d1d442df3e38d1d89716dc036b4a57ab0941b1 
>   3rdparty/stout/tests/strings_tests.cpp 
> a51144df652c5d456d8dab49ca8b2cbec69ea4b6 
> 
> 
> Diff: https://reviews.apache.org/r/61565/diff/2/
> 
> 
> Testing
> ---
> 
> The stout tests still pass.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 60494: Exposed LinuxLauncher cgroups helper.

[Qian Zhang]

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




src/slave/containerizer/mesos/linux_launcher.hpp
Lines 64 (patched)


Better to move this method to the beginning together with `create()` and 
`available()`.


- Qian Zhang


On July 11, 2017, 7:11 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60494/
> ---
> 
> (Updated July 11, 2017, 7:11 p.m.)
> 
> 
> Review request for mesos, Qian Zhang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7675
> https://issues.apache.org/jira/browse/MESOS-7675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Expose the LinuxLauncher cgroups helper to generate the cgroups
> path from a container ID. This is needed by the `network/ports`
> isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> e1525231e0a374bc044e929e82b8d051732e97cb 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 1cea04edac8e0c4aea8c1c7d946b5065f3eac931 
> 
> 
> Diff: https://reviews.apache.org/r/60494/diff/8/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 61580: Extracted JNI code into a protected function for clarity.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [61579, 61580]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Aug. 10, 2017, 8:41 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61580/
> ---
> 
> (Updated Aug. 10, 2017, 8:41 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-7872
> https://issues.apache.org/jira/browse/MESOS-7872
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 
> 1f58fbff4e8414e4d2ae4c8f69b637ee3315e411 
> 
> 
> Diff: https://reviews.apache.org/r/61580/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61473: Do not kill non partition aware tasks.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [61473]

Logs available here: http://104.210.40.105/logs/master/61473

- Mesos Reviewbot Windows


On Aug. 10, 2017, 4:07 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61473/
> ---
> 
> (Updated Aug. 10, 2017, 4:07 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7215
> https://issues.apache.org/jira/browse/MESOS-7215
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master will not kill the tasks for non-Partition aware frameworks
> when an unreachable agent re-registers with the master.
> Master used to send a ShutdownFrameworkMessages to the agent
> to kill the tasks from non partition aware frameworks including the
> ones that are still registered which was problematic because the offer
> from this agent could still go to the same framework which could then
> launch new tasks. The agent would then receive tasks of the same
> framework and ignore them because it thinks the framework is shutting
> down. The framework is not shutting down of course, so from the master
> and the scheduler’s perspective the task is pending in STAGING forever
> until the next agent reregistration, which could happen much later.
> This commit fixes the problem by not shutting down the non-partition
> aware frameworks on such an agent.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
>   src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
>   src/tests/partition_tests.cpp 62a84f797201ccd18b71490949e3130d2b9c3668 
> 
> 
> Diff: https://reviews.apache.org/r/61473/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 61575: Added a test for verifying signal escalation on the default executor.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [61569, 61570, 61571, 61572, 61573, 61574, 61575]

Logs available here: http://104.210.40.105/logs/master/61575

- Mesos Reviewbot Windows


On Aug. 10, 2017, 7:32 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61575/
> ---
> 
> (Updated Aug. 10, 2017, 7:32 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test uses the kill policy helper and blocks the SIGTERM signal.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> b9776314a8781963b92ba9ac297654f61a443bc8 
> 
> 
> Diff: https://reviews.apache.org/r/61575/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61575: Added a test for verifying signal escalation on the default executor.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [61569, 61570, 61571, 61572, 61573, 61574, 61575]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Aug. 10, 2017, 7:32 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61575/
> ---
> 
> (Updated Aug. 10, 2017, 7:32 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test uses the kill policy helper and blocks the SIGTERM signal.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> b9776314a8781963b92ba9ac297654f61a443bc8 
> 
> 
> Diff: https://reviews.apache.org/r/61575/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61588: Added a `--skip-style-check` option when applying reviews.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61588]

Logs available here: http://104.210.40.105/logs/master/61588

- Mesos Reviewbot Windows


On Aug. 10, 2017, 11:41 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61588/
> ---
> 
> (Updated Aug. 10, 2017, 11:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Kevin Klues.
> 
> 
> Bugs: MESOS-7880
> https://issues.apache.org/jira/browse/MESOS-7880
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a `--skip-style-check` option when applying reviews.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py aa2eb8d23bed52a4f33510c53ace2c63bd66ec49 
> 
> 
> Diff: https://reviews.apache.org/r/61588/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61408: Added test cases for V1 teardown Call.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [61222, 61408]

Logs available here: http://104.210.40.105/logs/master/61408

- Mesos Reviewbot Windows


On Aug. 10, 2017, 10:47 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61408/
> ---
> 
> (Updated Aug. 10, 2017, 10:47 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Greg Mann.
> 
> 
> Bugs: MESOS-6846
> https://issues.apache.org/jira/browse/MESOS-6846
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for teardown operation in V1 operator API.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 75f7a585ec75f1d8b78453dd203bb78822ee7bc9 
> 
> 
> Diff: https://reviews.apache.org/r/61408/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61588: Added a `--skip-style-check` option when applying reviews.

[Benjamin Bannier]

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




support/apply-reviews.py
Lines 263-266 (original), 269-273 (patched)


Not yours, but we don't necessarily need to disambiguate the keys we map 
the values onto, e.g.,

cmd = u'git commit --author \"{author}\" {amend} -aF \"{message}\" 
{verify}'.format(
author=quote(data['author']),
amend=amend,
message=message_file,
verify=verify)



support/apply-reviews.py
Line 364 (original), 371-373 (patched)


Since this prevents running any (pre-)commit hooks, I wonder if we should 
give this a more general name, e.g., simply expose `[-n|--no-verify]`.


- Benjamin Bannier


On Aug. 11, 2017, 1:41 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61588/
> ---
> 
> (Updated Aug. 11, 2017, 1:41 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Kevin Klues.
> 
> 
> Bugs: MESOS-7880
> https://issues.apache.org/jira/browse/MESOS-7880
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a `--skip-style-check` option when applying reviews.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py aa2eb8d23bed52a4f33510c53ace2c63bd66ec49 
> 
> 
> Diff: https://reviews.apache.org/r/61588/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 60495: Added network ports isolator listen socket utilities.

[Qian Zhang]

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




src/slave/containerizer/mesos/isolators/network/ports.cpp
Lines 181 (patched)


Remove this blank line.



src/slave/containerizer/mesos/isolators/network/ports.cpp
Lines 183 (patched)


Add a newline before.


- Qian Zhang


On Aug. 10, 2017, 4:19 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60495/
> ---
> 
> (Updated Aug. 10, 2017, 4:19 a.m.)
> 
> 
> Review request for mesos, Qian Zhang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7675
> https://issues.apache.org/jira/browse/MESOS-7675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented network ports isolator socket utilities to find the
> inode numbers of all the listening sockets, and the inodes of the
> open sockets for a process. Together, these utilities can tell us
> which sockets a process is listening on.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60495/diff/8/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>