Re: Review Request 61753: Refactored CMake build system.

[Mesos Reviewbot]

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



Bad review!

Reviews applied: [61753, 61752, 61751, 61750, 61597, 61516, 61515, 61514, 
61513, 61512, 61365]

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

- Mesos Reviewbot


On Aug. 18, 2017, 2:10 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61753/
> ---
> 
> (Updated Aug. 18, 2017, 2:10 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit deletes extraneous CMake code. With a proper CMake
> dependency graph, CMake automatically calculates the correct compiler
> and linker options based on the traversal of the graph, and the
> properties of each target. With the correct importing of the 3rdparty
> dependencies, this allows us to remove all uses of deprecated CMake
> commands such as `include_directories`, `link_directories`, and
> `link_libraries`. These commands rely on setting directory-wide
> side-effects, unlike their explicit `target_include_directories`
> counterparts, which instead add the information to the dependency graph.
> By utilizing CMake's dependency graph, most uses of `add_dependencies`
> were removed, as an implicit dependency is created when
> `target_link_libraries` is used. Note that there are explicit cases of
> manually dependencies, such as module libraries which are not linked and
> therefore are not automatically added to the graph.
> 
> Furthermore, superfluous variables were removed. Due to how the original
> system was written (based on copying a working template), many variables
> were introduced that contained single items, such that the declaration
> and interpolation of the variable became far more verbose than simply
> using the contents directly. Variables holding target names were also
> removed, as they were redundant. See the changes to
> `src/master/CMakeLists.txt` for an example of an executable's build
> going from twelve logical lines of code to two.
> 
> "Configuration" files such as `AgentConfigure.cmake`, were deleted in
> their entirety, because the information they provided was redundant when
> using the CMake dependency graph. The only CMake configuration files
> remaining are for actual project configuration.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 870bd9ba2d5edfe31092d602135edd28dc4982e0 
>   cmake/CompilationConfigure.cmake 2109697e02e58c673948f2ae67ef8255d16aa736 
>   cmake/MesosConfigure.cmake 5ecad2c0fbd245dd14e9c09869dd6da2ca4e602a 
>   src/CMakeLists.txt 0816c6ebaa9caaa5b6a606e601427ed17112a300 
>   src/checks/CMakeLists.txt 4854b2765202e10f5b791f08ed1416f7740e2aad 
>   src/cli/CMakeLists.txt 056d1d734e4424dac3eb29c323961d3a2e87a8dc 
>   src/docker/CMakeLists.txt 2d7c30db5854853625a99227568a27dde281ec2e 
>   src/examples/CMakeLists.txt e8b7ea4bd91c20536ce7bd8d0ffdfd4e549b056f 
>   src/examples/cmake/ExamplesConfigure.cmake 
> d85da4e2e236df9408c86405caeca4d8eae3a35d 
>   src/launcher/CMakeLists.txt 97edc4a28a995da8a5e963aaa62c4c955cc59719 
>   src/local/CMakeLists.txt 7c9a718d17772be1c08e132136739cfd9449470d 
>   src/log/CMakeLists.txt 2fad6cca467dbde9989d26424a2a6c608cd251e8 
>   src/master/CMakeLists.txt 0062fb3e9d67fdfcda1057112d4ba34abbd4be5f 
>   src/master/cmake/MasterConfigure.cmake 
> 173dc36b5a1b7ff23cd0542ea4313ae6e5e0ae3f 
>   src/slave/CMakeLists.txt 9dbadca53b9ac34db08cfb9c9e9fea741c01114b 
>   src/slave/cmake/AgentConfigure.cmake 
> af2f74f82b68920427f36c0c863b97c0191b8f06 
>   src/slave/container_loggers/CMakeLists.txt 
> c0774ce3110fd8d2aac6dc00d2b800e336214f96 
>   src/slave/containerizer/mesos/CMakeLists.txt 
> fa2043efadd1c863c5ad54fdc518397c97488dcb 
>   src/slave/qos_controllers/CMakeLists.txt 
> 65ab338a71276a77e43af962fbc9b76e050efca6 
>   src/slave/resource_estimators/CMakeLists.txt 
> 1d28bd457c7e567219d892f0940903b12c5a877b 
>   src/tests/CMakeLists.txt 6c05ea93c3288fb9608c191234f95e270390d89e 
>   src/tests/cmake/MesosTestsConfigure.cmake 
> 48b4d2fd74b545c03162c7cb06013baf67298241 
>   src/usage/CMakeLists.txt fc87b297832157b679039c68baf9ddc68a292060 
> 
> 
> Diff: https://reviews.apache.org/r/61753/diff/1/
> 
> 
> Testing
> ---
> 
> Tested with `make check` and `ninja-build check` on CentOS 7. All tests pass. 
> Tested with libevent and OpenSSL, all tests passed. Tested with Java, all 
> tests passed (the "example" Java tests which depend on a target that's not 
> yet enabled).
> 
> Tested on Windows with and without Java, all tests passed. With Java requires 
> some environment setup which will be documented.
> 
> Tested release configurations, all tests pass.
> 

Re: Review Request 61189: Added authorization for V1 events.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61262, 61189]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 18, 2017, 8:33 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61189/
> ---
> 
> (Updated Aug. 18, 2017, 8:33 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, and Greg Mann.
> 
> 
> Bugs: MESOS-7785
> https://issues.apache.org/jira/browse/MESOS-7785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization filtering for the V1 operator events on the
> master, the subscriber should only receive events that are
> authorized based on their principal and ACLs. Since the authorizer
> is called for every event emitted on the stream, the change of
> authorization rule will affect events that the subscribers can
> receive.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b09a9d0ab5fab179d0e400932dce7d3ca958d7a8 
>   src/master/master.hpp d9cfc42646c874661e4ec79322126d93a2caf604 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
>   src/tests/api_tests.cpp 34480ea6818c904c64cb678562866f0b43dae0d6 
> 
> 
> Diff: https://reviews.apache.org/r/61189/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> GLOG_v=2 ./bin/mesos-tests.sh 
> --gtest_filter="ContentType/MasterAPITest.EventsAuthorizationFiltering*" 
> --verbose --gtest_repeat=100 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61189: Added authorization for V1 events.

[Quinn Leng]

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

(Updated Aug. 19, 2017, 3:33 a.m.)


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


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


Repository: mesos


Description
---

Added authorization filtering for the V1 operator events on the
master, the subscriber should only receive events that are
authorized based on their principal and ACLs. Since the authorizer
is called for every event emitted on the stream, the change of
authorization rule will affect events that the subscribers can
receive.


Diffs (updated)
-

  src/master/http.cpp b09a9d0ab5fab179d0e400932dce7d3ca958d7a8 
  src/master/master.hpp d9cfc42646c874661e4ec79322126d93a2caf604 
  src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
  src/tests/api_tests.cpp 34480ea6818c904c64cb678562866f0b43dae0d6 


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

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


Testing
---

make check
GLOG_v=2 ./bin/mesos-tests.sh 
--gtest_filter="ContentType/MasterAPITest.EventsAuthorizationFiltering*" 
--verbose --gtest_repeat=100 --gtest_break_on_failure


Thanks,

Quinn Leng

Re: Review Request 61706: Added Autotools rules to build the CSI client module in Mesos.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [61706, 61705, 61704, 61703, 61748, 61600]

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

Error:
error: missing binary patch data for '3rdparty/csi-0.1.0.tar.gz'
error: binary patch does not apply to '3rdparty/csi-0.1.0.tar.gz'
error: 3rdparty/csi-0.1.0.tar.gz: patch does not apply

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

- Mesos Reviewbot Windows


On Aug. 19, 2017, 12:38 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61706/
> ---
> 
> (Updated Aug. 19, 2017, 12:38 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.
> 
> 
> Bugs: mesos-7491
> https://issues.apache.org/jira/browse/mesos-7491
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Autotools rules to build the CSI client module in Mesos.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 21828379197858ae2b92648c1d18d3eb1bc7fb7c 
>   3rdparty/versions.am 82d66be7d25ba9b8753f3c6b60415d991074b1a9 
>   src/Makefile.am 68fff148f3d0c1710305bd9afcba62336d194b55 
> 
> 
> Diff: https://reviews.apache.org/r/61706/diff/2/
> 
> 
> Testing
> ---
> 
> ../configure --enable-grpc
> make distcheck DISTCHECK_CONFIGURE_FLAGS="--enable-grpc"
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61706: Added Autotools rules to build the CSI client module in Mesos.

[Chun-Hung Hsiao]

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

(Updated Aug. 19, 2017, 12:38 a.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.


Changes
---

Updated due to changes upper in the review chain.


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


Repository: mesos


Description
---

Added Autotools rules to build the CSI client module in Mesos.


Diffs (updated)
-

  3rdparty/Makefile.am 21828379197858ae2b92648c1d18d3eb1bc7fb7c 
  3rdparty/versions.am 82d66be7d25ba9b8753f3c6b60415d991074b1a9 
  src/Makefile.am 68fff148f3d0c1710305bd9afcba62336d194b55 


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

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


Testing
---

../configure --enable-grpc
make distcheck DISTCHECK_CONFIGURE_FLAGS="--enable-grpc"


Thanks,

Chun-Hung Hsiao

Re: Review Request 61705: Added a mock CSI plugin and a unit test for CSI client classes.

[Chun-Hung Hsiao]

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

(Updated Aug. 19, 2017, 12:37 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Addressed @jieyu's comments.


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


Repository: mesos


Description (updated)
---

The mock plugin simply starts the `Identity`, `Controller` and `Node`
CSI services and return a success with an empty response protocol buffer
for each RPC. The unit test verifies that each method in the `Client`
class makes the corresponding RPC call through the gRPC interface in
libprocess.


Diffs (updated)
-

  src/tests/csi_client_tests.cpp PRE-CREATION 
  src/tests/mock_csi_plugin.hpp PRE-CREATION 
  src/tests/mock_csi_plugin.cpp PRE-CREATION 


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

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


Testing
---

Tests described in r/61706.


Thanks,

Chun-Hung Hsiao

Re: Review Request 61704: Added CSI client classes to talk to CSI plugins.

[Chun-Hung Hsiao]

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

(Updated Aug. 19, 2017, 12:36 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Addressed @jieyu's comments.


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


Repository: mesos


Description
---

Added CSI client classes to talk to CSI plugins.


Diffs (updated)
-

  src/csi/client.hpp PRE-CREATION 
  src/csi/client.cpp PRE-CREATION 
  src/csi/spec.hpp PRE-CREATION 


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

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


Testing
---

Tests described in r/61706.


Thanks,

Chun-Hung Hsiao

Re: Review Request 61600: Rewrited rules for generating Protobuf and gRPC code.

[Chun-Hung Hsiao]

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

(Updated Aug. 19, 2017, 12:34 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Added a dependency between `tests` and `$(GRPC_TESTS_PROTOS)` since the 
generated code need to be created after `grpc` is built (enforced through 
`BUILT_SOURCES`) but before `grpc_tests.cpp` is compiled.


Summary (updated)
-

Rewrited rules for generating Protobuf and gRPC code.


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


Repository: mesos


Description
---

Made code generation for Proobuf and gRPC more generic and changed the
variable names in `Makefile.am` to follow the same style in Mesos.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am c2fa3f9e81cb57bc9cb43f8f057014668bc49246 


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

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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 61697: Included nested command checks output in the executor logs.

[Gastón Kleiman]

> On Aug. 17, 2017, 2:22 p.m., Alexander Rukletsov wrote:
> > src/checks/checker_process.cpp
> > Lines 855-858 (original), 903-906 (patched)
> > 
> >
> > While we're on it, let's consistently print out HTTP and TCP check 
> > output. Instead of including it into the failure. As a separate patch, 
> > please.

Do you mean that we should always print the output of the HTTP/TCP commands?

We're including it in the Failure here, because in this case curl returned 
something unexpected, otherwise I don't think it makes much sense to print the 
exit status and output of curl.

Anyway, I made the output more consistent here: 
https://reviews.apache.org/r/61766/. Let's continue this conversation there.


> On Aug. 17, 2017, 2:22 p.m., Alexander Rukletsov wrote:
> > src/checks/checker_process.cpp
> > Lines 999-1009 (original), 1047-1057 (patched)
> > 
> >
> > Ditto. Let's make the output consistent.

Ditto https://reviews.apache.org/r/61766/.


- Gastón


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


On Aug. 18, 2017, 11:20 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61697/
> ---
> 
> (Updated Aug. 18, 2017, 11:20 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7861
> https://issues.apache.org/jira/browse/MESOS-7861
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch makes the default executor include the output of the COMMAND
> (health) checks in its logs.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.cpp 30dda0e6efca31aa6b9cd4f753f96b979717ab2e 
> 
> 
> Diff: https://reviews.apache.org/r/61697/diff/3/
> 
> 
> Testing
> ---
> 
> Manual tests.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61766: Made the output handling of TCP and HTTP checks consistent.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [61766, 61697]

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

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/256/console

- Mesos Reviewbot Windows


On Aug. 18, 2017, 5:27 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61766/
> ---
> 
> (Updated Aug. 18, 2017, 5:27 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7861
> https://issues.apache.org/jira/browse/MESOS-7861
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the output handling of TCP and HTTP checks consistent.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.cpp 30dda0e6efca31aa6b9cd4f753f96b979717ab2e 
> 
> 
> Diff: https://reviews.apache.org/r/61766/diff/1/
> 
> 
> Testing
> ---
> 
> `make tests` on GNU/Linux
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Review Request 61766: Made the output handling of TCP and HTTP checks consistent.

[Gastón Kleiman]

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

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


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


Repository: mesos


Description
---

Made the output handling of TCP and HTTP checks consistent.


Diffs
-

  src/checks/checker_process.cpp 30dda0e6efca31aa6b9cd4f753f96b979717ab2e 


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


Testing
---

`make tests` on GNU/Linux


Thanks,

Gastón Kleiman

Re: Review Request 61189: Added authorization for V1 events.

[Greg Mann]

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




src/master/master.cpp
Lines 9626 (patched)


Need a space before the `{`



src/master/master.cpp
Lines 9705-9713 (patched)


If you use `else if` here instead, we can avoid unnecessary comparisons.



src/tests/api_tests.cpp
Lines 2190-2192 (patched)


Maybe:

"Verifies that operators subscribed to the master's operator API event 
stream only receive events that they are authorized to see."



src/tests/api_tests.cpp
Lines 2198-2233 (patched)


Similar to the recent authZ filtering test we did, could you not set the 
permissive bit to false and explicitly specify the ACLs we need? This means you 
could also get rid of the `register_agents` and `register_frameworks` ACLs here.



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


s/use/user/



src/tests/api_tests.cpp
Lines 2261-2262 (patched)


Let's not abbreviate these and just call them `executor1` and `executor2`



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


s/execs/executors/



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


Instead:

```
ASSERT_FALSE(offers->offers().empty());
```



src/tests/api_tests.cpp
Lines 2346-2356 (patched)


Could you put each one of these event-handling sections into a scope?



src/tests/api_tests.cpp
Lines 2358-2362 (patched)


This could go into a scope.



src/tests/api_tests.cpp
Lines 2381-2391 (patched)


Could you move this block above the declaration of `task1`?



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


How about `execTask1` instead?



src/tests/api_tests.cpp
Lines 2434-2436 (patched)


Let's use a separate `Future offers2` object 
here instead of re-using the one from before. In this case, should probably 
rename the first one to `offers1` as well.



src/tests/api_tests.cpp
Lines 2442-2444 (patched)


You should do the same check here: `ASSERT_FALSE(offers->offers().empty());`



src/tests/api_tests.cpp
Lines 2458-2465 (patched)


Let's move this block above the declaration of `task2`.



src/tests/api_tests.cpp
Lines 2458-2460 (patched)


Let's create a new Future for this update rather than re-using the first 
one.



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


How about `execTask2` instead?



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


Do we need this?



src/tests/api_tests.cpp
Lines 2493-2498 (patched)


Status updates will be retried until the scheduler acknowledges them. To 
prevent this, you can have the scheduler ack the update immediately after it's 
received, like this: 
https://github.com/apache/mesos/blob/4d0e692e72e56e827d2136b78311db0b72ada6a2/src/tests/scheduler_tests.cpp#L690-L702


- Greg Mann


On Aug. 18, 2017, 9:53 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61189/
> ---
> 
> (Updated Aug. 18, 2017, 9:53 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, and Greg Mann.
> 
> 
> Bugs: MESOS-7785
> https://issues.apache.org/jira/browse/MESOS-7785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization filtering for the V1 operator events on the
> master, the subscriber should only receive events that are
> authorized based on their principal and ACLs. Since the authorizer
> is called for every event emitted on the stream, the change of
> authorization rule will affect events that the subscribers can
> receive.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
>   src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
>   src/tests/api_tests.cpp 

Re: Review Request 61600: WIP: Rewrited rules for generating Protobuf and gRPC code.

[Chun-Hung Hsiao]

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

(Updated Aug. 18, 2017, 11:58 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Summary (updated)
-

WIP: Rewrited rules for generating Protobuf and gRPC code.


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


Repository: mesos


Description
---

Made code generation for Proobuf and gRPC more generic and changed the
variable names in `Makefile.am` to follow the same style in Mesos.


Diffs
-

  3rdparty/libprocess/Makefile.am c2fa3f9e81cb57bc9cb43f8f057014668bc49246 


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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 61763: Added heartbeat interval for V1 Operator API.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61759, 61763]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 18, 2017, 4:40 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61763/
> ---
> 
> (Updated Aug. 18, 2017, 4:40 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 interval parameter to the SUBSCRIBED event for
> V1 operator API. Updated the heartbeat test case to check the
> heartbeat interval.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 7dc5881b843addc2ec59fadb45f31049c7e5a588 
>   include/mesos/v1/master/master.proto 
> db19c5ccc8aca78f901e72f3caf6d46761270bdd 
>   src/master/http.cpp b09a9d0ab5fab179d0e400932dce7d3ca958d7a8 
>   src/tests/api_tests.cpp 34480ea6818c904c64cb678562866f0b43dae0d6 
> 
> 
> Diff: https://reviews.apache.org/r/61763/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61697: Included nested command checks output in the executor logs.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [61697]

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

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/256/console

- Mesos Reviewbot Windows


On Aug. 18, 2017, 4:20 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61697/
> ---
> 
> (Updated Aug. 18, 2017, 4:20 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7861
> https://issues.apache.org/jira/browse/MESOS-7861
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch makes the default executor include the output of the COMMAND
> (health) checks in its logs.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.cpp 30dda0e6efca31aa6b9cd4f753f96b979717ab2e 
> 
> 
> Diff: https://reviews.apache.org/r/61697/diff/3/
> 
> 
> Testing
> ---
> 
> Manual tests.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61763: Added heartbeat interval for V1 Operator API.

[Quinn Leng]

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

(Updated Aug. 18, 2017, 11:40 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 interval parameter to the SUBSCRIBED event for
V1 operator API. Updated the heartbeat test case to check the
heartbeat interval.


Diffs (updated)
-

  include/mesos/master/master.proto 7dc5881b843addc2ec59fadb45f31049c7e5a588 
  include/mesos/v1/master/master.proto db19c5ccc8aca78f901e72f3caf6d46761270bdd 
  src/master/http.cpp b09a9d0ab5fab179d0e400932dce7d3ca958d7a8 
  src/tests/api_tests.cpp 34480ea6818c904c64cb678562866f0b43dae0d6 


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

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


Testing
---

make check


Thanks,

Quinn Leng

Re: Review Request 61640: Improved the reason and message for killed queued tasks updates.

[Vinod Kone]

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




src/slave/slave.cpp
Lines 3033 (patched)


Can you add a TODO to the protobuf that contains this enum saying that it 
is no longer used.

Also, should we use a better named enum for this case? I would like to move 
towards all mesos generated updates to have reasons.


- Vinod Kone


On Aug. 15, 2017, 7 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61640/
> ---
> 
> (Updated Aug. 15, 2017, 7 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, if a task was killed before delivery to a registering
> executor, we sent REASON_EXECUTOR_UNREGISTERED with a message of
> "Unregistered executor". Whereas this case is just that the task
> was killed while queued.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 
>   src/tests/slave_recovery_tests.cpp 9ba6f6005c2edce67e1f63005518ab71ac981423 
>   src/tests/slave_tests.cpp 1d9d142ed9e801b79535a2c28f5a94ffbf1bf160 
> 
> 
> Diff: https://reviews.apache.org/r/61640/diff/1/
> 
> 
> Testing
> ---
> 
> Updated the tests.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 61763: Added heartbeat interval for V1 Operator API.

[Greg Mann]

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




src/tests/api_tests.cpp
Line 2440 (original), 2447 (patched)


You can move this statement before the `for` loop (i.e. we only need to 
call it once).



src/tests/api_tests.cpp
Line 2448 (original), 2455 (patched)


Since you paused the clock above, resume the clock right before the end of 
the test:

```
Clock::resume();
```


- Greg Mann


On Aug. 18, 2017, 11:11 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61763/
> ---
> 
> (Updated Aug. 18, 2017, 11:11 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 interval parameter to the SUBSCRIBED event for
> V1 operator API. Updated the heartbeat test case to check the
> heartbeat interval.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 7dc5881b843addc2ec59fadb45f31049c7e5a588 
>   include/mesos/v1/master/master.proto 
> db19c5ccc8aca78f901e72f3caf6d46761270bdd 
>   src/master/http.cpp b09a9d0ab5fab179d0e400932dce7d3ca958d7a8 
>   src/tests/api_tests.cpp 34480ea6818c904c64cb678562866f0b43dae0d6 
> 
> 
> Diff: https://reviews.apache.org/r/61763/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61639: Fixed an bug where the agent kills and still launches a task.

[Vinod Kone]

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


Fix it, then Ship it!





src/slave/slave.cpp
Lines 4502 (patched)


Can you expand a bit more in the comment on why we need to synchronously 
transition the task, for posterity?


- Vinod Kone


On Aug. 15, 2017, 7 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61639/
> ---
> 
> (Updated Aug. 15, 2017, 7 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-7744 and MESOS-7865
> https://issues.apache.org/jira/browse/MESOS-7744
> https://issues.apache.org/jira/browse/MESOS-7865
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The following race leads to the agent both killing and launching a task:
> 
>   (1) Slave::__run completes, task is now within Executor::queuedTasks.
>   (2) Slave::killTask locates the executor based on the task ID residing
>   in queuedTasks, calls Slave::statusUpdate() with TASK_KILLED.
>   (3) Slave::___run assumes that killed tasks have been removed from
>   Executor::queuedTasks, but this now occurs asynchronously in
>   Slave::_statusUpdate. So, the agent still sees the queued task
>   and delivers it and adds the task to Executor::launchedTasks.
>   (3) Slave::_statusUpdate runs, removes the task from
>   Executor::launchedTasks and adds it to Executor::terminatedTasks.
> 
> The fix applied here is to synchronously transition queued tasks to
> a terminal state when statusUpdate is called. This can be done because
> for queued tasks, we do not need to retrieve the container status (the
> task never reached the container).
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 1fe93dab1b2bef24721cc1bcffebe1b259e96d79 
>   src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 
> 
> 
> Diff: https://reviews.apache.org/r/61639/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> SlaveTest.KillQueuedTaskDuringExecutorRegistration captures this case, but it 
> did not delay retrieving the container status. This test could have been 
> updated previously to delay the container status, but now there is no 
> container status to delay, so I've left the test alone.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 61697: Included nested command checks output in the executor logs.

[Gastón Kleiman]

> On Aug. 17, 2017, 2:22 p.m., Alexander Rukletsov wrote:
> > src/checks/checker_process.cpp
> > Lines 630-631 (patched)
> > 
> >
> > I assume line feed and carriage return characters are included in the 
> > output. In this case, how a reader can distinguish between executor output 
> > and check output? Is it because check output lines will not be prepended by 
> > a timestamp? Have you tested it and checked that the output is sane?

Exaclty, the check output lines will not be prepended with anything, here's an 
example output with the health check command `ls >&2 && echo 'foo bar baz' && 
true`:

```
I0818 16:24:04.182832 36687 checker_process.cpp:639] Output of the COMMAND 
health check for task 'test-task-id' (stdout):
foo bar baz
I0818 16:24:04.182896 36687 checker_process.cpp:643] Output of the COMMAND 
health check for task 'test-task-id' (stderr):
containers
stderr
stdout
```


- Gastón


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


On Aug. 18, 2017, 11:20 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61697/
> ---
> 
> (Updated Aug. 18, 2017, 11:20 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7861
> https://issues.apache.org/jira/browse/MESOS-7861
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch makes the default executor include the output of the COMMAND
> (health) checks in its logs.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.cpp 30dda0e6efca31aa6b9cd4f753f96b979717ab2e 
> 
> 
> Diff: https://reviews.apache.org/r/61697/diff/3/
> 
> 
> Testing
> ---
> 
> Manual tests.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61697: Included nested command checks output in the executor logs.

[Gastón Kleiman]

> On Aug. 16, 2017, 11:03 p.m., Greg Mann wrote:
> > src/checks/checker_process.cpp
> > Lines 145 (patched)
> > 
> >
> > s/It/This function/

Fixed, I guess that we'll wnat to change 
https://github.com/apache/mesos/blob/4d0e692e72e56e827d2136b78311db0b72ada6a2/src/tests/api_tests.cpp#L3036-L3038
 as well?


- Gastón


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


On Aug. 18, 2017, 11:20 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61697/
> ---
> 
> (Updated Aug. 18, 2017, 11:20 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7861
> https://issues.apache.org/jira/browse/MESOS-7861
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch makes the default executor include the output of the COMMAND
> (health) checks in its logs.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.cpp 30dda0e6efca31aa6b9cd4f753f96b979717ab2e 
> 
> 
> Diff: https://reviews.apache.org/r/61697/diff/3/
> 
> 
> Testing
> ---
> 
> Manual tests.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61697: Included nested command checks output in the executor logs.

[Gastón Kleiman]

> On Aug. 17, 2017, 2:22 p.m., Alexander Rukletsov wrote:
> > src/checks/checker_process.cpp
> > Lines 144-145 (patched)
> > 
> >
> > Insert a blank line here.

Ditto 
https://github.com/apache/mesos/blob/4d0e692e72e56e827d2136b78311db0b72ada6a2/src/tests/api_tests.cpp#L3036-L3038.


- Gastón


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


On Aug. 18, 2017, 11:20 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61697/
> ---
> 
> (Updated Aug. 18, 2017, 11:20 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7861
> https://issues.apache.org/jira/browse/MESOS-7861
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch makes the default executor include the output of the COMMAND
> (health) checks in its logs.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.cpp 30dda0e6efca31aa6b9cd4f753f96b979717ab2e 
> 
> 
> Diff: https://reviews.apache.org/r/61697/diff/3/
> 
> 
> Testing
> ---
> 
> Manual tests.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61697: Included nested command checks output in the executor logs.

[Gastón Kleiman]

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

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


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


Changes
---

Addressed feedback.


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


Repository: mesos


Description
---

This patch makes the default executor include the output of the COMMAND
(health) checks in its logs.


Diffs (updated)
-

  src/checks/checker_process.cpp 30dda0e6efca31aa6b9cd4f753f96b979717ab2e 


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

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


Testing
---

Manual tests.


Thanks,

Gastón Kleiman

Re: Review Request 61763: Added heartbeat interval for V1 Operator API.

[Greg Mann]

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


Fix it, then Ship it!





src/tests/api_tests.cpp
Lines 2420-2422 (patched)


Before you do this, do 
`ASSERT_TRUE(event->get().subscribed().has_heartbeat_interval_seconds());` to 
make sure we don't crash during this statement.



src/tests/api_tests.cpp
Line 2441 (original), 2448 (patched)


Can't you just use `event->get().subscribed().heartbeat_interval_seconds()` 
here directly, and get rid of the temporary variable?


- Greg Mann


On Aug. 18, 2017, 11:11 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61763/
> ---
> 
> (Updated Aug. 18, 2017, 11:11 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 interval parameter to the SUBSCRIBED event for
> V1 operator API. Updated the heartbeat test case to check the
> heartbeat interval.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 7dc5881b843addc2ec59fadb45f31049c7e5a588 
>   include/mesos/v1/master/master.proto 
> db19c5ccc8aca78f901e72f3caf6d46761270bdd 
>   src/master/http.cpp b09a9d0ab5fab179d0e400932dce7d3ca958d7a8 
>   src/tests/api_tests.cpp 34480ea6818c904c64cb678562866f0b43dae0d6 
> 
> 
> Diff: https://reviews.apache.org/r/61763/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Review Request 61763: Added heartbeat interval for V1 Operator API.

[Quinn Leng]

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

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 interval parameter to the SUBSCRIBED event for
V1 operator API. Updated the heartbeat test case to check the
heartbeat interval.


Diffs
-

  include/mesos/master/master.proto 7dc5881b843addc2ec59fadb45f31049c7e5a588 
  include/mesos/v1/master/master.proto db19c5ccc8aca78f901e72f3caf6d46761270bdd 
  src/master/http.cpp b09a9d0ab5fab179d0e400932dce7d3ca958d7a8 
  src/tests/api_tests.cpp 34480ea6818c904c64cb678562866f0b43dae0d6 


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


Testing
---

make check


Thanks,

Quinn Leng

Re: Review Request 61759: Updated the docs for V1 HEARTBEAT Event.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61759]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 18, 2017, 10:04 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61759/
> ---
> 
> (Updated Aug. 18, 2017, 10:04 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
> ---
> 
> Updated the docs for V1 HEARTBEAT Event.
> 
> 
> Diffs
> -
> 
>   docs/operator-http-api.md c56f3b567b07b73602d6cbd7666fc2a31b2475c9 
> 
> 
> Diff: https://reviews.apache.org/r/61759/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61759: Updated the docs for V1 HEARTBEAT Event.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Aug. 18, 2017, 10:04 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61759/
> ---
> 
> (Updated Aug. 18, 2017, 10:04 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
> ---
> 
> Updated the docs for V1 HEARTBEAT Event.
> 
> 
> Diffs
> -
> 
>   docs/operator-http-api.md c56f3b567b07b73602d6cbd7666fc2a31b2475c9 
> 
> 
> Diff: https://reviews.apache.org/r/61759/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

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


Ship it!




Ship It!

- Greg Mann


On Aug. 17, 2017, 9:33 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61408/
> ---
> 
> (Updated Aug. 17, 2017, 9:33 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/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

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

[Andrew Schwartzmeyer]

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


Fix it, then Ship it!




Ship It!


support/apply-reviews.py
Line 209 (original), 209 (patched)


As long as we're fixing things, `GitHub` :)


- Andrew Schwartzmeyer


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
> 
>

Review Request 61759: Updated the docs for V1 HEARTBEAT Event.

[Quinn Leng]

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

Review request for mesos, Anand Mazumdar and Greg Mann.


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


Repository: mesos


Description
---

Updated the docs for V1 HEARTBEAT Event.


Diffs
-

  docs/operator-http-api.md c56f3b567b07b73602d6cbd7666fc2a31b2475c9 


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


Testing
---


Thanks,

Quinn Leng

Re: Review Request 61189: Added authorization for V1 events.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [61189, 61262]

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

Error:
error: patch failed: include/mesos/master/master.proto:484
error: include/mesos/master/master.proto: patch does not apply
error: patch failed: include/mesos/v1/master/master.proto:484
error: include/mesos/v1/master/master.proto: patch does not apply
error: patch failed: src/master/http.cpp:855
error: src/master/http.cpp: patch does not apply
error: patch failed: src/master/master.hpp:1829
error: src/master/master.hpp: patch does not apply
error: patch failed: src/tests/api_tests.cpp:73
error: src/tests/api_tests.cpp: patch does not apply

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

- Mesos Reviewbot Windows


On Aug. 18, 2017, 9:53 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61189/
> ---
> 
> (Updated Aug. 18, 2017, 9:53 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, and Greg Mann.
> 
> 
> Bugs: MESOS-7785
> https://issues.apache.org/jira/browse/MESOS-7785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization filtering for the V1 operator events on the
> master, the subscriber should only receive events that are
> authorized based on their principal and ACLs. Since the authorizer
> is called for every event emitted on the stream, the change of
> authorization rule will affect events that the subscribers can
> receive.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
>   src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
>   src/tests/api_tests.cpp 3ab4740bcac29ecb89585da6adb1f563d6fc1f5f 
> 
> 
> Diff: https://reviews.apache.org/r/61189/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> GLOG_v=2 ./bin/mesos-tests.sh 
> --gtest_filter="ContentType/MasterAPITest.EventsAuthorizationFiltering*" 
> --verbose --gtest_repeat=100 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61189: Added authorization for V1 events.

[Quinn Leng]

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

(Updated Aug. 18, 2017, 9:53 p.m.)


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


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


Repository: mesos


Description
---

Added authorization filtering for the V1 operator events on the
master, the subscriber should only receive events that are
authorized based on their principal and ACLs. Since the authorizer
is called for every event emitted on the stream, the change of
authorization rule will affect events that the subscribers can
receive.


Diffs (updated)
-

  src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
  src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
  src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
  src/tests/api_tests.cpp 3ab4740bcac29ecb89585da6adb1f563d6fc1f5f 


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

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


Testing
---

make check
GLOG_v=2 ./bin/mesos-tests.sh 
--gtest_filter="ContentType/MasterAPITest.EventsAuthorizationFiltering*" 
--verbose --gtest_repeat=100 --gtest_break_on_failure


Thanks,

Quinn Leng

Re: Review Request 61189: Added authorization for V1 events.

[Greg Mann]

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




src/master/master.hpp
Lines 1832-1833 (original), 1835-1841 (patched)


It looks like in this file, the indentation style we follow more often is:

```
Subscriber(Master* _master,
   const HttpConnection& _http,
   const Option 
_principal)
  : master(_master),
http(_http),
principal(_principal) {}
```



src/master/master.hpp
Lines 1848-1851 (patched)


Can we pass these by const ref?


- Greg Mann


On Aug. 17, 2017, 10:59 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61189/
> ---
> 
> (Updated Aug. 17, 2017, 10:59 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, and Greg Mann.
> 
> 
> Bugs: MESOS-7785
> https://issues.apache.org/jira/browse/MESOS-7785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization filtering for the V1 operator events on the
> master, the subscriber should only receive events that are
> authorized based on their principal and ACLs. Since the authorizer
> is called for every event emitted on the stream, the change of
> authorization rule will affect events that the subscribers can
> receive.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
>   src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
>   src/tests/api_tests.cpp 3ab4740bcac29ecb89585da6adb1f563d6fc1f5f 
> 
> 
> Diff: https://reviews.apache.org/r/61189/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> GLOG_v=2 ./bin/mesos-tests.sh 
> --gtest_filter="ContentType/MasterAPITest.EventsAuthorizationFiltering*" 
> --verbose --gtest_repeat=100 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61675: Updated doc for the changes of pid namespace sharing.

[Vinod Kone]

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


Fix it, then Ship it!





docs/mesos-containerizer.md
Lines 79 (patched)


s/concern/measure/


- Vinod Kone


On Aug. 15, 2017, 10:50 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61675/
> ---
> 
> (Updated Aug. 15, 2017, 10:50 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Gastón Kleiman, Greg Mann, Jie Yu, 
> Qian Zhang, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated doc for the changes of pid namespace sharing.
> 
> 
> Diffs
> -
> 
>   docs/mesos-containerizer.md feaaa556c9646435c6c9d677569962576ebf4c74 
> 
> 
> Diff: https://reviews.apache.org/r/61675/diff/1/
> 
> 
> Testing
> ---
> 
> N/A.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 60592: Configure the `network/ports` isolator watch interval.

[James Peach]

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

(Updated Aug. 18, 2017, 9:17 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
---

Added the `--container_ports_watch_interval` option to tune the
interval at which the `network/ports` isolator scans for rogue
listening ports.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 
  src/slave/flags.hpp 2970fea0cfac6af275a758d4bfedfe9a943c2b60 
  src/slave/flags.cpp 3b02f3e909a554f15104739832ae3f252926b45f 


Diff: https://reviews.apache.org/r/60592/diff/10/

Changes: https://reviews.apache.org/r/60592/diff/9-10/


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 61753: Refactored CMake build system.

[Mesos Reviewbot Windows]

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



Bad review!

Reviews applied: [61753, 61752, 61751, 61750, 61597, 61516, 61515, 61514, 
61513, 61512, 61365]

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

- Mesos Reviewbot Windows


On Aug. 18, 2017, 9:10 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61753/
> ---
> 
> (Updated Aug. 18, 2017, 9:10 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit deletes extraneous CMake code. With a proper CMake
> dependency graph, CMake automatically calculates the correct compiler
> and linker options based on the traversal of the graph, and the
> properties of each target. With the correct importing of the 3rdparty
> dependencies, this allows us to remove all uses of deprecated CMake
> commands such as `include_directories`, `link_directories`, and
> `link_libraries`. These commands rely on setting directory-wide
> side-effects, unlike their explicit `target_include_directories`
> counterparts, which instead add the information to the dependency graph.
> By utilizing CMake's dependency graph, most uses of `add_dependencies`
> were removed, as an implicit dependency is created when
> `target_link_libraries` is used. Note that there are explicit cases of
> manually dependencies, such as module libraries which are not linked and
> therefore are not automatically added to the graph.
> 
> Furthermore, superfluous variables were removed. Due to how the original
> system was written (based on copying a working template), many variables
> were introduced that contained single items, such that the declaration
> and interpolation of the variable became far more verbose than simply
> using the contents directly. Variables holding target names were also
> removed, as they were redundant. See the changes to
> `src/master/CMakeLists.txt` for an example of an executable's build
> going from twelve logical lines of code to two.
> 
> "Configuration" files such as `AgentConfigure.cmake`, were deleted in
> their entirety, because the information they provided was redundant when
> using the CMake dependency graph. The only CMake configuration files
> remaining are for actual project configuration.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 870bd9ba2d5edfe31092d602135edd28dc4982e0 
>   cmake/CompilationConfigure.cmake 2109697e02e58c673948f2ae67ef8255d16aa736 
>   cmake/MesosConfigure.cmake 5ecad2c0fbd245dd14e9c09869dd6da2ca4e602a 
>   src/CMakeLists.txt 0816c6ebaa9caaa5b6a606e601427ed17112a300 
>   src/checks/CMakeLists.txt 4854b2765202e10f5b791f08ed1416f7740e2aad 
>   src/cli/CMakeLists.txt 056d1d734e4424dac3eb29c323961d3a2e87a8dc 
>   src/docker/CMakeLists.txt 2d7c30db5854853625a99227568a27dde281ec2e 
>   src/examples/CMakeLists.txt e8b7ea4bd91c20536ce7bd8d0ffdfd4e549b056f 
>   src/examples/cmake/ExamplesConfigure.cmake 
> d85da4e2e236df9408c86405caeca4d8eae3a35d 
>   src/launcher/CMakeLists.txt 97edc4a28a995da8a5e963aaa62c4c955cc59719 
>   src/local/CMakeLists.txt 7c9a718d17772be1c08e132136739cfd9449470d 
>   src/log/CMakeLists.txt 2fad6cca467dbde9989d26424a2a6c608cd251e8 
>   src/master/CMakeLists.txt 0062fb3e9d67fdfcda1057112d4ba34abbd4be5f 
>   src/master/cmake/MasterConfigure.cmake 
> 173dc36b5a1b7ff23cd0542ea4313ae6e5e0ae3f 
>   src/slave/CMakeLists.txt 9dbadca53b9ac34db08cfb9c9e9fea741c01114b 
>   src/slave/cmake/AgentConfigure.cmake 
> af2f74f82b68920427f36c0c863b97c0191b8f06 
>   src/slave/container_loggers/CMakeLists.txt 
> c0774ce3110fd8d2aac6dc00d2b800e336214f96 
>   src/slave/containerizer/mesos/CMakeLists.txt 
> fa2043efadd1c863c5ad54fdc518397c97488dcb 
>   src/slave/qos_controllers/CMakeLists.txt 
> 65ab338a71276a77e43af962fbc9b76e050efca6 
>   src/slave/resource_estimators/CMakeLists.txt 
> 1d28bd457c7e567219d892f0940903b12c5a877b 
>   src/tests/CMakeLists.txt 6c05ea93c3288fb9608c191234f95e270390d89e 
>   src/tests/cmake/MesosTestsConfigure.cmake 
> 48b4d2fd74b545c03162c7cb06013baf67298241 
>   src/usage/CMakeLists.txt fc87b297832157b679039c68baf9ddc68a292060 
> 
> 
> Diff: https://reviews.apache.org/r/61753/diff/1/
> 
> 
> Testing
> ---
> 
> Tested with `make check` and `ninja-build check` on CentOS 7. All tests pass. 
> Tested with libevent and OpenSSL, all tests passed. Tested with Java, all 
> tests passed (the "example" Java tests which depend on a target that's not 
> yet enabled).
> 
> Tested on Windows with and without Java, all tests passed. With Java requires 
> some environment setup which will be documented.
> 
> Tested release configurations, all tests 

Review Request 61753: Refactored CMake build system.

[Andrew Schwartzmeyer]

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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

This commit deletes extraneous CMake code. With a proper CMake
dependency graph, CMake automatically calculates the correct compiler
and linker options based on the traversal of the graph, and the
properties of each target. With the correct importing of the 3rdparty
dependencies, this allows us to remove all uses of deprecated CMake
commands such as `include_directories`, `link_directories`, and
`link_libraries`. These commands rely on setting directory-wide
side-effects, unlike their explicit `target_include_directories`
counterparts, which instead add the information to the dependency graph.
By utilizing CMake's dependency graph, most uses of `add_dependencies`
were removed, as an implicit dependency is created when
`target_link_libraries` is used. Note that there are explicit cases of
manually dependencies, such as module libraries which are not linked and
therefore are not automatically added to the graph.

Furthermore, superfluous variables were removed. Due to how the original
system was written (based on copying a working template), many variables
were introduced that contained single items, such that the declaration
and interpolation of the variable became far more verbose than simply
using the contents directly. Variables holding target names were also
removed, as they were redundant. See the changes to
`src/master/CMakeLists.txt` for an example of an executable's build
going from twelve logical lines of code to two.

"Configuration" files such as `AgentConfigure.cmake`, were deleted in
their entirety, because the information they provided was redundant when
using the CMake dependency graph. The only CMake configuration files
remaining are for actual project configuration.


Diffs
-

  CMakeLists.txt 870bd9ba2d5edfe31092d602135edd28dc4982e0 
  cmake/CompilationConfigure.cmake 2109697e02e58c673948f2ae67ef8255d16aa736 
  cmake/MesosConfigure.cmake 5ecad2c0fbd245dd14e9c09869dd6da2ca4e602a 
  src/CMakeLists.txt 0816c6ebaa9caaa5b6a606e601427ed17112a300 
  src/checks/CMakeLists.txt 4854b2765202e10f5b791f08ed1416f7740e2aad 
  src/cli/CMakeLists.txt 056d1d734e4424dac3eb29c323961d3a2e87a8dc 
  src/docker/CMakeLists.txt 2d7c30db5854853625a99227568a27dde281ec2e 
  src/examples/CMakeLists.txt e8b7ea4bd91c20536ce7bd8d0ffdfd4e549b056f 
  src/examples/cmake/ExamplesConfigure.cmake 
d85da4e2e236df9408c86405caeca4d8eae3a35d 
  src/launcher/CMakeLists.txt 97edc4a28a995da8a5e963aaa62c4c955cc59719 
  src/local/CMakeLists.txt 7c9a718d17772be1c08e132136739cfd9449470d 
  src/log/CMakeLists.txt 2fad6cca467dbde9989d26424a2a6c608cd251e8 
  src/master/CMakeLists.txt 0062fb3e9d67fdfcda1057112d4ba34abbd4be5f 
  src/master/cmake/MasterConfigure.cmake 
173dc36b5a1b7ff23cd0542ea4313ae6e5e0ae3f 
  src/slave/CMakeLists.txt 9dbadca53b9ac34db08cfb9c9e9fea741c01114b 
  src/slave/cmake/AgentConfigure.cmake af2f74f82b68920427f36c0c863b97c0191b8f06 
  src/slave/container_loggers/CMakeLists.txt 
c0774ce3110fd8d2aac6dc00d2b800e336214f96 
  src/slave/containerizer/mesos/CMakeLists.txt 
fa2043efadd1c863c5ad54fdc518397c97488dcb 
  src/slave/qos_controllers/CMakeLists.txt 
65ab338a71276a77e43af962fbc9b76e050efca6 
  src/slave/resource_estimators/CMakeLists.txt 
1d28bd457c7e567219d892f0940903b12c5a877b 
  src/tests/CMakeLists.txt 6c05ea93c3288fb9608c191234f95e270390d89e 
  src/tests/cmake/MesosTestsConfigure.cmake 
48b4d2fd74b545c03162c7cb06013baf67298241 
  src/usage/CMakeLists.txt fc87b297832157b679039c68baf9ddc68a292060 


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


Testing
---

Tested with `make check` and `ninja-build check` on CentOS 7. All tests pass. 
Tested with libevent and OpenSSL, all tests passed. Tested with Java, all tests 
passed (the "example" Java tests which depend on a target that's not yet 
enabled).

Tested on Windows with and without Java, all tests passed. With Java requires 
some environment setup which will be documented.

Tested release configurations, all tests pass.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 61752: Audited linkage of 3rdparty dependencies.

[Mesos Reviewbot Windows]

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



Bad review!

Reviews applied: [61752, 61751, 61750, 61597, 61516, 61515, 61514, 61513, 
61512, 61365]

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

- Mesos Reviewbot Windows


On Aug. 18, 2017, 9:04 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61752/
> ---
> 
> (Updated Aug. 18, 2017, 9:04 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch forwards the `CMAKE_POSITION_INDEPENDENT_CODE` property to
> 3rdparty projects built with CMake.
> 
> It marks as `GLOBAL` imported (non-interface) libraries to enable Cotire
> to find the targets. Note that while "global" is usually frowned upon,
> it makes sense for these targets as they would be top-level targets if
> they were not also 3rdparty targets, causing us to keep them separate.
> 
> It adds `IMPORTED_IMPLIB` locations for dependencies which can be built
> as shared libraries (`BUILD_SHARED_LIBS=ON`) so that CMake can link to
> them correctly. For the libraries that cannot be built in this way, the
> linkage was set explicitly and appropriately (e.g. Googletest must
> always be built statically).
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt f4feaf90b4750338cfa2127524a5e6556a86d063 
> 
> 
> Diff: https://reviews.apache.org/r/61752/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 61752: Audited linkage of 3rdparty dependencies.

[Andrew Schwartzmeyer]

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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

This patch forwards the `CMAKE_POSITION_INDEPENDENT_CODE` property to
3rdparty projects built with CMake.

It marks as `GLOBAL` imported (non-interface) libraries to enable Cotire
to find the targets. Note that while "global" is usually frowned upon,
it makes sense for these targets as they would be top-level targets if
they were not also 3rdparty targets, causing us to keep them separate.

It adds `IMPORTED_IMPLIB` locations for dependencies which can be built
as shared libraries (`BUILD_SHARED_LIBS=ON`) so that CMake can link to
them correctly. For the libraries that cannot be built in this way, the
linkage was set explicitly and appropriately (e.g. Googletest must
always be built statically).


Diffs
-

  3rdparty/CMakeLists.txt f4feaf90b4750338cfa2127524a5e6556a86d063 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 61750: Updated ZooKeeper patch to link with /MTd in Debug configurations.

[Andrew Schwartzmeyer]

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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

This patch has also been submitted upstream to ZooKeeper. This
eliminates: "warning LNK4098: defaultlib 'libcmt.lib' conflicts with use
of other libs".


Diffs
-

  3rdparty/zookeeper-3.4.8.patch 486df1ae96af3426835c9d47ff2e36dd47ccde3f 


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


Testing
---

This patch has since been accepted to ZooKeeper and backported to both 3.4 and 
3.5 branches. When 3.4.11 is released, we can update our bundle and remove our 
patches.


Thanks,

Andrew Schwartzmeyer

Review Request 61751: Updated Cotire.cmake module to version 1.7.10.

[Andrew Schwartzmeyer]

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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

Commit 516d78476f6dda336181fc5514f72774e1e0a8e2.


Diffs
-

  3rdparty/cmake/cotire.cmake ab611007dc4e5ed872a629b99b5cde659c8eb7f2 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 60496: Added socket checking to the network ports isolator.

[James Peach]

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

(Updated Aug. 18, 2017, 9 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
---

Implemented ports resource restrictions in the network ports isolator.
Periodically, scan for listening sockets and match them up to all
the open sockets in the containers we are tracking in the network.
Check any sockets we find against the ports resource and trigger a
resource limitation if the port has not been allocated.


Diffs (updated)
-

  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/60496/diff/12/

Changes: https://reviews.apache.org/r/60496/diff/11-12/


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 60496: Added socket checking to the network ports isolator.

[James Peach]

> On Aug. 18, 2017, 2:39 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/network/ports.cpp
> > Lines 362 (patched)
> > 
> >
> > I see you put the actual used resources (i.e. unallocated ports) here, 
> > however I am a bit confused if we should do it or should put the requested 
> > resources (allocated ports) here. In 
> > `PosixDiskIsolatorProcess::_collect()`, it seems we use the requested 
> > resource (disk quota), but in `MemorySubsystem::oomWaited()`, it seems we 
> > use the actual used resources.
> > 
> > I think what you did here is correct, so we may need to update the code 
> > of `PosixDiskIsolatorProcess::_collect()`?

Let's not update any other isolators in this review chain. I think in the case 
of the other isolators, it doesn't matter how much you exceed the resource by 
so it's not that important how the resource is reported. This isolator is a 
litle different because reporting the rogue ports is helpful for diagnostics.


- James


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


On Aug. 17, 2017, 5:36 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60496/
> ---
> 
> (Updated Aug. 17, 2017, 5:36 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
> ---
> 
> Implemented ports resource restrictions in the network ports isolator.
> Periodically, scan for listening sockets and match them up to all
> the open sockets in the containers we are tracking in the network.
> Check any sockets we find against the ports resource and trigger a
> resource limitation if the port has not been allocated.
> 
> 
> 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/60496/diff/11/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 61600: Rewrited rules for generating Protobuf and gRPC code.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Aug. 18, 2017, 6:58 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61600/
> ---
> 
> (Updated Aug. 18, 2017, 6:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7889
> https://issues.apache.org/jira/browse/MESOS-7889
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made code generation for Proobuf and gRPC more generic and changed the
> variable names in `Makefile.am` to follow the same style in Mesos.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am c2fa3f9e81cb57bc9cb43f8f057014668bc49246 
> 
> 
> Diff: https://reviews.apache.org/r/61600/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

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



Patch looks great!

Reviews applied: [61262]

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

- Mesos Reviewbot Windows


On Aug. 18, 2017, 6:54 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61262/
> ---
> 
> (Updated Aug. 18, 2017, 6:54 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 fc5bd894ce55fe8e946d4c5b4b33d3c0505f3c2b 
>   include/mesos/v1/master/master.proto 
> c3fb31de2509adcdec8204f8bbe46b46a31540e8 
>   src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
>   src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
>   src/tests/api_tests.cpp 3ab4740bcac29ecb89585da6adb1f563d6fc1f5f 
> 
> 
> Diff: https://reviews.apache.org/r/61262/diff/8/
> 
> 
> Testing
> ---
> 
> make check -j48
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61748: Style cleanup for grpc tests.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Aug. 18, 2017, 7 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61748/
> ---
> 
> (Updated Aug. 18, 2017, 7 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Style cleanup for grpc tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/grpc_tests.cpp 
> a11b6f02e59b83934a5fe9c62f68e7a361601a94 
> 
> 
> Diff: https://reviews.apache.org/r/61748/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61748: Style cleanup for grpc tests.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61600, 61748]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 18, 2017, 7 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61748/
> ---
> 
> (Updated Aug. 18, 2017, 7 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Style cleanup for grpc tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/grpc_tests.cpp 
> a11b6f02e59b83934a5fe9c62f68e7a361601a94 
> 
> 
> Diff: https://reviews.apache.org/r/61748/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

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



Patch looks great!

Reviews applied: [61262]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 18, 2017, 6:54 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61262/
> ---
> 
> (Updated Aug. 18, 2017, 6:54 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 fc5bd894ce55fe8e946d4c5b4b33d3c0505f3c2b 
>   include/mesos/v1/master/master.proto 
> c3fb31de2509adcdec8204f8bbe46b46a31540e8 
>   src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
>   src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
>   src/tests/api_tests.cpp 3ab4740bcac29ecb89585da6adb1f563d6fc1f5f 
> 
> 
> Diff: https://reviews.apache.org/r/61262/diff/8/
> 
> 
> Testing
> ---
> 
> make check -j48
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Review Request 61748: Style cleanup for grpc tests.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

Style cleanup for grpc tests.


Diffs
-

  3rdparty/libprocess/src/tests/grpc_tests.cpp 
a11b6f02e59b83934a5fe9c62f68e7a361601a94 


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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 61600: Rewrited rules for generating Protobuf and gRPC code.

[Chun-Hung Hsiao]

> On Aug. 16, 2017, 8:36 a.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/src/tests/grpc_tests.cpp
> > Line 95 (original), 95 (patched)
> > 
> >
> > Does it make sens to move this unrelated cleanup into a separate patch?
> > 
> > I also see that in this file generated GRPC-related headers are not 
> > sorted correctly, and invocations like `runtime.call(...)` are always 
> > wrapped though not needed.

Addressed in r/61748.


- Chun-Hung


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


On Aug. 18, 2017, 6:58 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61600/
> ---
> 
> (Updated Aug. 18, 2017, 6:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7889
> https://issues.apache.org/jira/browse/MESOS-7889
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made code generation for Proobuf and gRPC more generic and changed the
> variable names in `Makefile.am` to follow the same style in Mesos.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am c2fa3f9e81cb57bc9cb43f8f057014668bc49246 
> 
> 
> Diff: https://reviews.apache.org/r/61600/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61600: Rewrited rules for generating Protobuf and gRPC code.

[Chun-Hung Hsiao]

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

(Updated Aug. 18, 2017, 6:58 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Addressed @bbannier's comments.


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


Repository: mesos


Description
---

Made code generation for Proobuf and gRPC more generic and changed the
variable names in `Makefile.am` to follow the same style in Mesos.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am c2fa3f9e81cb57bc9cb43f8f057014668bc49246 


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

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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 61745: Fixed capitalization and typos in endpoint helps.

[Till Toenshoff]

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


Ship it!




Ship It!

- Till Toenshoff


On Aug. 18, 2017, 5:25 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61745/
> ---
> 
> (Updated Aug. 18, 2017, 5:25 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed capitalization and typos in endpoint helps.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
>   src/slave/http.cpp 544a052b8b2c3e70d2d109f3ec4fe6a4ca4a05ee 
> 
> 
> Diff: https://reviews.apache.org/r/61745/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

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

[Quinn Leng]

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

(Updated Aug. 18, 2017, 6:54 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 (updated)
-

  include/mesos/master/master.proto fc5bd894ce55fe8e946d4c5b4b33d3c0505f3c2b 
  include/mesos/v1/master/master.proto c3fb31de2509adcdec8204f8bbe46b46a31540e8 
  src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
  src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
  src/tests/api_tests.cpp 3ab4740bcac29ecb89585da6adb1f563d6fc1f5f 


Diff: https://reviews.apache.org/r/61262/diff/8/

Changes: https://reviews.apache.org/r/61262/diff/7-8/


Testing
---

make check -j48


Thanks,

Quinn Leng

Re: Review Request 61271: Implemented HTTP connection handling for the resource provider driver.

[Jie Yu]

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


Fix it, then Ship it!




Great job!!


src/resource_provider/http_connection.hpp
Lines 115 (patched)


We should not call out Resource Provider here if we want this to be re-used.


- Jie Yu


On Aug. 18, 2017, 10:18 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61271/
> ---
> 
> (Updated Aug. 18, 2017, 10:18 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7469
> https://issues.apache.org/jira/browse/MESOS-7469
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Similar to the existing HTTP connection handling of schedulers and
> executors, the resource provider driver will create two connections
> with the resource provider manager, one for streaming events and another
> one for sending calls. This connection handling has been generalized as
> a 'HttpConnectionProcess' and can be reused in other cases.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/resource_provider.hpp 
> 88b606212ea57fee1c1ea522d2dc7f8124a9adef 
>   src/CMakeLists.txt 98ccaf41bf5e7d14164e1c8b6e49268ac865a52c 
>   src/Makefile.am 68fff148f3d0c1710305bd9afcba62336d194b55 
>   src/resource_provider/detector.hpp PRE-CREATION 
>   src/resource_provider/detector.cpp PRE-CREATION 
>   src/resource_provider/driver.cpp 6778ec9c863022446f141ee88f70eb563178ea05 
>   src/resource_provider/http_connection.hpp PRE-CREATION 
>   src/resource_provider/storage/provider.cpp 
> 4c39312be5e4a6d783df3d385a66be6b3dcf8603 
> 
> 
> Diff: https://reviews.apache.org/r/61271/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

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

[Greg Mann]

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




src/master/master.hpp
Lines 309-312 (patched)


Could you add something about the `delay` parameter here?



src/master/master.hpp
Lines 321 (patched)


Let's give this a default value of `None()`.



src/master/master.hpp
Lines 2762 (patched)


If you give this parameter a default value, you could get rid of this.


- Greg Mann


On Aug. 18, 2017, 6:12 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61262/
> ---
> 
> (Updated Aug. 18, 2017, 6:12 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 fc5bd894ce55fe8e946d4c5b4b33d3c0505f3c2b 
>   include/mesos/v1/master/master.proto 
> c3fb31de2509adcdec8204f8bbe46b46a31540e8 
>   src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
>   src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
>   src/tests/api_tests.cpp 3ab4740bcac29ecb89585da6adb1f563d6fc1f5f 
> 
> 
> Diff: https://reviews.apache.org/r/61262/diff/7/
> 
> 
> Testing
> ---
> 
> make check -j48
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61189: Added authorization for V1 events.

[Quinn Leng]

> On Aug. 8, 2017, 9:36 a.m., Alexander Rojas wrote:
> > src/master/master.cpp
> > Lines 9547-9569 (patched)
> > 
> >
> > I don't think this part should be done as it is. Consider the case when 
> > you have an `Acceptor` which uses a security module which connects to LDAP 
> > for ACLs.
> > 
> > There is a delay for each request you make. It would be wasteful to 
> > create an Acceptor which you will later not use.

After discussion in the #ship-mesos-api-filter channel, we decide to keep the 
logic. Thus the authorizer is called for every events emitted by the stream.


> On Aug. 8, 2017, 9:36 a.m., Alexander Rojas wrote:
> > src/master/master.cpp
> > Lines 9547-9569 (patched)
> > 
> >
> > In fact, why not having one acceptor for each event, something like:
> > 
> > ```c++
> > // This code doesn't necesarily compile.
> > 
> > class EventTaskAddedAcceptor
> > {
> > public:
> >   static Owned create(Principal, Authorizer);
> >   
> >   bool accept(Task, FrameworkInfo);
> > 
> > private:
> >   EventTaskAddedAcceptor(
> > Owned authorizeTask,
> > Owned authorizeFramework);
> >  
> >   Owned authorizeTask_;
> >   Owned authorizeFramework_;
> > };
> > 
> > Owned EventTaskAddedAcceptor::create(
> > Principal principal, 
> > Authorizer authorizer)
> > {
> >   Future authorizeRole =
> > AuthorizationAcceptor::create(
> > subscriber->principal,
> > subscriber->master->authorizer,
> > authorization::VIEW_ROLE);
> > 
> >   Future authorizeFramework =
> > AuthorizationAcceptor::create(
> > subscriber->principal,
> > subscriber->authorizer,
> > authorization::VIEW_FRAMEWORK);
> > 
> >   return collect(authorizeRole, authorizeFramework)
> > .then([](tuple >Owned> acceptors) {
> >   return new EventTaskAddedAcceptor(acceptors[0], acceptors[1]);
> > });
> > }
> > 
> > EventTaskAddedAcceptor::EventTaskAddedAcceptor(
> > Owned authorizeTask,
> > Owned authorizeFramework)
> >   : authorizeTask_(authorizeTask),
> > authorizeFramework_(authorizeFramework)
> > {
> > }
> > 
> > bool EventTaskAddedAcceptor::accept(Task task, FrameworkInfo info)
> > {
> >   return authorizeTask->accept(task, info) &&
> >  authorizeFramework->accept(info);
> > }
> > ```
> > 
> > This way you only need one acceptor per event and you hid the details 
> > on how the authorization is made inside the acceptor API and remove that 
> > code from the caller.

Given our decision to call authorizer for every event, this combination doesn't 
bring in much benefit.


- Quinn


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


On Aug. 17, 2017, 10:59 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61189/
> ---
> 
> (Updated Aug. 17, 2017, 10:59 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, and Greg Mann.
> 
> 
> Bugs: MESOS-7785
> https://issues.apache.org/jira/browse/MESOS-7785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization filtering for the V1 operator events on the
> master, the subscriber should only receive events that are
> authorized based on their principal and ACLs. Since the authorizer
> is called for every event emitted on the stream, the change of
> authorization rule will affect events that the subscribers can
> receive.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
>   src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
>   src/tests/api_tests.cpp 3ab4740bcac29ecb89585da6adb1f563d6fc1f5f 
> 
> 
> Diff: https://reviews.apache.org/r/61189/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> GLOG_v=2 ./bin/mesos-tests.sh 
> --gtest_filter="ContentType/MasterAPITest.EventsAuthorizationFiltering*" 
> --verbose --gtest_repeat=100 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

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

[Quinn Leng]

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

(Updated Aug. 18, 2017, 6:12 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 (updated)
-

  include/mesos/master/master.proto fc5bd894ce55fe8e946d4c5b4b33d3c0505f3c2b 
  include/mesos/v1/master/master.proto c3fb31de2509adcdec8204f8bbe46b46a31540e8 
  src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
  src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
  src/tests/api_tests.cpp 3ab4740bcac29ecb89585da6adb1f563d6fc1f5f 


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

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


Testing
---

make check -j48


Thanks,

Quinn Leng

Re: Review Request 61725: Added secrets docs.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [61725]

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

Error:
error: patch failed: docs/home.md:35
error: docs/home.md: patch does not apply
error: docs/secrets.md: already exists in index

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

- Mesos Reviewbot Windows


On Aug. 18, 2017, 1:29 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61725/
> ---
> 
> (Updated Aug. 18, 2017, 1:29 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7418
> https://issues.apache.org/jira/browse/MESOS-7418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added secrets docs.
> 
> 
> Diffs
> -
> 
>   docs/home.md ab32838f621de76498262c9dd04e1cf01f8378ca 
>   docs/secrets.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61725/diff/4/
> 
> 
> Testing
> ---
> 
> ./mesos-website-dev.sh
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 61732: Updated endpoints help.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [61732, 61745, 61733, 61731]

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

Error:
error: patch failed: docs/configuration.md:1649
error: docs/configuration.md: patch does not apply

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

- Mesos Reviewbot Windows


On Aug. 18, 2017, 5:24 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61732/
> ---
> 
> (Updated Aug. 18, 2017, 5:24 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated endpoints help.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/index.md eb88154aaa46c340112acee149c716cf32a495bb 
>   docs/endpoints/master/frameworks.md 
> 1a7950a935c6ad7b23798e55f65f08d7db671135 
>   docs/endpoints/master/slaves.md f9bb7ef8dfa007a98fa64e00c7ad8fbfb32ea901 
>   docs/endpoints/master/tasks.json.md 
> f6b808d1ab2539f5754860f3b582d864482a6e8b 
>   docs/endpoints/master/tasks.md da04c9f48a612371a5ae167dafe2ac80bf11009f 
>   docs/endpoints/slave/api/v1/resource_provider.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61732/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 61731: Updated configuration.md to reflect new flags.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61731]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 18, 2017, 1:32 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61731/
> ---
> 
> (Updated Aug. 18, 2017, 1:32 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated configuration.md to reflect new flags.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md e43f9ea6f650e0beef6efd370a329006ca8875ea 
> 
> 
> Diff: https://reviews.apache.org/r/61731/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 61725: Added secrets docs.

[Kapil Arya]

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

(Updated Aug. 18, 2017, 1:29 p.m.)


Review request for mesos, Gilbert Song, Greg Mann, and Till Toenshoff.


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


Repository: mesos


Description
---

Added secrets docs.


Diffs
-

  docs/home.md ab32838f621de76498262c9dd04e1cf01f8378ca 
  docs/secrets.md PRE-CREATION 


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


Testing
---

./mesos-website-dev.sh


Thanks,

Kapil Arya

Re: Review Request 61733: Synchronized comment in v1 and unversioned master.proto.

[Kapil Arya]

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

(Updated Aug. 18, 2017, 1:28 p.m.)


Review request for mesos, Anand Mazumdar and Till Toenshoff.


Repository: mesos


Description
---

Synchronized comment in v1 and unversioned master.proto.


Diffs
-

  include/mesos/v1/master/master.proto c3fb31de2509adcdec8204f8bbe46b46a31540e8 


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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 61275: Added a URL parameter to the resource provider driver.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Aug. 2, 2017, 11:37 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61275/
> ---
> 
> (Updated Aug. 2, 2017, 11:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7469
> https://issues.apache.org/jira/browse/MESOS-7469
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The URL will be used by the resource provider driver to determine
> the endpoint of the resource provider manager API.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/resource_provider.hpp 
> 88b606212ea57fee1c1ea522d2dc7f8124a9adef 
>   src/resource_provider/daemon.hpp 467a5d30510f0c8f7f042d989c615c97bbae52af 
>   src/resource_provider/daemon.cpp adcb60af8244840c56fbd2d846bdaeecb4f62282 
>   src/resource_provider/driver.cpp 6778ec9c863022446f141ee88f70eb563178ea05 
>   src/resource_provider/local.hpp 604c5d06b393349c352d0b698609ad6bfb16454a 
>   src/resource_provider/local.cpp a57c7c99c674f18036c36ec4b6df71947f700db8 
>   src/resource_provider/storage/provider.hpp 
> c6ea440feeb4cd79c59aa1f25851513e5d8dcc86 
>   src/resource_provider/storage/provider.cpp 
> 4c39312be5e4a6d783df3d385a66be6b3dcf8603 
>   src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 
> 
> 
> Diff: https://reviews.apache.org/r/61275/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 61745: Fixed capitalization and typos in endpoint helps.

[Kapil Arya]

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

Review request for mesos and Till Toenshoff.


Repository: mesos


Description
---

Fixed capitalization and typos in endpoint helps.


Diffs
-

  src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
  src/slave/http.cpp 544a052b8b2c3e70d2d109f3ec4fe6a4ca4a05ee 


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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 61732: Updated endpoints help.

[Kapil Arya]

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

(Updated Aug. 18, 2017, 1:24 p.m.)


Review request for mesos, Anand Mazumdar and Till Toenshoff.


Repository: mesos


Description
---

Updated endpoints help.


Diffs (updated)
-

  docs/endpoints/index.md eb88154aaa46c340112acee149c716cf32a495bb 
  docs/endpoints/master/frameworks.md 1a7950a935c6ad7b23798e55f65f08d7db671135 
  docs/endpoints/master/slaves.md f9bb7ef8dfa007a98fa64e00c7ad8fbfb32ea901 
  docs/endpoints/master/tasks.json.md f6b808d1ab2539f5754860f3b582d864482a6e8b 
  docs/endpoints/master/tasks.md da04c9f48a612371a5ae167dafe2ac80bf11009f 
  docs/endpoints/slave/api/v1/resource_provider.md PRE-CREATION 


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

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 61733: Synchronized comment in v1 and unversioned master.proto.

[Kapil Arya]

> On Aug. 18, 2017, 12:47 p.m., Till Toenshoff wrote:
> > include/mesos/v1/master/master.proto
> > Line 362 (original), 362 (patched)
> > 
> >
> > Is there any documentation around our deprecation cycles?
> > I kind of understand that these pubic promises should be handled with 
> > very long deprecation but why a 2.0 and not a 1.2 + N (e.g. 3). We need to 
> > be clear and have this publically documented I believe.
> > 
> > If there was no JIRA around it, we should create one.

Dropping the issues since the RR just syncs the two files. We should file a 
separate JIRA to fix those.


- Kapil


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


On Aug. 18, 2017, 9:32 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61733/
> ---
> 
> (Updated Aug. 18, 2017, 9:32 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Synchronized comment in v1 and unversioned master.proto.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/master/master.proto 
> c3fb31de2509adcdec8204f8bbe46b46a31540e8 
> 
> 
> Diff: https://reviews.apache.org/r/61733/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 61732: Updated endpoints help.

[Till Toenshoff]

> On Aug. 18, 2017, 4:55 p.m., Till Toenshoff wrote:
> > docs/endpoints/slave/api/v1/resource_provider.md
> > Lines 12 (patched)
> > 
> >
> > Not yours but the capitalizing here is inconsitent - we should possibly 
> > fix the source. 
> > 
> > "Local resource Provider" vs. "local resource provider" later in this 
> > document. Also why are "Call" and "Event" capitalized -- they are neither 
> > product nor company names.
> > 
> > @anandmazumdar What do you think?

Actually "Call" and "Event" make sense as they are naming specific messages.


- Till


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


On Aug. 18, 2017, 1:48 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61732/
> ---
> 
> (Updated Aug. 18, 2017, 1:48 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated endpoints help.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/index.md eb88154aaa46c340112acee149c716cf32a495bb 
>   docs/endpoints/master/frameworks.md 
> 1a7950a935c6ad7b23798e55f65f08d7db671135 
>   docs/endpoints/master/slaves.md f9bb7ef8dfa007a98fa64e00c7ad8fbfb32ea901 
>   docs/endpoints/master/tasks.json.md 
> f6b808d1ab2539f5754860f3b582d864482a6e8b 
>   docs/endpoints/master/tasks.md da04c9f48a612371a5ae167dafe2ac80bf11009f 
>   docs/endpoints/slave/api/v1/resource_provider.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61732/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 61732: Updated endpoints help.

[Till Toenshoff]

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


Fix it, then Ship it!





docs/endpoints/master/frameworks.md
Lines 24 (patched)


s/framework ID specified/framework ID is specified/



docs/endpoints/slave/api/v1/resource_provider.md
Lines 12 (patched)


Not yours but the capitalizing here is inconsitent - we should possibly fix 
the source. 

"Local resource Provider" vs. "local resource provider" later in this 
document. Also why are "Call" and "Event" capitalized -- they are neither 
product nor company names.

@anandmazumdar What do you think?


- Till Toenshoff


On Aug. 18, 2017, 1:48 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61732/
> ---
> 
> (Updated Aug. 18, 2017, 1:48 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated endpoints help.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/index.md eb88154aaa46c340112acee149c716cf32a495bb 
>   docs/endpoints/master/frameworks.md 
> 1a7950a935c6ad7b23798e55f65f08d7db671135 
>   docs/endpoints/master/slaves.md f9bb7ef8dfa007a98fa64e00c7ad8fbfb32ea901 
>   docs/endpoints/master/tasks.json.md 
> f6b808d1ab2539f5754860f3b582d864482a6e8b 
>   docs/endpoints/master/tasks.md da04c9f48a612371a5ae167dafe2ac80bf11009f 
>   docs/endpoints/slave/api/v1/resource_provider.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61732/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 61733: Synchronized comment in v1 and unversioned master.proto.

[Till Toenshoff]

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




include/mesos/v1/master/master.proto
Line 362 (original), 362 (patched)


Is there any documentation around our deprecation cycles?
I kind of understand that these pubic promises should be handled with very 
long deprecation but why a 2.0 and not a 1.2 + N (e.g. 3). We need to be clear 
and have this publically documented I believe.

If there was no JIRA around it, we should create one.



include/mesos/v1/master/master.proto
Line 377 (original), 377 (patched)


Why did the start of deprecation period version change here?



include/mesos/v1/master/master.proto
Line 410 (original), 409 (patched)


See above - kinda confusing. Guessing that you have simply made it 
consistant with unversioned but we should double-check with the maintainers 
here, I feel.


- Till Toenshoff


On Aug. 18, 2017, 1:32 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61733/
> ---
> 
> (Updated Aug. 18, 2017, 1:32 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Synchronized comment in v1 and unversioned master.proto.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/master/master.proto 
> c3fb31de2509adcdec8204f8bbe46b46a31540e8 
> 
> 
> Diff: https://reviews.apache.org/r/61733/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 61742: Added a master-registry back resource provider manager registry.

[Mesos Reviewbot Windows]

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



Bad review!

Reviews applied: []

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

- Mesos Reviewbot Windows


On Aug. 18, 2017, 3:46 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61742/
> ---
> 
> (Updated Aug. 18, 2017, 3:46 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds an implementation of the resource provider registrar
> back by the master's registrar. With that it becomes possible to
> persist resource provider manager state in a single master registrar,
> but use the narrowly defined resource provider registry.
> 
> 
> Diffs
> -
> 
>   src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
>   src/resource_provider/registrar.hpp PRE-CREATION 
>   src/resource_provider/registrar.cpp PRE-CREATION 
>   src/tests/resource_provider_manager_tests.cpp 
> 85906ea5e1bb3516ef264de22913ce0a3c9c58c5 
> 
> 
> Diff: https://reviews.apache.org/r/61742/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 61725: Added secrets docs.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61731, 61732, 61733, 61725]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 18, 2017, 11:46 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61725/
> ---
> 
> (Updated Aug. 18, 2017, 11:46 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7418
> https://issues.apache.org/jira/browse/MESOS-7418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added secrets docs.
> 
> 
> Diffs
> -
> 
>   docs/home.md ab32838f621de76498262c9dd04e1cf01f8378ca 
>   docs/secrets.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61725/diff/4/
> 
> 
> Testing
> ---
> 
> ./mesos-website-dev.sh
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Review Request 61742: Added a master-registry back resource provider manager registry.

[Benjamin Bannier]

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

Review request for mesos.


Repository: mesos


Description
---

This patch adds an implementation of the resource provider registrar
back by the master's registrar. With that it becomes possible to
persist resource provider manager state in a single master registrar,
but use the narrowly defined resource provider registry.


Diffs
-

  src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
  src/resource_provider/registrar.hpp PRE-CREATION 
  src/resource_provider/registrar.cpp PRE-CREATION 
  src/tests/resource_provider_manager_tests.cpp 
85906ea5e1bb3516ef264de22913ce0a3c9c58c5 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 61740: Added a no-op master registry operation.

[Benjamin Bannier]

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

Review request for mesos.


Repository: mesos


Description
---

Added a no-op master registry operation.


Diffs
-

  src/master/registrar.hpp c439f6a11274c081059d666f07ab0ddc47208a7d 
  src/master/registrar.cpp 488b0896b8bfbb02583d614376053bde35156dee 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

[Benjamin Bannier]

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

(Updated Aug. 18, 2017, 5:46 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Followed Jie's implementation suggestion.


Summary (updated)
-

Implemented a registrar for resource provider manager state.


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


Repository: mesos


Description (updated)
---

This patch adds a registry and registrar interface for resource
provider managers. The registrar interface is modelled after the
master registrar and supports similar operations. Currently a single,
LevelDB-backed registrar is implemented which we plan to use for
resource provider managers in agents.

Current the registry allows to add and remove resource provider IDs.


Diffs (updated)
-

  src/CMakeLists.txt 98ccaf41bf5e7d14164e1c8b6e49268ac865a52c 
  src/Makefile.am 68fff148f3d0c1710305bd9afcba62336d194b55 
  src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
  src/resource_provider/registrar.hpp PRE-CREATION 
  src/resource_provider/registrar.cpp PRE-CREATION 
  src/resource_provider/registry.hpp PRE-CREATION 
  src/resource_provider/registry.proto PRE-CREATION 
  src/tests/resource_provider_manager_tests.cpp 
85906ea5e1bb3516ef264de22913ce0a3c9c58c5 


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

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


Testing
---


Thanks,

Benjamin Bannier

Re: Review Request 61725: Added secrets docs.

[Kapil Arya]

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

(Updated Aug. 18, 2017, 11:46 a.m.)


Review request for mesos, Gilbert Song, Greg Mann, and Till Toenshoff.


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


Repository: mesos


Description
---

Added secrets docs.


Diffs
-

  docs/home.md ab32838f621de76498262c9dd04e1cf01f8378ca 
  docs/secrets.md PRE-CREATION 


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


Testing (updated)
---

./mesos-website-dev.sh


Thanks,

Kapil Arya

Re: Review Request 61725: Added secrets docs.

[Kapil Arya]

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

(Updated Aug. 18, 2017, 11:45 a.m.)


Review request for mesos, Gilbert Song, Greg Mann, and Till Toenshoff.


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


Repository: mesos


Description
---

Added secrets docs.


Diffs (updated)
-

  docs/home.md ab32838f621de76498262c9dd04e1cf01f8378ca 
  docs/secrets.md PRE-CREATION 


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

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 61735: Removed diacritics from docs/health-checks.md.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61735]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 18, 2017, 2:41 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61735/
> ---
> 
> (Updated Aug. 18, 2017, 2:41 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gastón Kleiman, and Kapil Arya.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md 41fc4f8f224115d9a1d634aa0a775a9d01b4e938 
> 
> 
> Diff: https://reviews.apache.org/r/61735/diff/1/
> 
> 
> Testing
> ---
> 
> site/mesos-website-dev.sh
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 61149: Added Future::condition.

[Alexander Rukletsov]

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



Looks like the summary became incorrect after the last diff update.

- Alexander Rukletsov


On July 27, 2017, 1:55 a.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61149/
> ---
> 
> (Updated July 27, 2017, 1:55 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Future::condition.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> cce950509f58022e79bb51a6e72ea1a005b9cb50 
>   3rdparty/libprocess/src/tests/future_tests.cpp 
> 0c8725b9a5e64aaac6e3979e450a11e84f9bd45e 
> 
> 
> Diff: https://reviews.apache.org/r/61149/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 61725: Added secrets docs.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61731, 61732, 61733, 61725]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 18, 2017, 2:18 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61725/
> ---
> 
> (Updated Aug. 18, 2017, 2:18 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7418
> https://issues.apache.org/jira/browse/MESOS-7418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added secrets docs.
> 
> 
> Diffs
> -
> 
>   docs/home.md ab32838f621de76498262c9dd04e1cf01f8378ca 
>   docs/secrets.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61725/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 61735: Removed diacritics from docs/health-checks.md.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On Aug. 18, 2017, 10:41 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61735/
> ---
> 
> (Updated Aug. 18, 2017, 10:41 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gastón Kleiman, and Kapil Arya.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md 41fc4f8f224115d9a1d634aa0a775a9d01b4e938 
> 
> 
> Diff: https://reviews.apache.org/r/61735/diff/1/
> 
> 
> Testing
> ---
> 
> site/mesos-website-dev.sh
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Review Request 61735: Removed diacritics from docs/health-checks.md.

[Till Toenshoff]

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

Review request for mesos, Alexander Rukletsov, Gastón Kleiman, and Kapil Arya.


Repository: mesos


Description
---

see summary.


Diffs
-

  docs/health-checks.md 41fc4f8f224115d9a1d634aa0a775a9d01b4e938 


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


Testing
---

site/mesos-website-dev.sh


Thanks,

Till Toenshoff

Re: Review Request 61725: Added secrets docs.

[Kapil Arya]

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

(Updated Aug. 18, 2017, 10:18 a.m.)


Review request for mesos, Gilbert Song, Greg Mann, and Till Toenshoff.


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


Repository: mesos


Description
---

Added secrets docs.


Diffs (updated)
-

  docs/home.md ab32838f621de76498262c9dd04e1cf01f8378ca 
  docs/secrets.md PRE-CREATION 


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

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 61733: Synchronized comment in v1 and unversioned master.proto.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61731, 61732, 61733]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 18, 2017, 1:32 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61733/
> ---
> 
> (Updated Aug. 18, 2017, 1:32 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Synchronized comment in v1 and unversioned master.proto.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/master/master.proto 
> c3fb31de2509adcdec8204f8bbe46b46a31540e8 
> 
> 
> Diff: https://reviews.apache.org/r/61733/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 61725: Added secrets docs.

[Kapil Arya]

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

(Updated Aug. 18, 2017, 10 a.m.)


Review request for mesos, Gilbert Song, Greg Mann, and Till Toenshoff.


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


Repository: mesos


Description
---

Added secrets docs.


Diffs (updated)
-

  docs/secrets.md PRE-CREATION 


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

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 61732: Updated endpoints help.

[Kapil Arya]

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

(Updated Aug. 18, 2017, 9:48 a.m.)


Review request for mesos, Anand Mazumdar and Till Toenshoff.


Repository: mesos


Description
---

Updated endpoints help.


Diffs (updated)
-

  docs/endpoints/index.md eb88154aaa46c340112acee149c716cf32a495bb 
  docs/endpoints/master/frameworks.md 1a7950a935c6ad7b23798e55f65f08d7db671135 
  docs/endpoints/master/slaves.md f9bb7ef8dfa007a98fa64e00c7ad8fbfb32ea901 
  docs/endpoints/master/tasks.json.md f6b808d1ab2539f5754860f3b582d864482a6e8b 
  docs/endpoints/master/tasks.md da04c9f48a612371a5ae167dafe2ac80bf11009f 
  docs/endpoints/slave/api/v1/resource_provider.md PRE-CREATION 


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

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 61725: Added secrets docs.

[Till Toenshoff]

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


Fix it, then Ship it!




Great to see this being documented - thanks so much Kapil!

We may want to be a bit more explicit when it comes to tha lack of Secrets 
within the docker containerizer. Couple of possibly entirely incorrect wording 
suggestions - sorry if they are BS as I am not a native speaker.


docs/secrets.md
Lines 7 (patched)


s/now// ?



docs/secrets.md
Lines 9 (patched)


s/one to specify/specifying ?



docs/secrets.md
Lines 11 (patched)


Isn't the UCR just a name for the Mesos containerizer using (docker) base 
images - I am not sure this won't confuse people. Also note that our docs do 
not mention "UCR" at all.

Maybe be explicit here; "... and not the docker containerizer"?



docs/secrets.md
Lines 14 (patched)


Maybe:
"Secrets can be specified using the following protobuf messages." ?



docs/secrets.md
Lines 41 (patched)


You are introducing "modules" here which may be a bit confusing. How about 
mentioning modules in the high level description already? Something that tells 
the user that a secret resolver would be commonly implemented using modules?



docs/secrets.md
Lines 111 (patched)


Maybe we should be explicit again and tell users that we do NOT mean docker 
containerizer here but docker containers handled by the mesos containerizer? By 
saying only docker containers, which ones are we not supporting - can we name 
them here?



docs/secrets.md
Lines 142 (patched)


s/A default/The default/?
s/resolved/resolvs/?


- Till Toenshoff


On Aug. 18, 2017, 1:30 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61725/
> ---
> 
> (Updated Aug. 18, 2017, 1:30 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7418
> https://issues.apache.org/jira/browse/MESOS-7418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added secrets docs.
> 
> 
> Diffs
> -
> 
>   docs/secrets.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61725/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Review Request 61733: Synchronized comment in v1 and unversioned master.proto.

[Kapil Arya]

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

Review request for mesos, Anand Mazumdar and Till Toenshoff.


Repository: mesos


Description
---

Synchronized comment in v1 and unversioned master.proto.


Diffs
-

  include/mesos/v1/master/master.proto c3fb31de2509adcdec8204f8bbe46b46a31540e8 


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


Testing
---


Thanks,

Kapil Arya

Review Request 61732: Updated endpoints help.

[Kapil Arya]

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

Review request for mesos, Anand Mazumdar and Till Toenshoff.


Repository: mesos


Description
---

Updated endpoints help.


Diffs
-

  docs/endpoints/index.md eb88154aaa46c340112acee149c716cf32a495bb 
  docs/endpoints/master/tasks.json.md f6b808d1ab2539f5754860f3b582d864482a6e8b 
  docs/endpoints/master/tasks.md da04c9f48a612371a5ae167dafe2ac80bf11009f 
  docs/endpoints/slave/api/v1/resource_provider.md PRE-CREATION 


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


Testing
---


Thanks,

Kapil Arya

Review Request 61731: Updated configuration.md to reflect new flags.

[Kapil Arya]

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

Review request for mesos, Anand Mazumdar and Till Toenshoff.


Repository: mesos


Description
---

Updated configuration.md to reflect new flags.


Diffs
-

  docs/configuration.md e43f9ea6f650e0beef6efd370a329006ca8875ea 


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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 61272: Added a MockResourceProvider.

[Jan Schlicht]

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

(Updated Aug. 18, 2017, 12:24 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added a MockResourceProvider.


Diffs (updated)
-

  src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45 


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

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 61271: Implemented HTTP connection handling for the resource provider driver.

[Jan Schlicht]

> On Aug. 3, 2017, noon, Benjamin Bannier wrote:
> > src/resource_provider/http_connection.hpp
> > Lines 90 (patched)
> > 
> >
> > I wonder if it makes more sense to return e.g., a `Try` here in 
> > order to surface the failure scenarios below, many of which might be caused 
> > by the caller. This could give them a chance to retry if we map `true` to 
> > success and `false` to transient errors.
> > 
> > Otherwise let's add a `TODO`.

It's returning a `Future` now.


- Jan


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


On Aug. 18, 2017, 12:18 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61271/
> ---
> 
> (Updated Aug. 18, 2017, 12:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7469
> https://issues.apache.org/jira/browse/MESOS-7469
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Similar to the existing HTTP connection handling of schedulers and
> executors, the resource provider driver will create two connections
> with the resource provider manager, one for streaming events and another
> one for sending calls. This connection handling has been generalized as
> a 'HttpConnectionProcess' and can be reused in other cases.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/resource_provider.hpp 
> 88b606212ea57fee1c1ea522d2dc7f8124a9adef 
>   src/CMakeLists.txt 98ccaf41bf5e7d14164e1c8b6e49268ac865a52c 
>   src/Makefile.am 68fff148f3d0c1710305bd9afcba62336d194b55 
>   src/resource_provider/detector.hpp PRE-CREATION 
>   src/resource_provider/detector.cpp PRE-CREATION 
>   src/resource_provider/driver.cpp 6778ec9c863022446f141ee88f70eb563178ea05 
>   src/resource_provider/http_connection.hpp PRE-CREATION 
>   src/resource_provider/storage/provider.cpp 
> 4c39312be5e4a6d783df3d385a66be6b3dcf8603 
> 
> 
> Diff: https://reviews.apache.org/r/61271/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 61271: Implemented HTTP connection handling for the resource provider driver.

[Jan Schlicht]

> On Aug. 17, 2017, 6:11 a.m., Jie Yu wrote:
> > src/resource_provider/http_connection.hpp
> > Lines 321 (patched)
> > 
> >
> > This should be `UNREACHABLE` as well?
> 
> Jan Schlicht wrote:
> No, the instance may get destructed before it's connected. In that case 
> this switch will be called while in `State::CONNECTING`.
> 
> Jie Yu wrote:
> ah, when connect attempt fails. this will be called. Can you add a 
> command here?

The switch was moved to `detected` to fix the issue below. This slightly 
changes where it can be called, there isn't any state that we would consider 
`UNREACHABLE` in that case, because `detected` could be called in any state of 
the instance.


> On Aug. 17, 2017, 6:11 a.m., Jie Yu wrote:
> > src/resource_provider/http_connection.hpp
> > Lines 407 (patched)
> > 
> >
> > scheduler?
> > 
> > Also, who is going to initiate the retry? How does the resource 
> > provider using this driver notice a failed subscribe call?
> > 
> > Is it based on timeout? That sounds super wierd given that we know it 
> > is an error.
> > 
> > I am wondering if we should make the `send` method returns a 
> > `Future` or `Future`, rather than just printing a 
> > warning in the log. Based on the failure the user receives, it might choose 
> > to retry subscription, or do something else.

Changed the driver to return a `Future`.


- Jan


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


On Aug. 18, 2017, 12:18 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61271/
> ---
> 
> (Updated Aug. 18, 2017, 12:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7469
> https://issues.apache.org/jira/browse/MESOS-7469
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Similar to the existing HTTP connection handling of schedulers and
> executors, the resource provider driver will create two connections
> with the resource provider manager, one for streaming events and another
> one for sending calls. This connection handling has been generalized as
> a 'HttpConnectionProcess' and can be reused in other cases.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/resource_provider.hpp 
> 88b606212ea57fee1c1ea522d2dc7f8124a9adef 
>   src/CMakeLists.txt 98ccaf41bf5e7d14164e1c8b6e49268ac865a52c 
>   src/Makefile.am 68fff148f3d0c1710305bd9afcba62336d194b55 
>   src/resource_provider/detector.hpp PRE-CREATION 
>   src/resource_provider/detector.cpp PRE-CREATION 
>   src/resource_provider/driver.cpp 6778ec9c863022446f141ee88f70eb563178ea05 
>   src/resource_provider/http_connection.hpp PRE-CREATION 
>   src/resource_provider/storage/provider.cpp 
> 4c39312be5e4a6d783df3d385a66be6b3dcf8603 
> 
> 
> Diff: https://reviews.apache.org/r/61271/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 61271: Implemented HTTP connection handling for the resource provider driver.

[Jan Schlicht]

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

(Updated Aug. 18, 2017, 12:18 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Addressed issues (`Driver::send` now returns a `Future`)


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


Repository: mesos


Description
---

Similar to the existing HTTP connection handling of schedulers and
executors, the resource provider driver will create two connections
with the resource provider manager, one for streaming events and another
one for sending calls. This connection handling has been generalized as
a 'HttpConnectionProcess' and can be reused in other cases.


Diffs (updated)
-

  include/mesos/v1/resource_provider.hpp 
88b606212ea57fee1c1ea522d2dc7f8124a9adef 
  src/CMakeLists.txt 98ccaf41bf5e7d14164e1c8b6e49268ac865a52c 
  src/Makefile.am 68fff148f3d0c1710305bd9afcba62336d194b55 
  src/resource_provider/detector.hpp PRE-CREATION 
  src/resource_provider/detector.cpp PRE-CREATION 
  src/resource_provider/driver.cpp 6778ec9c863022446f141ee88f70eb563178ea05 
  src/resource_provider/http_connection.hpp PRE-CREATION 
  src/resource_provider/storage/provider.cpp 
4c39312be5e4a6d783df3d385a66be6b3dcf8603 


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

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 61544: WIP: Rewrote Mesos CMake build.

[Andrew Schwartzmeyer]

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

(Updated Aug. 18, 2017, 9:30 a.m.)


Review request for mesos, Aaron Wood, Benjamin Bannier, Chun-Hung Hsiao, Jeff 
Coffler, Jie Yu, James Peach, and Joseph Wu.


Repository: mesos


Description
---

WIP: Rewrote Mesos CMake build.


Diffs
-

  cmake/CompilationConfigure.cmake 2109697e02e58c673948f2ae67ef8255d16aa736 
  cmake/MesosConfigure.cmake 5ecad2c0fbd245dd14e9c09869dd6da2ca4e602a 
  src/CMakeLists.txt 98ccaf41bf5e7d14164e1c8b6e49268ac865a52c 
  src/checks/CMakeLists.txt 4854b2765202e10f5b791f08ed1416f7740e2aad 
  src/cli/CMakeLists.txt 056d1d734e4424dac3eb29c323961d3a2e87a8dc 
  src/docker/CMakeLists.txt 2d7c30db5854853625a99227568a27dde281ec2e 
  src/examples/CMakeLists.txt e8b7ea4bd91c20536ce7bd8d0ffdfd4e549b056f 
  src/launcher/CMakeLists.txt 97edc4a28a995da8a5e963aaa62c4c955cc59719 
  src/local/CMakeLists.txt 7c9a718d17772be1c08e132136739cfd9449470d 
  src/log/CMakeLists.txt 2fad6cca467dbde9989d26424a2a6c608cd251e8 
  src/master/CMakeLists.txt 0062fb3e9d67fdfcda1057112d4ba34abbd4be5f 
  src/master/cmake/MasterConfigure.cmake 
173dc36b5a1b7ff23cd0542ea4313ae6e5e0ae3f 
  src/slave/CMakeLists.txt 9dbadca53b9ac34db08cfb9c9e9fea741c01114b 
  src/slave/cmake/AgentConfigure.cmake af2f74f82b68920427f36c0c863b97c0191b8f06 
  src/slave/container_loggers/CMakeLists.txt 
c0774ce3110fd8d2aac6dc00d2b800e336214f96 
  src/slave/containerizer/mesos/CMakeLists.txt 
fa2043efadd1c863c5ad54fdc518397c97488dcb 
  src/slave/qos_controllers/CMakeLists.txt 
65ab338a71276a77e43af962fbc9b76e050efca6 
  src/slave/resource_estimators/CMakeLists.txt 
1d28bd457c7e567219d892f0940903b12c5a877b 
  src/tests/CMakeLists.txt 6dd2716de942adf6cefa5a464ef664f3c3ebb7a3 
  src/tests/cmake/MesosTestsConfigure.cmake 
48b4d2fd74b545c03162c7cb06013baf67298241 
  src/usage/CMakeLists.txt fc87b297832157b679039c68baf9ddc68a292060 


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


Testing
---

Built and ran mesos-tests on CentOS 7 and Windows 10. THIS IS WORK IN PROGRESS 
FOR REVIEW.


Thanks,

Andrew Schwartzmeyer

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

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


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 61262: Added 'heartbeat' event for the operator API.

[Greg Mann]

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




src/master/master.hpp
Lines 309 (patched)


Could you also explain the template parameters here? i.e. the first 
parameter represents the type of the input message, the second is the type of 
the event we send on the connection.

Also please explain the purpose of the `delay` parameter to the constructor.



src/master/master.hpp
Lines 310 (patched)


I think you can get rid of the default template parameter ` = 
v1::scheduler::Event` here, since you specify it explicitly in both cases.



src/master/master.hpp
Lines 329-333 (patched)


I think making `delay` an `Option` is more appropriate here. Then 
this check could be `if (delay.isSome())`


- Greg Mann


On Aug. 17, 2017, 9:20 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61262/
> ---
> 
> (Updated Aug. 17, 2017, 9:20 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 fc5bd894ce55fe8e946d4c5b4b33d3c0505f3c2b 
>   include/mesos/v1/master/master.proto 
> c3fb31de2509adcdec8204f8bbe46b46a31540e8 
>   src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
>   src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
>   src/tests/api_tests.cpp 3ab4740bcac29ecb89585da6adb1f563d6fc1f5f 
> 
> 
> Diff: https://reviews.apache.org/r/61262/diff/6/
> 
> 
> Testing
> ---
> 
> make check -j48
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>