Re: Review Request 60594: Add a`network/ports` isolator nested container test.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [60594, 60593, 60765, 60766, 60591, 60592, 60496, 60495, 
60492, 60836, 60764, 60494, 60493, 60491]

Failed command: python support/apply-reviews.py -n -r 60836

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 158, in apply_review
fetch_patch(options)
  File "support/apply-reviews.py", line 191, in fetch_patch
context=ssl_create_default_context())
  File "C:\Python27\lib\urllib2.py", line 154, in urlopen
return opener.open(url, data, timeout)
  File "C:\Python27\lib\urllib2.py", line 435, in open
response = meth(req, response)
  File "C:\Python27\lib\urllib2.py", line 548, in http_response
'http', request, response, code, msg, hdrs)
  File "C:\Python27\lib\urllib2.py", line 473, in error
return self._call_chain(*args)
  File "C:\Python27\lib\urllib2.py", line 407, in _call_chain
result = func(*args)
  File "C:\Python27\lib\urllib2.py", line 556, in http_error_default
raise HTTPError(req.get_full_url(), code, msg, hdrs, fp)
urllib2.HTTPError: HTTP Error 404: NOT FOUND

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/150/console

- Mesos Reviewbot Windows


On July 3, 2017, 10:32 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60594/
> ---
> 
> (Updated July 3, 2017, 10:32 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
> ---
> 
> Add a test to verify that the `network/ports` isolator works
> correctly with nested containers.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/network_ports_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60594/diff/4/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60594: Add a`network/ports` isolator nested container test.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [60594, 60593, 60765, 60766, 60591, 60592, 60496, 60495, 
60492, 60836, 60764, 60494, 60493, 60491]

Failed command: python support/apply-reviews.py -n -r 60836

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 158, in apply_review
fetch_patch(options)
  File "support/apply-reviews.py", line 191, in fetch_patch
context=ssl_create_default_context())
  File "C:\Python27\lib\urllib2.py", line 154, in urlopen
return opener.open(url, data, timeout)
  File "C:\Python27\lib\urllib2.py", line 435, in open
response = meth(req, response)
  File "C:\Python27\lib\urllib2.py", line 548, in http_response
'http', request, response, code, msg, hdrs)
  File "C:\Python27\lib\urllib2.py", line 473, in error
return self._call_chain(*args)
  File "C:\Python27\lib\urllib2.py", line 407, in _call_chain
result = func(*args)
  File "C:\Python27\lib\urllib2.py", line 556, in http_error_default
raise HTTPError(req.get_full_url(), code, msg, hdrs, fp)
urllib2.HTTPError: HTTP Error 404: NOT FOUND

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/149/console

- Mesos Reviewbot Windows


