Re: Review Request 69082: Correctly propagated `close` failures in some instances.

> On Jan. 15, 2019, 7:22 p.m., Benjamin Mahler wrote: > > Should we be surfacing a close EINTR as an error or let that be silent? > > I think these errors need some message pre-fixing? E.g. > > > > ``` > > Failed to close '3': Bad file number > > ``` > > > > As it stands the error messages

Re: Review Request 69493: Documented the `linux/seccomp` isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69493/#review212067 --- FAIL: Failed to apply the dependent review: 67844. Failed

Re: Review Request 69612: Refactored parallel test runner.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69612/#review212068 --- Fix it, then Ship it! Thanks for the cleanup, Andrei!

Re: Review Request 69612: Refactored parallel test runner.

> On Jan. 16, 2019, 2:02 p.m., Benjamin Bannier wrote: > > support/mesos-gtest-runner.py > > Line 246 (original) > > > > > > Any reason to not preserve the path normalization here? `EXECUTABLE` class variable is

Re: Review Request 69082: Correctly propagated `close` failures in some instances.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69082/ --- (Updated Jan. 16, 2019, 2:06 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 68017: Added Seccomp-related protobuf messages.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68017/ --- (Updated Jan. 16, 2019, 1:41 p.m.) Review request for mesos, Gilbert Song, Jie

Re: Review Request 69612: Refactored parallel test runner.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69612/#review212069 --- support/mesos-gtest-runner.py Line 246 (original)

Re: Review Request 69719: Exposed subscriptions and disconnections RP manager metrics.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69719/ --- (Updated Jan. 16, 2019, 12:49 p.m.) Review request for mesos, Chun-Hung Hsiao

Re: Review Request 69158: Added an integration test for resource provider removal.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69158/#review212062 --- FAIL: Some of the unit tests failed. Please check the relevant

Re: Review Request 69082: Correctly propagated `close` failures in some instances.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69082/#review212065 --- FAIL: Some of the unit tests failed. Please check the relevant

Re: Review Request 68018: Added `SeccompFilter` class.

> On Jan. 14, 2019, 8:31 a.m., Qian Zhang wrote: > > src/linux/seccomp/seccomp.cpp > > Lines 137-139 (patched) > > > > > > Will this affect the task run by Mesos? E.g., a task may want to run a > > program which

Re: Review Request 69719: Exposed subscriptions and disconnections RP manager metrics.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69719/#review212064 --- FAIL: Some of the unit tests failed. Please check the relevant

Re: Review Request 69719: Exposed subscriptions and disconnections RP manager metrics.

> On Jan. 15, 2019, 12:55 a.m., Chun-Hung Hsiao wrote: > > src/resource_provider/manager.cpp > > Lines 823-832 (original), 827-836 (patched) > > > > > > How about moving this snippt to the end of this function, so

[GitHub] kaysoky commented on issue #324: MESOS-9499 extended URI syntax to support any Zookeeper authentication schemes

kaysoky commented on issue #324: MESOS-9499 extended URI syntax to support any Zookeeper authentication schemes URL: https://github.com/apache/mesos/pull/324#issuecomment-454875262 Yes, a unit test for this URL parsing would be good, if that is the approach we end up with. In terms

[GitHub] dlazarus commented on issue #324: MESOS-9499 extended URI syntax to support any Zookeeper authentication schemes

dlazarus commented on issue #324: MESOS-9499 extended URI syntax to support any Zookeeper authentication schemes URL: https://github.com/apache/mesos/pull/324#issuecomment-454839100 > We will want to add some regression tests too, before we commit any changes. > > You can start

[GitHub] dlazarus commented on a change in pull request #324: MESOS-9499 extended URI syntax to support any Zookeeper authentication schemes

dlazarus commented on a change in pull request #324: MESOS-9499 extended URI syntax to support any Zookeeper authentication schemes URL: https://github.com/apache/mesos/pull/324#discussion_r248392832 ## File path: include/mesos/zookeeper/url.hpp ## @@ -101,16 +102,35 @@

[GitHub] dlazarus commented on a change in pull request #324: MESOS-9499 extended URI syntax to support any Zookeeper authentication schemes

dlazarus commented on a change in pull request #324: MESOS-9499 extended URI syntax to support any Zookeeper authentication schemes URL: https://github.com/apache/mesos/pull/324#discussion_r248392832 ## File path: include/mesos/zookeeper/url.hpp ## @@ -101,16 +102,35 @@

Re: Review Request 69723: Enabled operation feedback on agent default resources.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69723/#review212076 --- FAIL: Some of the unit tests failed. Please check the relevant

[GitHub] dlazarus commented on a change in pull request #324: MESOS-9499 extended URI syntax to support any Zookeeper authentication schemes

dlazarus commented on a change in pull request #324: MESOS-9499 extended URI syntax to support any Zookeeper authentication schemes URL: https://github.com/apache/mesos/pull/324#discussion_r248393365 ## File path: include/mesos/zookeeper/url.hpp ## @@ -101,16 +102,35 @@

Re: Review Request 69612: Refactored parallel test runner.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69612/#review212072 --- FAIL: Some of the unit tests failed. Please check the relevant

[GitHub] kaysoky commented on a change in pull request #324: MESOS-9499 extended URI syntax to support any Zookeeper authentication schemes

kaysoky commented on a change in pull request #324: MESOS-9499 extended URI syntax to support any Zookeeper authentication schemes URL: https://github.com/apache/mesos/pull/324#discussion_r248384958 ## File path: include/mesos/zookeeper/url.hpp ## @@ -101,16 +102,35 @@

Re: Review Request 69723: Enabled operation feedback on agent default resources.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69723/ --- (Updated Jan. 16, 2019, 3:54 p.m.) Review request for mesos, Gastón Kleiman,

Re: Review Request 69697: Reverted cleanup step of `verify-reviews.py`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69697/#review212077 --- support/verify-reviews.py Line 174 (original), 171 (patched)

Review Request 69775: Updated master fail() logging from FATAL to ERROR.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69775/ --- Review request for mesos, Andrei Budnik, Benjamin Mahler, Greg Mann, and Qian

Re: Review Request 69776: Fixed flakiness by adding per agent config dir for mesos test.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69776/#review212083 --- Fix it, then Ship it! src/tests/mesos.cpp Line 173

Re: Review Request 69755: Moved flags and constants in MesosContainerLaunch into header.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69755/#review212079 --- Could you please link a ticket to this? This is a weird way to

Re: Review Request 69775: Updated master fail() logging from FATAL to ERROR.

> On Jan. 16, 2019, 9:39 p.m., Vinod Kone wrote: > > src/master/master.cpp > > Line 1627 (original), 1627 (patched) > > > > > > This exited the process before, now we no longer do? Is that safe? We could do: ```

Re: Review Request 68017: Added Seccomp-related protobuf messages.

> On Jan. 15, 2019, 2:39 a.m., Gilbert Song wrote: > > include/mesos/mesos.proto > > Lines 3158-3159 (patched) > > > > > > Seems like this was added recently. > > > > Is this field only used when there is

Re: Review Request 69775: Updated master fail() logging from FATAL to ERROR.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69775/#review212082 --- src/master/master.cpp Line 1627 (original), 1627 (patched)

Re: Review Request 69776: Fixed flakiness by adding per agent config dir for mesos test.

> On Jan. 16, 2019, 2:10 p.m., Jie Yu wrote: > > src/tests/mesos.cpp > > Line 173 (original), 173 (patched) > > > > > > s/agentDir/configDir/ I thought about `configDir`, sounds not appropriate to me, since store

Review Request 69776: Fixed flakiness by adding per agent config dir for mesos test.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69776/ --- Review request for mesos, Andrei Budnik, Greg Mann, Jie Yu, and Qian Zhang.

Re: Review Request 69775: Updated master fail() logging from FATAL to ERROR.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69775/#review212087 --- PASS: Mesos patch 69775 was successfully built and tested.

Re: Review Request 68017: Added Seccomp-related protobuf messages.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68017/#review212088 --- Ship it! Ship It! - Gilbert Song On Jan. 16, 2019, 5:41

Re: Review Request 69612: Refactored parallel test runner.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69612/#review212080 --- FAIL: Some of the unit tests failed. Please check the relevant

Re: Review Request 69775: Updated master fail() logging from FATAL to ERROR.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69775/ --- (Updated Jan. 16, 2019, 3:07 p.m.) Review request for mesos, Andrei Budnik,

Re: Review Request 69719: Exposed subscriptions and disconnections RP manager metrics.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69719/#review212099 --- Ship it! Ship It! - Chun-Hung Hsiao On Jan. 16, 2019, 11:49

Re: Review Request 69776: Fixed flakiness by adding per agent config dir for mesos test.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69776/#review212091 --- FAIL: Some of the unit tests failed. Please check the relevant

Re: Review Request 68018: Added `SeccompFilter` class.

> On Jan. 14, 2019, 4:31 p.m., Qian Zhang wrote: > > src/linux/seccomp/seccomp.cpp > > Lines 137-139 (patched) > > > > > > Will this affect the task run by Mesos? E.g., a task may want to run a > > program which

Re: Review Request 68147: Added agent support to remove local resource providers.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/#review212103 --- Fix it, then Ship it! src/slave/http.cpp Lines 3355 (patched)

Re: Review Request 69158: Added an integration test for resource provider removal.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69158/#review212104 --- src/tests/slave_tests.cpp Lines 10731 (patched)

Re: Review Request 69588: Removed outdated authorization logic for offer operations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69588/#review212089 --- Patch looks great! Reviews applied: [69571, 69563, 69577, 69578,

Re: Review Request 69776: Fixed flakiness by adding per agent config dir for mesos test.

> On Jan. 16, 2019, 10:10 p.m., Jie Yu wrote: > > src/tests/mesos.cpp > > Line 173 (original), 173 (patched) > > > > > > s/agentDir/configDir/ > > Gilbert Song wrote: > I thought about `configDir`, sounds not

Re: Review Request 69775: Updated master fail() logging from FATAL to ERROR.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69775/#review212092 --- Thanks for doing this, this will avoid a lot of confusion around

Re: Review Request 69158: Added an integration test for resource provider removal.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69158/#review212097 --- Patch looks great! Reviews applied: [68147, 69158] Passed

Re: Review Request 69158: Added an integration test for resource provider removal.

> On Jan. 15, 2019, 6:58 a.m., Chun-Hung Hsiao wrote: > > src/tests/slave_tests.cpp > > Lines 10625-10626 (patched) > > > > > > ``` > > Owned detector = master.get()->createDetector(); > > Try> slave =

Re: Review Request 68147: Added agent support to remove local resource providers.

> On Jan. 15, 2019, 6:53 a.m., Chun-Hung Hsiao wrote: > > src/tests/api_tests.cpp > > Lines 7857 (patched) > > > > > > ``` > > Owned detector = master.get()->createDetector(); > > ``` > > And > >

Re: Review Request 69158: Added an integration test for resource provider removal.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69158/ --- (Updated Jan. 16, 2019, 11:51 a.m.) Review request for mesos, Chun-Hung Hsiao

Re: Review Request 68147: Added agent support to remove local resource providers.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/ --- (Updated Jan. 16, 2019, 11:51 a.m.) Review request for mesos, Chun-Hung Hsiao,

Re: Review Request 68147: Added agent support to remove local resource providers.

> On Aug. 18, 2018, 12:29 a.m., Chun-Hung Hsiao wrote: > > src/slave/slave.cpp > > Lines 8195 (patched) > > > > > > Let's validate that there is no task using the resources provided by > > this RP before doing the

Re: Review Request 68018: Added `SeccompFilter` class.

> On Jan. 14, 2019, 8:31 a.m., Qian Zhang wrote: > > src/linux/seccomp/seccomp.cpp > > Lines 137-139 (patched) > > > > > > Will this affect the task run by Mesos? E.g., a task may want to run a > > program which

Re: Review Request 68018: Added `SeccompFilter` class.

> On Jan. 14, 2019, 8:31 a.m., Qian Zhang wrote: > > src/linux/seccomp/seccomp.cpp > > Lines 137-139 (patched) > > > > > > Will this affect the task run by Mesos? E.g., a task may want to run a > > program which

Re: Review Request 68017: Added Seccomp-related protobuf messages.

> On Jan. 14, 2019, 3:38 p.m., Gilbert Song wrote: > > include/mesos/seccomp/seccomp.proto > > Lines 21-28 (patched) > > > > > > This is my fault. Sorry, Andrei! > > > > Would you mind moving this .proto to

Re: Review Request 68017: Added Seccomp-related protobuf messages.

> On Jan. 14, 2019, 6:39 p.m., Gilbert Song wrote: > > include/mesos/mesos.proto > > Lines 3158-3159 (patched) > > > > > > Seems like this was added recently. > > > > Is this field only used when there is