On July 3, 2017, 10:32 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60594/
> ---
> 
> (Updated July 3, 2017, 10:32 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
> ---
> 
> Add a test to verify that the `network/ports` isolator works
> correctly with nested containers.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/network_ports_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60594/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 56895: Added tests to ensure slave recovery post reboot.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [60103, 60104, 60105, 56895]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On July 13, 2017, 4:22 a.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56895/
> ---
> 
> (Updated July 13, 2017, 4:22 a.m.)
> 
> 
> Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6223
> https://issues.apache.org/jira/browse/MESOS-6223
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests to verify that the state is recovered post reboot and
> agentId is retained given the recovery finishes without errors and
> if the recovery fails due to slaveInfo mismatch then agent no
> state is recoverd making the agent register as a new agent.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp f6eafcbe3d89c7a69c03db0fd7bc10ae73d06584 
> 
> 
> Diff: https://reviews.apache.org/r/56895/diff/17/
> 
> 
> Testing
> ---
> 
> make check done together with 60104 and 60103
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 60593: Test the `network/ports` isolator recovery.

[James Peach]

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

(Updated July 13, 2017, 4:58 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
---

Add a test for the `network/ports` isolator recovery by starting a
task that is listening on a rogue port. We only configure the
isolator when we restart the agent to simulate the case where a
task only starts to misbehave after an agent recovery.


Diffs (updated)
-

  src/tests/containerizer/network_ports_isolator_tests.cpp PRE-CREATION 


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

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


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Review Request 60836: Add IntervalSet to Ranges conversion helper declarations.

[James Peach]

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

Review request for mesos, Qian Zhang and Jiang Yan Xu.


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


Repository: mesos


Description
---

Add a new `common/values.hpp` header file to expose IntervalSet
to Ranges conversion helper declarations.


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 60491: Capture the inode when scanning for sockets.

[James Peach]

> On July 12, 2017, 2:53 p.m., Qian Zhang wrote:
> > src/linux/routing/diagnosis/diagnosis.cpp
> > Line 77 (original), 78 (patched)
> > 
> >
> > Mind to explain a bit about why making this change?
> > And with this change, I think you do not need to explicitly call the 
> > constructor of `Info` anymore.

I was near the code and I thought it could be improved by using `emplace_back`. 
Good point to remove unnecessary constructor.


- James


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


On June 28, 2017, 7:44 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60491/
> ---
> 
> (Updated June 28, 2017, 7:44 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
> ---
> 
> Capture the socket inode in the diagnosis Info when we use netlink
> to enumerate the open sockets. This can be used to indentify which
> process(es) have the socket open.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/diagnosis/diagnosis.hpp 
> 7722fd2843614c3869cc6c3d06bc4c82ed282381 
>   src/linux/routing/diagnosis/diagnosis.cpp 
> aa2d020fc009040410ef557d10d386d04d71d16e 
> 
> 
> Diff: https://reviews.apache.org/r/60491/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 56895: Added tests to ensure slave recovery post reboot.

[Megha Sharma]

> On July 12, 2017, 5:47 p.m., Jiang Yan Xu wrote:
> > LGTM. Just a few minor style issues.

Thanks, addressed the comments.


- Megha


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


On July 13, 2017, 4:22 a.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56895/
> ---
> 
> (Updated July 13, 2017, 4:22 a.m.)
> 
> 
> Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6223
> https://issues.apache.org/jira/browse/MESOS-6223
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests to verify that the state is recovered post reboot and
> agentId is retained given the recovery finishes without errors and
> if the recovery fails due to slaveInfo mismatch then agent no
> state is recoverd making the agent register as a new agent.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp f6eafcbe3d89c7a69c03db0fd7bc10ae73d06584 
> 
> 
> Diff: https://reviews.apache.org/r/56895/diff/17/
> 
> 
> Testing
> ---
> 
> make check done together with 60104 and 60103
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 56895: Added tests to ensure slave recovery post reboot.

[Megha Sharma]

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

(Updated July 13, 2017, 4:22 a.m.)


Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Added tests to verify that the state is recovered post reboot and
agentId is retained given the recovery finishes without errors and
if the recovery fails due to slaveInfo mismatch then agent no
state is recoverd making the agent register as a new agent.


Diffs (updated)
-

  src/tests/slave_recovery_tests.cpp f6eafcbe3d89c7a69c03db0fd7bc10ae73d06584 


Diff: https://reviews.apache.org/r/56895/diff/17/

Changes: https://reviews.apache.org/r/56895/diff/16-17/


Testing
---

make check done together with 60104 and 60103


Thanks,

Megha Sharma

Re: Review Request 60764: Refactor isolator dependency checking.

[Qian Zhang]

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




src/slave/containerizer/mesos/containerizer.cpp
Lines 151 (patched)


No need `std::`



src/slave/containerizer/mesos/containerizer.cpp
Line 155 (original), 170 (patched)


Kill this empty line.



src/slave/containerizer/mesos/containerizer.cpp
Line 181 (original), 203 (patched)


Not yours. A period in the end.



src/slave/containerizer/mesos/containerizer.cpp
Lines 182-192 (original), 204-221 (patched)


Can we merge the logic of these two `if` blocks? I mean:
1. Get the count by calling `std::count_if()`.
2. If the count > 1, error out.
3. Else if the count == 0, do the insert.

In this way, we do not need to call `strings::contains()` anymore.



src/slave/containerizer/mesos/containerizer.cpp
Lines 238 (patched)


Ditto



src/slave/containerizer/mesos/containerizer.cpp
Lines 207-210 (original), 238-250 (patched)


Ditto.


- Qian Zhang


On July 11, 2017, 7:13 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60764/
> ---
> 
> (Updated July 11, 2017, 7:13 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
> ---
> 
> Refactor the isolator dependency checks to immediately tokenize the
> isolator string, which makes it easier to check various consistency
> conditions.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 9376d14d66f5dc7e91c7c0e9da253f5eb9347539 
> 
> 
> Diff: https://reviews.apache.org/r/60764/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60821: Introduced a "no sender" UPID.

[James Peach]

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



I think that a better approach is to define a static `UPID UPID::anonymous()` 
function that returns a well-defined anonymous UPID for the system. Remove all 
the sending functions that don't specify a `from` UPID and force them to 
explicitly pass the anonymous UPID. This makes the used of anonymous message 
sends explicit which I think is preferable to this implicit approach.

- James Peach


On July 13, 2017, 12:04 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60821/
> ---
> 
> (Updated July 13, 2017, 12:04 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, James Peach, and Joseph Wu.
> 
> 
> Bugs: MESOS-7786
> https://issues.apache.org/jira/browse/MESOS-7786
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The immediate use of this is to fix MESOS-7753 but I feel this is a general 
> concept that's defined in similar systems (e.g., akka).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> b4d7791782a5d9e10fa44489057c8504d594bab2 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> c6109547d04bd06e9395e4ec5f5130fd1500f9a0 
> 
> 
> Diff: https://reviews.apache.org/r/60821/diff/1/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 60788: Windows: Fixed long path support in `reparsepoint.hpp`.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [60788]

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 July 12, 2017, 9:44 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60788/
> ---
> 
> (Updated July 12, 2017, 9:44 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixes an error on my part where I accidentally didn't use the
> `longpath` helper, but just `wide_stringify` in two instances.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
> 066c192586f0a9bf74127342e827f38c9de1b4ca 
> 
> 
> Diff: https://reviews.apache.org/r/60788/diff/1/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> Fixes a bug I'd rather not talk about.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 60821: Introduced a "no sender" UPID.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [60821]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On July 12, 2017, 5:04 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60821/
> ---
> 
> (Updated July 12, 2017, 5:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, James Peach, and Joseph Wu.
> 
> 
> Bugs: MESOS-7786
> https://issues.apache.org/jira/browse/MESOS-7786
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The immediate use of this is to fix MESOS-7753 but I feel this is a general 
> concept that's defined in similar systems (e.g., akka).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> b4d7791782a5d9e10fa44489057c8504d594bab2 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> c6109547d04bd06e9395e4ec5f5130fd1500f9a0 
> 
> 
> Diff: https://reviews.apache.org/r/60821/diff/1/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 60820: Add class definition for SlaveID, ContainerID acceptors.

[Quinn Leng]

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



To resolve dependency issue between reviews
(correlation between these three endpoints
through SlaveWriter, FullFrameworkWriter)

https://reviews.apache.org/r/60712/ ,
https://reviews.apache.org/r/60580/ ,
https://reviews.apache.org/r/60581/
These three patches are merged and closed. This is
the class definition part of the merged code.

- Quinn Leng


On July 13, 2017, 12:22 a.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60820/
> ---
> 
> (Updated July 13, 2017, 12:22 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit contains the class definition for SlaveID, ContainerID,
> which are used in filtering for '/frameworks', '/slaves' and
> '/slave/containers' endpoint.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 93c9b2e58600189867b85175fe4de2dc2f6bf33e 
>   src/common/http.cpp 7dce4cdc82a64702431bbc4307757ec797cf4309 
> 
> 
> Diff: https://reviews.apache.org/r/60820/diff/4/
> 
> 
> Testing
> ---
> 
> make check -j48
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60784: Fixed a bug in operator== for DiskInfo::Source.

[Greg Mann]

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



One question: if we were to enforce in our validation code that the relevant 
member of `Resource::DiskInfo::Source` must be set when its corresponding type 
is set (in this case, the `path` or `mount` fields), would we still want this 
patch for `operator==`? Should we add that to the validation code, or is it a 
valid state for `type` to be set and the corresponding optional member to be 
unset?

- Greg Mann


On July 11, 2017, 10:29 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60784/
> ---
> 
> (Updated July 11, 2017, 10:29 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous implementation was incorrect for the case when either
> `right.path` or `right.mount` is set but the corresponding field in
> `left` is unset.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 3f4fff4f1dbe9c8acc55e3077212fe329e53e4d9 
>   src/v1/resources.cpp 55d493e89114acc94b1524f3f94a47ccea20469a 
> 
> 
> Diff: https://reviews.apache.org/r/60784/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> I tried to write a unit test for this specific problem but wasn't able to 
> repro :-\ Current coding seems wrong / inconsistent in any case, though.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 60822: Add filtering to /slaves and /frameworks endpoints.

[Quinn Leng]

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

Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod 
Kone.


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


Repository: mesos


Description
---

Add filtering to /slaves and /frameworks endpoints.

Added query parameter support for 'slaves'
endpoint. We can use 'slave_id' to specify
which slave information to query. Thus it
looks like '/slaves?slave_id=[slave id]'

If not slave id is specified, it will return
information of all slaves, just like before

The structure for response is the same for both
specifying or not specifying slave id. Thus even
for request with slave id specified, the response
will still contain both fields 'slaves', 
'recovered_slaves' etc..

Add query parameter support for '/frameworks' endpoint.
To query the information about a specific framework, we
can use 'framework_id' to specify the framework, thus it
looks like'/frameworks?framework_id=[framework id]'

To maintain consistency, the structure for response is
reserved, thus the response still contains field 'frameworks'
and 'completed_frameworks' etc..

Since code for '/state' endpoint and '/frameworks' are coupled
(they share 'FullFrameworkWriter' class), thus the code for '/state'
endpoint is also updated.


Diffs
-

  src/master/http.cpp 948aa118101b6ce03410c9e0c945b6ca16668ca2 


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


Testing
---

Passed 'make check -j48'
Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check 
-j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh 
--gtest_filter="MasterTest.FrameworksEndpointQueryFramework" 
--gtest_repeat=1000 --gtest_break_on_failure'
Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh 
--gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 
--gtest_break_on_failure'


Thanks,

Quinn Leng

Re: Review Request 60822: Add filtering to /slaves, /slave/containers and /frameworks endpoints.

[Quinn Leng]

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

(Updated July 13, 2017, 12:32 a.m.)


Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod 
Kone.


Summary (updated)
-

Add filtering to /slaves, /slave/containers and /frameworks endpoints.


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


Repository: mesos


Description (updated)
---

Added query parameter support for 'slaves'
endpoint. We can use 'slave_id' to specify
which slave information to query. Thus it
looks like '/slaves?slave_id=[slave id]'

If not slave id is specified, it will return
information of all slaves, just like before

The structure for response is the same for both
specifying or not specifying slave id. Thus even
for request with slave id specified, the response
will still contain both fields 'slaves',
'recovered_slaves' etc..

Add query parameter support for '/frameworks' endpoint.
To query the information about a specific framework, we
can use 'framework_id' to specify the framework, thus it
looks like'/frameworks?framework_id=[framework id]'

To maintain consistency, the structure for response is
reserved, thus the response still contains field 'frameworks'
and 'completed_frameworks' etc..

Since code for '/state' endpoint and '/frameworks' are coupled
(they share 'FullFrameworkWriter' class), thus the code for '/state'
endpoint is also updated.

Add query parameter support for '/slave/containers' endpoint.
To get the information about a specific container, you can specify
the container_id. The request would look like
'/slave/containers?container_id=[container id]'.

When no container id is specified, it will return the information
about all containers.


Diffs (updated)
-

  src/master/http.cpp 948aa118101b6ce03410c9e0c945b6ca16668ca2 
  src/slave/http.hpp ad412d4e2fde13a7afa3563c611ef50b11829cac 
  src/slave/http.cpp 3070b3b06a9d9e75b9d5afb255098e63a1056ec2 


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

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


Testing
---

Passed 'make check -j48'
Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check 
-j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh 
--gtest_filter="MasterTest.FrameworksEndpointQueryFramework" 
--gtest_repeat=1000 --gtest_break_on_failure'
Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh 
--gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 
--gtest_break_on_failure'


Thanks,

Quinn Leng

Re: Review Request 60820: Add class definition for SlaveID, ContainerID acceptors.

[Quinn Leng]

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

(Updated July 13, 2017, 12:22 a.m.)


Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod 
Kone.


Summary (updated)
-

Add class definition for SlaveID, ContainerID acceptors.


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


Repository: mesos


Description (updated)
---

Add class definition for SlaveID, ContainerID acceptors.

To resolve dependency issue 
(correlation between these three endpoints through SlaveWriter, 
FullFrameworkWriter)
between reviews
https://reviews.apache.org/r/60712/ ,
https://reviews.apache.org/r/60580/ ,
https://reviews.apache.org/r/60581/ 
These three patches are merged and closed. This is
the class definition part of the merged code.


Diffs (updated)
-

  src/common/http.hpp 93c9b2e58600189867b85175fe4de2dc2f6bf33e 
  src/common/http.cpp 7dce4cdc82a64702431bbc4307757ec797cf4309 


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

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


Testing
---

make check -j48


Thanks,

Quinn Leng

Re: Review Request 60820: Add class definition for SlaveID acceptor.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [60279, 60716, 60820]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On July 12, 2017, 11:35 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60820/
> ---
> 
> (Updated July 12, 2017, 11:35 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add class definition for SlaveID acceptor.
> 
> To resolve the dependency issue (they are correlated
> by SlaveWriter, FrameworkWriter through the /state endpoint)
> between reviews
> https://reviews.apache.org/r/60712/ ,
> https://reviews.apache.org/r/60580/ ,
> https://reviews.apache.org/r/60581/
> these three patches are merged and closed.
> This patch contains the class definition
> for the merged code.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 93c9b2e58600189867b85175fe4de2dc2f6bf33e 
>   src/common/http.cpp 7dce4cdc82a64702431bbc4307757ec797cf4309 
> 
> 
> Diff: https://reviews.apache.org/r/60820/diff/1/
> 
> 
> Testing
> ---
> 
> make check -j48
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60520: Removed posix/subprocess.hpp dependencies.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On June 28, 2017, 10:49 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60520/
> ---
> 
> (Updated June 28, 2017, 10:49 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Greg Mann.
> 
> 
> Bugs: MESOS-7778
> https://issues.apache.org/jira/browse/MESOS-7778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix a number of places where we were depending on symbols made
> visible by . In most cases, this is
> just tweaking the includes, but in the Checker we just duplicate
> the trivial fork(2) helper since that is easer than trying to
> export it as a libprocess API.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.cpp 66f5452bb5f9c7f45819c221be7b098e5999b7a7 
>   src/slave/containerizer/mesos/isolators/volume/image.cpp 
> c89af3e0a16f769c45f2846d1e76baedb8248d4d 
>   src/uri/fetchers/docker.cpp 7f3e4e73d1dc5d15f875e2890acbe5369fb59d78 
> 
> 
> Diff: https://reviews.apache.org/r/60520/diff/5/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26).
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 60821: Introduced a "no sender" UPID.

[Jiang Yan Xu]

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

Review request for mesos, Benjamin Mahler, James Peach, and Joseph Wu.


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


Repository: mesos


Description
---

The immediate use of this is to fix MESOS-7753 but I feel this is a general 
concept that's defined in similar systems (e.g., akka).


Diffs
-

  3rdparty/libprocess/src/process.cpp b4d7791782a5d9e10fa44489057c8504d594bab2 
  3rdparty/libprocess/src/tests/process_tests.cpp 
c6109547d04bd06e9395e4ec5f5130fd1500f9a0 


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


Testing
---

make check.


Thanks,

Jiang Yan Xu

Review Request 60820: Add class definition for SlaveID acceptor.

[Quinn Leng]

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

Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod 
Kone.


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


Repository: mesos


Description
---

Add class definition for SlaveID acceptor.

To resolve the dependency issue (they are correlated
by SlaveWriter, FrameworkWriter through the /state endpoint)
between reviews
https://reviews.apache.org/r/60712/ ,
https://reviews.apache.org/r/60580/ ,
https://reviews.apache.org/r/60581/
these three patches are merged and closed.
This patch contains the class definition
for the merged code.


Diffs
-

  src/common/http.hpp 93c9b2e58600189867b85175fe4de2dc2f6bf33e 
  src/common/http.cpp 7dce4cdc82a64702431bbc4307757ec797cf4309 


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


Testing
---

make check -j48


Thanks,

Quinn Leng

Re: Review Request 60719: Add test infrastructure for src/python/lib/mesos.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [60719]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On July 12, 2017, 11:21 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60719/
> ---
> 
> (Updated July 12, 2017, 11:21 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Part of MESOS-7310, this patch adds the test infrastructure necessary
> for reliably running unit tests for the mesos package located under
> src/python/lib.
> 
> setup.py is added under src/python/lib to both define the Python package
> and to allow tests to be run via `python setup.py test`, which delegates
> tests to pytest.
> 
> Review: https://reviews.apache.org/r/60719/
> 
> 
> Diffs
> -
> 
>   src/python/.gitignore 0d20b6487c61e7d1bde93acf4a14b7a89083a16d 
>   src/python/lib/mesos/__init__.py e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/requirements-test.in PRE-CREATION 
>   src/python/lib/requirements.in PRE-CREATION 
>   src/python/lib/setup.cfg PRE-CREATION 
>   src/python/lib/setup.py PRE-CREATION 
>   src/python/lib/test.sh PRE-CREATION 
>   src/python/lib/tests/__init__.py PRE-CREATION 
>   support/mesos-style.py 48d816f72df65d77364769f812fc06afcd6b5aec 
> 
> 
> Diff: https://reviews.apache.org/r/60719/diff/6/
> 
> 
> Testing
> ---
> 
> under src/python/lib, run `bash test.sh`
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 60719: Add test infrastructure for src/python/lib/mesos.

[Eric Chung]

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

(Updated July 12, 2017, 11:06 p.m.)


Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.


Changes
---

use pytest directly instead of using pytest-runner


Repository: mesos


Description
---

Part of MESOS-7310, this patch adds the test infrastructure necessary
for reliably running unit tests for the mesos package located under
src/python/lib.

setup.py is added under src/python/lib to both define the Python package
and to allow tests to be run via `python setup.py test`, which delegates
tests to pytest.


Diffs (updated)
-

  src/python/.gitignore 0d20b6487c61e7d1bde93acf4a14b7a89083a16d 
  src/python/lib/mesos/__init__.py e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
  src/python/lib/requirements-test.in PRE-CREATION 
  src/python/lib/requirements.in PRE-CREATION 
  src/python/lib/setup.cfg PRE-CREATION 
  src/python/lib/setup.py PRE-CREATION 
  src/python/lib/test.sh PRE-CREATION 
  src/python/lib/tests/__init__.py PRE-CREATION 
  support/mesos-style.py 48d816f72df65d77364769f812fc06afcd6b5aec 


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

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


Testing (updated)
---

under src/python/lib, run `bash test.sh`


Thanks,

Eric Chung

Re: Review Request 60719: Add test infrastructure for src/python/lib/mesos.

[Eric Chung]

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

(Updated July 12, 2017, 11:21 p.m.)


Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.


Repository: mesos


Description (updated)
---

Part of MESOS-7310, this patch adds the test infrastructure necessary
for reliably running unit tests for the mesos package located under
src/python/lib.

setup.py is added under src/python/lib to both define the Python package
and to allow tests to be run via `python setup.py test`, which delegates
tests to pytest.

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


Diffs (updated)
-

  src/python/.gitignore 0d20b6487c61e7d1bde93acf4a14b7a89083a16d 
  src/python/lib/mesos/__init__.py e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
  src/python/lib/requirements-test.in PRE-CREATION 
  src/python/lib/requirements.in PRE-CREATION 
  src/python/lib/setup.cfg PRE-CREATION 
  src/python/lib/setup.py PRE-CREATION 
  src/python/lib/test.sh PRE-CREATION 
  src/python/lib/tests/__init__.py PRE-CREATION 
  support/mesos-style.py 48d816f72df65d77364769f812fc06afcd6b5aec 


Diff: https://reviews.apache.org/r/60719/diff/6/

Changes: https://reviews.apache.org/r/60719/diff/5-6/


Testing
---

under src/python/lib, run `bash test.sh`


Thanks,

Eric Chung

Re: Review Request 60719: Add test infrastructure for src/python/lib/mesos.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [60719]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On July 7, 2017, 9:44 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60719/
> ---
> 
> (Updated July 7, 2017, 9:44 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Part of MESOS-7310, this patch adds the test infrastructure necessary
> for reliably running unit tests for the mesos package located under
> src/python/lib.
> 
> setup.py is added under src/python/lib to both define the Python package
> and to allow tests to be run via `python setup.py test`, which delegates
> tests to pytest.
> 
> 
> Diffs
> -
> 
>   src/python/lib/mesos/__init__.py e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/requirements-test.in PRE-CREATION 
>   src/python/lib/requirements.in PRE-CREATION 
>   src/python/lib/setup.cfg PRE-CREATION 
>   src/python/lib/setup.py PRE-CREATION 
>   support/mesos-style.py 48d816f72df65d77364769f812fc06afcd6b5aec 
> 
> 
> Diff: https://reviews.apache.org/r/60719/diff/4/
> 
> 
> Testing
> ---
> 
> 1. under src/python/lib, run `virtualenv env`
> 2. `. env/bin/activate`
> 3. `pip install setuptools --upgrade`
> 4. `python setup.py test`
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 60712: Added filtering support for '/slave/containers' endpoint.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [60712, 60581, 60580, 60716, 60279]

Failed command: python support/apply-reviews.py -n -r 60581

Error:
error: patch failed: src/master/http.cpp:2326
error: src/master/http.cpp: patch does not apply

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/144/console

- Mesos Reviewbot Windows


On July 7, 2017, 11:47 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60712/
> ---
> 
> (Updated July 7, 2017, 11:47 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added filtering support for '/slave/containers' endpoint.
> 
> Add query parameter support for '/slave/containers' endpoint.
> To get the information about a specific container, you can specify
> the container_id. The request would look like
> '/slave/containers?container_id=[container id]'.
> 
> When no container id is specified, it will return the information
> about all containers.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp b7e4a8adcbcaa3a962af795c67694a35161b6c1a 
>   src/common/http.cpp fdb591eccf273260902f3f695cf431f72ee3d817 
>   src/slave/http.hpp ad412d4e2fde13a7afa3563c611ef50b11829cac 
>   src/slave/http.cpp 700871e1502a65b0bb1fc31219e09219dbdb5340 
>   src/tests/slave_tests.cpp 8a69cc2ede0b2f17a31986e9142aa2081691eb5e 
> 
> 
> Diff: https://reviews.apache.org/r/60712/diff/2/
> 
> 
> Testing
> ---
> 
> Passed 'make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-test.sh 
> --gtest_filter=SlaveTest.ContainersEndpoint --gtest_repeat=1000 
> --gtest_break_on_failure'
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60712: Added filtering support for '/slave/containers' endpoint.

[Greg Mann]

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




src/common/http.hpp
Lines 227 (patched)


s/Filtering/Filters/



src/common/http.cpp
Lines 1190 (patched)


I think that testing for strict equality here will not work, since the 
`ContainerID` we are testing against may have the `parent` field set. This 
would come out in the test if we launched a task group (since those containers 
would have a parent), but since we aren't launching nested containers the test 
still passes.

Instead, we should probably check that `containerId->value() == 
_containerId.value()`


- Greg Mann


On July 7, 2017, 11:47 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60712/
> ---
> 
> (Updated July 7, 2017, 11:47 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added filtering support for '/slave/containers' endpoint.
> 
> Add query parameter support for '/slave/containers' endpoint.
> To get the information about a specific container, you can specify
> the container_id. The request would look like
> '/slave/containers?container_id=[container id]'.
> 
> When no container id is specified, it will return the information
> about all containers.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp b7e4a8adcbcaa3a962af795c67694a35161b6c1a 
>   src/common/http.cpp fdb591eccf273260902f3f695cf431f72ee3d817 
>   src/slave/http.hpp ad412d4e2fde13a7afa3563c611ef50b11829cac 
>   src/slave/http.cpp 700871e1502a65b0bb1fc31219e09219dbdb5340 
>   src/tests/slave_tests.cpp 8a69cc2ede0b2f17a31986e9142aa2081691eb5e 
> 
> 
> Diff: https://reviews.apache.org/r/60712/diff/2/
> 
> 
> Testing
> ---
> 
> Passed 'make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-test.sh 
> --gtest_filter=SlaveTest.ContainersEndpoint --gtest_repeat=1000 
> --gtest_break_on_failure'
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60719: Add test infrastructure for src/python/lib/mesos.

[Eric Chung]

> On July 9, 2017, 9:10 a.m., Armand Grillet wrote:
> > I have not been able to reproduce the steps to test the patch, the last one 
> > returns this error:
> > ```
> > (env) lib (60719) $ python setup.py test
> > zip_safe flag not set; analyzing archive contents...
> > 
> > Installed 
> > /Users/Armand/Code/apache-mesos/src/python/lib/.eggs/pytest_runner-2.11.1-py2.7.egg
> > /Users/Armand/Code/apache-mesos/src/python/lib/env/lib/python2.7/site-packages/setuptools/dist.py:336:
> >  UserWarning: Normalizing '0.0.0.dev' to '0.0.0.dev0'
> >   normalized_version,
> > running pytest
> > Searching for pytest-cov
> > Reading https://pypi.python.org/simple/pytest-cov/
> > Downloading 
> > https://pypi.python.org/packages/24/b4/7290d65b2f3633db51393bdf8ae66309b37620bc3ec116c5e357e3e37238/pytest-cov-2.5.1.tar.gz#md5=5acf38d4909e19819eb5c1754fbfc0ac
> > Best match: pytest-cov 2.5.1
> > Processing pytest-cov-2.5.1.tar.gz
> > Writing 
> > /var/folders/mt/n_tlp88537s9kt9n9173_mdhgn/T/easy_install-zEq9cm/pytest-cov-2.5.1/setup.cfg
> > Running pytest-cov-2.5.1/setup.py -q bdist_egg --dist-dir 
> > /var/folders/mt/n_tlp88537s9kt9n9173_mdhgn/T/easy_install-zEq9cm/pytest-cov-2.5.1/egg-dist-tmp-CB8boX
> > warning: no files found matching '.isort.cfg'
> > warning: no files found matching '.pylintrc'
> > warning: no previously-included files matching '*.py[cod]' found anywhere 
> > in distribution
> > warning: no previously-included files matching '__pycache__' found anywhere 
> > in distribution
> > warning: no previously-included files matching '*.so' found anywhere in 
> > distribution
> > creating 
> > /Users/Armand/Code/apache-mesos/src/python/lib/.eggs/pytest_cov-2.5.1-py2.7.egg
> > Extracting pytest_cov-2.5.1-py2.7.egg to 
> > /Users/Armand/Code/apache-mesos/src/python/lib/.eggs
> > 
> > Installed 
> > /Users/Armand/Code/apache-mesos/src/python/lib/.eggs/pytest_cov-2.5.1-py2.7.egg
> > Searching for pytest
> > Downloading 
> > https://pypi.python.org/packages/24/b4/7290d65b2f3633db51393bdf8ae66309b37620bc3ec116c5e357e3e37238/pytest-cov-2.5.1.tar.gz#md5=5acf38d4909e19819eb5c1754fbfc0ac
> > Best match: pytest cov-2.5.1
> > Processing pytest-cov-2.5.1.tar.gz
> > Writing 
> > /var/folders/mt/n_tlp88537s9kt9n9173_mdhgn/T/easy_install-bfwvC6/pytest-cov-2.5.1/setup.cfg
> > Running pytest-cov-2.5.1/setup.py -q bdist_egg --dist-dir 
> > /var/folders/mt/n_tlp88537s9kt9n9173_mdhgn/T/easy_install-bfwvC6/pytest-cov-2.5.1/egg-dist-tmp-ss6Ebd
> > warning: no files found matching '.isort.cfg'
> > warning: no files found matching '.pylintrc'
> > warning: no previously-included files matching '*.py[cod]' found anywhere 
> > in distribution
> > warning: no previously-included files matching '__pycache__' found anywhere 
> > in distribution
> > warning: no previously-included files matching '*.so' found anywhere in 
> > distribution
> > removing 
> > '/Users/Armand/Code/apache-mesos/src/python/lib/.eggs/pytest_cov-2.5.1-py2.7.egg'
> >  (and everything under it)
> > creating 
> > /Users/Armand/Code/apache-mesos/src/python/lib/.eggs/pytest_cov-2.5.1-py2.7.egg
> > Extracting pytest_cov-2.5.1-py2.7.egg to 
> > /Users/Armand/Code/apache-mesos/src/python/lib/.eggs
> > 
> > Installed 
> > /Users/Armand/Code/apache-mesos/src/python/lib/.eggs/pytest_cov-2.5.1-py2.7.egg
> > Traceback (most recent call last):
> >   File "setup.py", line 63, in 
> > zip_safe=False,
> >   File 
> > "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/distutils/core.py",
> >  line 151, in setup
> > dist.run_commands()
> >   File 
> > "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/distutils/dist.py",
> >  line 953, in run_commands
> > self.run_command(cmd)
> >   File 
> > "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/distutils/dist.py",
> >  line 972, in run_command
> > cmd_obj.run()
> >   File "build/bdist.macosx-10.12-intel/egg/ptr.py", line 150, in run
> >   File "build/bdist.macosx-10.12-intel/egg/ptr.py", line 133, in _super_run
> >   File 
> > "/Users/Armand/Code/apache-mesos/src/python/lib/env/lib/python2.7/site-packages/setuptools/command/test.py",
> >  line 199, in run
> > installed_dists = self.install_dists(self.distribution)
> >   File "build/bdist.macosx-10.12-intel/egg/ptr.py", line 91, in 
> > install_dists
> >   File 
> > "/Users/Armand/Code/apache-mesos/src/python/lib/env/lib/python2.7/site-packages/setuptools/command/test.py",
> >  line 195, in install_dists
> > tr_d = dist.fetch_build_eggs(dist.tests_require or [])
> >   File 
> > "/Users/Armand/Code/apache-mesos/src/python/lib/env/lib/python2.7/site-packages/setuptools/dist.py",
> >  line 377, in fetch_build_eggs
> > replace_conflicting=True,
> >   File 
> > "/Users/Armand/Code/apache-mesos/src/python/lib/env/lib/python2.7/site-packages/pkg_resources/__init__.py",
> >  line 855, in resolve
> > raise DistributionNotFound(req, requirers)
> > pkg_resources.DistributionNotFound: The 

Review Request 60815: Prefer leading instead of trailing spaces for allocator log messages.

[Gastón Kleiman]

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

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


Repository: mesos


Description
---

Prefer leading instead of trailing spaces for allocator log messages.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
f021c34ef11aac42026ba39c5a1b775794982035 


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


Testing
---

Not a functional change.


Thanks,

Gastón Kleiman

Re: Review Request 60815: Prefer leading instead of trailing spaces for allocator log messages.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [60815, 60525, 60721, 60524]

Failed command: python support/apply-reviews.py -n -r 60524

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 160, in apply_review
commit_patch(options)
  File "support/apply-reviews.py", line 268, in commit_patch
shell(cmd, options['dry_run'])
  File "support/apply-reviews.py", line 144, in shell
error_code = subprocess.call(command, stderr=subprocess.STDOUT, shell=True)
  File "C:\Python27\lib\subprocess.py", line 168, in call
return Popen(*popenargs, **kwargs).wait()
  File "C:\Python27\lib\subprocess.py", line 390, in __init__
errread, errwrite)
  File "C:\Python27\lib\subprocess.py", line 610, in _execute_child
args = '{} /c "{}"'.format (comspec, args)
UnicodeEncodeError: 'ascii' codec can't encode character u'\xf3' in position 
25: ordinal not in range(128)

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/142/console

- Mesos Reviewbot Windows


On July 12, 2017, 9:49 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60815/
> ---
> 
> (Updated July 12, 2017, 9:49 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, 
> and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prefer leading instead of trailing spaces for allocator log messages.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> f021c34ef11aac42026ba39c5a1b775794982035 
> 
> 
> Diff: https://reviews.apache.org/r/60815/diff/1/
> 
> 
> Testing
> ---
> 
> Not a functional change.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

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

[Gastón Kleiman]

> On July 10, 2017, 7:52 a.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 1048-1049 (original), 1048-1050 (patched)
> > 
> >
> > We usually use leading spaces instead of trailing spaces, but I see 
> > that you follow the pattern here. Maybe clean the whole file in a separate 
> > patch?

https://reviews.apache.org/r/60815/


- Gastón


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


On July 12, 2017, 9:41 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60525/
> ---
> 
> (Updated July 12, 2017, 9:41 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 allocator use the largest possible filter duration
> in this case.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> f021c34ef11aac42026ba39c5a1b775794982035 
> 
> 
> Diff: https://reviews.apache.org/r/60525/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

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

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [60525, 60721, 60524]

Failed command: python support/apply-reviews.py -n -r 60524

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 160, in apply_review
commit_patch(options)
  File "support/apply-reviews.py", line 268, in commit_patch
shell(cmd, options['dry_run'])
  File "support/apply-reviews.py", line 144, in shell
error_code = subprocess.call(command, stderr=subprocess.STDOUT, shell=True)
  File "C:\Python27\lib\subprocess.py", line 168, in call
return Popen(*popenargs, **kwargs).wait()
  File "C:\Python27\lib\subprocess.py", line 390, in __init__
errread, errwrite)
  File "C:\Python27\lib\subprocess.py", line 610, in _execute_child
args = '{} /c "{}"'.format (comspec, args)
UnicodeEncodeError: 'ascii' codec can't encode character u'\xf3' in position 
25: ordinal not in range(128)

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/141/console

- Mesos Reviewbot Windows


On July 12, 2017, 9:41 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60525/
> ---
> 
> (Updated July 12, 2017, 9:41 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 allocator use the largest possible filter duration
> in this case.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> f021c34ef11aac42026ba39c5a1b775794982035 
> 
> 
> Diff: https://reviews.apache.org/r/60525/diff/4/
> 
> 
> 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 July 12, 2017, 9:41 p.m.)


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


Changes
---

Added a blank line, fixed whitespace.


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 allocator use the largest possible filter duration
in this case.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
f021c34ef11aac42026ba39c5a1b775794982035 


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

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


Testing
---

`make check`


Thanks,

Gastón Kleiman

Re: Review Request 60524: Stout: Made the `Duration` operators handle int overflows explicitly.

[Gastón Kleiman]

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

(Updated July 12, 2017, 9:40 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Made the `Duration` arithmetic operators return `Duration::max()` if the
operation would result in an integer overflow, and `Duration::min()` if
it would result in an underflow.


Diffs (updated)
-

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


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

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


Testing
---

Added new Stout tests and confirmed that the Mesos test suite still passes.


Thanks,

Gastón Kleiman

Re: Review Request 60716: Cleaned up authentication acceptor to have one class.

[Quinn Leng]

> On July 11, 2017, 5:23 p.m., Greg Mann wrote:
> > src/common/http.hpp
> > Line 178 (original), 188 (patched)
> > 
> >
> > If we take the approver by rvalue reference here - i.e., 
> > `Owned&&` - then we can avoid copies:
> > 
> > ```
> > AuthorizationAcceptor(const process::Owned&& approver)
> >   : objectApprover(std::forward(approver)) {}
> > 
> > ```
> 
> Greg Mann wrote:
> Could you drop this instead of resolving, since we decided to drop it?

Oh, since it's written in TODO, I guess it's not 'dropped' in definition. 
Anyway, I can drop it now.


> On July 11, 2017, 5:23 p.m., Greg Mann wrote:
> > src/common/http.cpp
> > Line 1144 (original), 1121 (patched)
> > 
> >
> > In this context, the `AcceptingObjectApprover` seems like an 
> > unnecessary level of indirection to me. What do you think about making the 
> > `objectApprover` member an `Option`, and then 
> > `accept()` could return true if `objectApprover.isNone()`?
> > 
> > We could take care of this with a follow-up patch.
> 
> Greg Mann wrote:
> Were you going to take care of this in a follow-up patch? If so, please 
> reply accordingly and drop the issue.

That's a good update, I can do that in follow-up patch.


- Quinn


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


On July 12, 2017, 1:17 a.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60716/
> ---
> 
> (Updated July 12, 2017, 1:17 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up authentication acceptor to have one class.
> 
> Replace different Authorization related Acceptor classes with one
> AuthorizationAcceptor class.
> 
> Single static create function for AuthorizationAcceptor. 
> Templated 'accept' function to take different number and types of
> parameters.
> 
> Removed ObjectAcceptor parent class, since no inheritance feature
> provided by it.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 93c9b2e58600189867b85175fe4de2dc2f6bf33e 
>   src/common/http.cpp 7dce4cdc82a64702431bbc4307757ec797cf4309 
>   src/master/http.cpp 948aa118101b6ce03410c9e0c945b6ca16668ca2 
> 
> 
> Diff: https://reviews.apache.org/r/60716/diff/3/
> 
> 
> Testing
> ---
> 
> Passed "make check -j48"
> Passed 'GTEST_FILTER="MasterTest.TasksEndpoint" make check -j48'
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60580: Added filtering to the '/frameworks' endpoint.

[Quinn Leng]

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

(Updated July 12, 2017, 6:19 p.m.)


Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod 
Kone.


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


Repository: mesos


Description
---

Added filtering to the '/frameworks' endpoint.

Add query parameter support for '/frameworks' endpoint.
To query the information about a specific framework, we
can use 'framework_id' to specify the framework, thus it
looks like'/frameworks?framework_id=[framework id]'

To maintain consistency, the structure for response is
reserved, thus the response still contains field 'frameworks'
and 'completed_frameworks' etc..

Since code for '/state' endpoint and '/frameworks' are coupled
(they share 'FullFrameworkWriter' class), thus the code for '/state'
endpoint is also updated.


Diffs (updated)
-

  src/master/http.cpp 948aa118101b6ce03410c9e0c945b6ca16668ca2 
  src/tests/master_tests.cpp 6e6461c2e13c3eb055aa3c2d8ad8e3ac54a1d197 


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

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


Testing
---

Passed 'make check -j48'
Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check 
-j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh 
--gtest_filter="MasterTest.FrameworksEndpointQueryFramework" 
--gtest_repeat=1000 --gtest_break_on_failure'


Thanks,

Quinn Leng

Re: Review Request 60716: Cleaned up authentication acceptor to have one class.

[Quinn Leng]

> On July 12, 2017, 5:50 p.m., Greg Mann wrote:
> > src/common/http.hpp
> > Lines 175-177 (original), 173-185 (patched)
> > 
> >
> > Is there a reason this implementation is placed in the header?

It is not possible to write the implementation of a template class in a 
separate cpp file and compile.  
https://stackoverflow.com/questions/1724036/splitting-templated-c-classes-into-hpp-cpp-files-is-it-possible


- Quinn


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


On July 12, 2017, 1:17 a.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60716/
> ---
> 
> (Updated July 12, 2017, 1:17 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up authentication acceptor to have one class.
> 
> Replace different Authorization related Acceptor classes with one
> AuthorizationAcceptor class.
> 
> Single static create function for AuthorizationAcceptor. 
> Templated 'accept' function to take different number and types of
> parameters.
> 
> Removed ObjectAcceptor parent class, since no inheritance feature
> provided by it.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 93c9b2e58600189867b85175fe4de2dc2f6bf33e 
>   src/common/http.cpp 7dce4cdc82a64702431bbc4307757ec797cf4309 
>   src/master/http.cpp 948aa118101b6ce03410c9e0c945b6ca16668ca2 
> 
> 
> Diff: https://reviews.apache.org/r/60716/diff/3/
> 
> 
> Testing
> ---
> 
> Passed "make check -j48"
> Passed 'GTEST_FILTER="MasterTest.TasksEndpoint" make check -j48'
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60716: Cleaned up authentication acceptor to have one class.

[Quinn Leng]

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

(Updated July 12, 2017, 6:09 p.m.)


Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod 
Kone.


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


Repository: mesos


Description
---

Cleaned up authentication acceptor to have one class.

Replace different Authorization related Acceptor classes with one
AuthorizationAcceptor class.

Single static create function for AuthorizationAcceptor. 
Templated 'accept' function to take different number and types of
parameters.

Removed ObjectAcceptor parent class, since no inheritance feature
provided by it.


Diffs (updated)
-

  src/common/http.hpp 93c9b2e58600189867b85175fe4de2dc2f6bf33e 
  src/common/http.cpp 7dce4cdc82a64702431bbc4307757ec797cf4309 
  src/master/http.cpp 948aa118101b6ce03410c9e0c945b6ca16668ca2 


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

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


Testing
---

Passed "make check -j48"
Passed 'GTEST_FILTER="MasterTest.TasksEndpoint" make check -j48'


Thanks,

Quinn Leng

Re: Review Request 60581: Added filtering to the '/slaves' endpoint.

[Greg Mann]

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




src/master/http.cpp
Lines 2318 (patched)


Remove the comma at the end of this line.



src/master/http.cpp
Lines 2319 (patched)


s/specified/is specified/



src/master/http.cpp
Lines 2320 (patched)


This empty string can be removed.



src/master/http.cpp
Lines 2329-2330 (original), 2334-2335 (patched)


For the `AuthorizationAcceptor`, we need to use an `Owned` because it holds 
onto an `Owned`. However, in this case, I don't think there's a 
good reason to use `Owned` rather than just `SlaveIDAcceptor`. 
You could do:

```
SlaveIDAcceptor selectSlaveID(request.url.query.get("slave_id"))
```

This is the case for other IDAcceptors as well - could you change other 
cases too?



src/tests/master_tests.cpp
Lines 2277-2278 (patched)


Maybe something like:

"Ensures that the '/slaves' endpoint returns the correct slave when 
provided with a slave ID query parameter."



src/tests/master_tests.cpp
Lines 2282 (patched)


Newline after this.



src/tests/master_tests.cpp
Lines 2299-2300 (patched)


Fits on one line.



src/tests/master_tests.cpp
Lines 2345-2381 (patched)


I think we can test the filtering for just one of the slaves.



src/tests/master_tests.cpp
Lines 2383-2400 (patched)


Is this racy? You stop `slave1` but not `slave2`, and then you check that 
`slave2` is returned in 'recovered_slaves'. Isn't it possible that `slave2` 
reregisters? Should we terminate both slaves while the master is down?


- Greg Mann


On July 7, 2017, 10:36 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60581/
> ---
> 
> (Updated July 7, 2017, 10:36 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added filtering to the '/slaves' endpoint.
> 
> Added query parameter support for 'slaves'
> endpoint. We can use 'slave_id' to specify
> which slave information to query. Thus it
> looks like '/slaves?slave_id=[slave id]'
> 
> If not slave id is specified, it will return
> information of all slaves, just like before
> 
> The structure for response is the same for both
> specifying or not specifying slave id. Thus even
> for request with slave id specified, the response
> will still contain both fields 'slaves', 
> 'recovered_slaves' etc..
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp b7e4a8adcbcaa3a962af795c67694a35161b6c1a 
>   src/common/http.cpp fdb591eccf273260902f3f695cf431f72ee3d817 
>   src/master/http.cpp 175a44ce7fb5be509453c25eaa9ec29f35adba3a 
>   src/tests/master_tests.cpp c778c6c56d47c4033189912cebee6024be79106f 
> 
> 
> Diff: https://reviews.apache.org/r/60581/diff/4/
> 
> 
> Testing
> ---
> 
> Passed 'make check -j48'
> Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 
> --gtest_break_on_failure'
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60716: Cleaned up authentication acceptor to have one class.

[Greg Mann]

> On July 11, 2017, 5:23 p.m., Greg Mann wrote:
> > src/common/http.hpp
> > Line 178 (original), 188 (patched)
> > 
> >
> > If we take the approver by rvalue reference here - i.e., 
> > `Owned&&` - then we can avoid copies:
> > 
> > ```
> > AuthorizationAcceptor(const process::Owned&& approver)
> >   : objectApprover(std::forward(approver)) {}
> > 
> > ```

Could you drop this instead of resolving, since we decided to drop it?


> On July 11, 2017, 5:23 p.m., Greg Mann wrote:
> > src/common/http.cpp
> > Line 1144 (original), 1121 (patched)
> > 
> >
> > In this context, the `AcceptingObjectApprover` seems like an 
> > unnecessary level of indirection to me. What do you think about making the 
> > `objectApprover` member an `Option`, and then 
> > `accept()` could return true if `objectApprover.isNone()`?
> > 
> > We could take care of this with a follow-up patch.

Were you going to take care of this in a follow-up patch? If so, please reply 
accordingly and drop the issue.


> On July 11, 2017, 5:23 p.m., Greg Mann wrote:
> > src/common/http.cpp
> > Line 1155 (original), 1130 (patched)
> > 
> >
> > If we change the acceptor constructor to take the approver by rvalue 
> > reference, then this will have to change to:
> > 
> > ```
> > new AuthorizationAcceptor(std::move(approver)));
> > ```

Could you drop this instead of resolving, since we decided to drop this for now?


- Greg


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


On July 12, 2017, 1:17 a.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60716/
> ---
> 
> (Updated July 12, 2017, 1:17 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up authentication acceptor to have one class.
> 
> Replace different Authorization related Acceptor classes with one
> AuthorizationAcceptor class.
> 
> Single static create function for AuthorizationAcceptor. 
> Templated 'accept' function to take different number and types of
> parameters.
> 
> Removed ObjectAcceptor parent class, since no inheritance feature
> provided by it.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 93c9b2e58600189867b85175fe4de2dc2f6bf33e 
>   src/common/http.cpp 7dce4cdc82a64702431bbc4307757ec797cf4309 
>   src/master/http.cpp 948aa118101b6ce03410c9e0c945b6ca16668ca2 
> 
> 
> Diff: https://reviews.apache.org/r/60716/diff/3/
> 
> 
> Testing
> ---
> 
> Passed "make check -j48"
> Passed 'GTEST_FILTER="MasterTest.TasksEndpoint" make check -j48'
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60716: Cleaned up authentication acceptor to have one class.

[Greg Mann]

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




src/common/http.hpp
Lines 175-177 (original), 173-185 (patched)


Is there a reason this implementation is placed in the header?



src/common/http.hpp
Lines 176-178 (patched)


This can fit on two lines.


- Greg Mann


On July 12, 2017, 1:17 a.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60716/
> ---
> 
> (Updated July 12, 2017, 1:17 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up authentication acceptor to have one class.
> 
> Replace different Authorization related Acceptor classes with one
> AuthorizationAcceptor class.
> 
> Single static create function for AuthorizationAcceptor. 
> Templated 'accept' function to take different number and types of
> parameters.
> 
> Removed ObjectAcceptor parent class, since no inheritance feature
> provided by it.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 93c9b2e58600189867b85175fe4de2dc2f6bf33e 
>   src/common/http.cpp 7dce4cdc82a64702431bbc4307757ec797cf4309 
>   src/master/http.cpp 948aa118101b6ce03410c9e0c945b6ca16668ca2 
> 
> 
> Diff: https://reviews.apache.org/r/60716/diff/3/
> 
> 
> Testing
> ---
> 
> Passed "make check -j48"
> Passed 'GTEST_FILTER="MasterTest.TasksEndpoint" make check -j48'
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 56895: Added tests to ensure slave recovery post reboot.

[Jiang Yan Xu]

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


Ship it!




LGTM. Just a few minor style issues.


src/tests/slave_recovery_tests.cpp
Line 2526 (original), 2526 (patched)


Doesn't look like we need to set this.



src/tests/slave_recovery_tests.cpp
Line 2641 (original), 2643 (patched)


This could be shortened: `recoverState->slave.get()`



src/tests/slave_recovery_tests.cpp
Lines 2647 (patched)


Strictly speaking this is EXPECT_*

Basically the rule is to prefer `EXPECT_*` and use `ASSERT_*` only when it 
doesn't make any sense to continue the test if this doesn't hold: 
https://github.com/google/googletest/blob/master/googletest/docs/Primer.md#assertions

Not all of these are a huge deal or it's always straighfoward what "doesn't 
make sense to continue" means but we should try to be idiomatic whenever 
possible. :)

Note that there are some bad examples in this file.



src/tests/slave_recovery_tests.cpp
Line 2655 (original), 2661 (patched)


Strictly speaking this is EXPECT_*



src/tests/slave_recovery_tests.cpp
Lines 2667-2668 (patched)


There's `EXPECT_SOME_EQ` to do this in one line.



src/tests/slave_recovery_tests.cpp
Lines 2698 (patched)


Doesn't look like we need to set this.



src/tests/slave_recovery_tests.cpp
Lines 2765 (patched)


Strictly speaking this is EXPECT_*



src/tests/slave_recovery_tests.cpp
Lines 2779 (patched)


Strictly speaking this is EXPECT_*


- Jiang Yan Xu


On July 11, 2017, 5:13 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56895/
> ---
> 
> (Updated July 11, 2017, 5:13 p.m.)
> 
> 
> Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6223
> https://issues.apache.org/jira/browse/MESOS-6223
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests to verify that the state is recovered post reboot and
> agentId is retained given the recovery finishes without errors and
> if the recovery fails due to slaveInfo mismatch then agent no
> state is recoverd making the agent register as a new agent.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp f6eafcbe3d89c7a69c03db0fd7bc10ae73d06584 
> 
> 
> Diff: https://reviews.apache.org/r/56895/diff/16/
> 
> 
> Testing
> ---
> 
> make check done together with 60104 and 60103
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 60397: Check perf version compatibility in tests with disabled coredumps.

[Andrei Budnik]

> On July 11, 2017, 5:48 p.m., Benjamin Mahler wrote:
> > src/linux/perf.cpp
> > Line 238 (original), 243-249 (patched)
> > 
> >
> > This is executing `perf --version` twice, and if someone were to call 
> > `perf::version()` (or other `perf::` functions) without calling 
> > `perf::supported()` then the core dump issue would surface again?
> > 
> > Could we instead just update `perf::version` and all other perf 
> > commands we run to disable core dumps? They all should be going through the 
> > `Perf` class in the implementation.
> > 
> > (I don't think we mandated that any `perf::` function must only be 
> > called after `perf::supported()` has been called and returned true)

`perf --check` is moved to `Perf::execute()`.


- Andrei


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


On July 11, 2017, 9:25 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60397/
> ---
> 
> (Updated July 11, 2017, 9:25 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Benjamin Bannier, James Peach, and 
> Neil Conway.
> 
> 
> Bugs: MESOS-7160
> https://issues.apache.org/jira/browse/MESOS-7160
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools build, the docker-build script performs a 'distcheck'
> build. This type of build warns if any unexpected files are left in
> the build directory after an uninstall, mainly to detect broken
> uninstall Makefile rules. The return status of the build container is
> the result of the distcheck.
> This fixes an issue where in some dockerized configurations
> invocations of 'perf' segfaulted (producing a core file as a
> side-effect), where the failure case was already anticipated and
> handled by the caller.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp b301e25d5cac6488c57e4f983e4867c72368a040 
>   src/tests/containerizer/perf_tests.cpp 
> d8aab08eb131f974821fb85662cbc6cc685d2f3e 
> 
> 
> Diff: https://reviews.apache.org/r/60397/diff/5/
> 
> 
> Testing
> ---
> 
> 1. make check (mac os x 10.12, fedora 25)
> 2. internal CI
> 
> Needs to be confirmed by Apache CI, e.g., reviewbot.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60769: Added evolve functions for resource provider Event/Call.

[Jan Schlicht]

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

(Updated July 12, 2017, 5:01 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Addressed issues.


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


Repository: mesos


Description
---

Added evolve functions for resource provider Event/Call.


Diffs (updated)
-

  src/internal/evolve.hpp 42ead345e94bf68f793d1beb98f7cd8d50fdf994 
  src/internal/evolve.cpp 3ac55ac1f3e4be6130b1772e18325053da8c55c8 


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

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 60494: Expose LinuxLauncher cgroups helper.

[Qian Zhang]

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




src/slave/containerizer/mesos/linux_launcher.cpp
Line 112 (original), 112-115 (patched)


Do we really need this wrapper? I see previously 
`LinuxLauncherProcess::cgroup()` was only called by 
`LinuxLauncherProcess::destroy()`, so what about just changing 
`LinuxLauncherProcess::destroy()` to call `LinuxLauncher::cgroup()` directly?


- 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/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60493: Remove diagnostic socket IPv4 assumptions.

[Qian Zhang]

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




src/linux/routing/diagnosis/diagnosis.cpp
Lines 48-49 (patched)


Add a newline here.


- Qian Zhang


On June 28, 2017, 3:45 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60493/
> ---
> 
> (Updated June 28, 2017, 3:45 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
> ---
> 
> Don't assume the diagnostic socket only returns IPv4 addresses.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/diagnosis/diagnosis.cpp 
> aa2d020fc009040410ef557d10d386d04d71d16e 
> 
> 
> Diff: https://reviews.apache.org/r/60493/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60491: Capture the inode when scanning for sockets.

[Qian Zhang]

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




src/linux/routing/diagnosis/diagnosis.cpp
Line 77 (original), 78 (patched)


Mind to explain a bit about why making this change?
And with this change, I think you do not need to explicitly call the 
constructor of `Info` anymore.


- Qian Zhang


On June 28, 2017, 3:44 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60491/
> ---
> 
> (Updated June 28, 2017, 3:44 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
> ---
> 
> Capture the socket inode in the diagnosis Info when we use netlink
> to enumerate the open sockets. This can be used to indentify which
> process(es) have the socket open.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/diagnosis/diagnosis.hpp 
> 7722fd2843614c3869cc6c3d06bc4c82ed282381 
>   src/linux/routing/diagnosis/diagnosis.cpp 
> aa2d020fc009040410ef557d10d386d04d71d16e 
> 
> 
> Diff: https://reviews.apache.org/r/60491/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60768: Added resource provider ID to calls.

[Jan Schlicht]

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

(Updated July 12, 2017, 4:34 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Addressed issues.


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


Repository: mesos


Description
---

The resource provider manager needs to know from which resource provider
calls originated.


Diffs (updated)
-

  include/mesos/resource_provider/resource_provider.proto 
11d01df114eafb729514dab64596c5f1ecf09f26 
  include/mesos/v1/resource_provider/resource_provider.proto 
97a85ce34eeecf86d14c097712867cd24624a6ec 


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

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 60793: Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [60500, 60557, 60558, 60600, 60760, 60761, 60793]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On July 12, 2017, 6:58 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60793/
> ---
> 
> (Updated July 12, 2017, 6:58 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-7709
> https://issues.apache.org/jira/browse/MESOS-7709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> ae0980bd671849fcd3e19941b33c7d3b09fdae7c 
> 
> 
> Diff: https://reviews.apache.org/r/60793/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60704: Handled EINTR for `waitpid()` in ChildHook::SUPERVISOR from subprocess.

[Andrei Budnik]

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

(Updated July 12, 2017, 7:36 a.m.)


Review request for mesos, Alexander Rukletsov, Alexander Rojas, Benjamin 
Bannier, and James Peach.


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


Repository: mesos


Description
---

Handled EINTR for `waitpid()` in ChildHook::SUPERVISOR from subprocess.


Diffs
-

  3rdparty/libprocess/src/subprocess.cpp 
0f1532b294d6d6b1e017468cfde47362f3faa84d 


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


Testing
---

make check (mac os x, fedora 25)


Thanks,

Andrei Budnik

Re: Review Request 60793: Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.

[Qian Zhang]

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

(Updated July 12, 2017, 2:58 p.m.)


Review request for mesos, Avinash sridharan and Jie Yu.


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


Repository: mesos


Description
---

Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.


Diffs
-

  src/tests/containerizer/cni_isolator_tests.cpp 
ae0980bd671849fcd3e19941b33c7d3b09fdae7c 


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


Testing (updated)
---

sudo make check


Thanks,

Qian Zhang

Review Request 60793: Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.

[Qian Zhang]

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

Review request for mesos, Avinash sridharan and Jie Yu.


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


Repository: mesos


Description
---

Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.


Diffs
-

  src/tests/containerizer/cni_isolator_tests.cpp 
ae0980bd671849fcd3e19941b33c7d3b09fdae7c 


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


Testing
---


Thanks,

Qian Zhang