Review Request 39086: Fixed typos in comments and docs.

2015-10-07 Thread Gastón Kleiman
/docker.cpp 6c975f904178e01797b67628a2d471ec7b3b1fbf src/zookeeper/authentication.hpp 5919ec021465cf4f27a51041ff635730d0043eb9 Diff: https://reviews.apache.org/r/39086/diff/ Testing --- I only changed comments and docs, but I made sure that "make check" passes on OS X 10.10.5. Thank

Re: Review Request 39086: Fixed typos in comments and docs.

2015-10-07 Thread Gastón Kleiman
name (for <gas...@mesosphere.com>) not allowed > > Failed to commit patch https://reviews.apache.org/r/39087/ should make this patch apply. - Gastón --- This is an automatically generated e-mail. To reply, vi

Re: Review Request 49958: Fixed Docker daemon args generation.

2016-07-12 Thread Gastón Kleiman
sit: https://reviews.apache.org/r/49958/#review141962 --- On July 12, 2016, 5:04 p.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To re

Re: Review Request 50366: Fixed a bug in docker volume driver generation code.

2016-07-22 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50366/#review143300 --- Ship it! LGTM, thanks for catching the bug! - Gastón Kleiman

Re: Review Request 50563: Made HierarchicalAllocatorProcess specify a process ID.

2016-07-31 Thread Gastón Kleiman
, Gastón Kleiman

Re: Review Request 50637: Added id to the libprocess Sequence constructor.

2016-07-31 Thread Gastón Kleiman
esses__" endpoint to them. This change makes it possible to give sequence processes a distinguishable ID. Diffs - 3rdparty/libprocess/include/process/sequence.hpp 78ccdb95422d0f7e12f1d66b4456c4f0014f4ffe Diff: https://reviews.apache.org/r/50637/diff/ Testing --- make check Thanks, Gastón Kleiman

Re: Review Request 50519: Changed AuthenticatorManagerProcess's Actor ID.

2016-07-31 Thread Gastón Kleiman
Diff: https://reviews.apache.org/r/50519/diff/ Testing (updated) --- `make check` in OS X and various linux distribution, checked the output of `__processes__`. Thanks, Gastón Kleiman

Re: Review Request 50515: Added missing process IDs.

2016-07-31 Thread Gastón Kleiman
n. I also looked at the output of the `__process__` endpoint. After this change, most of the unidentifiable processes have a nicer ID. The goal of this review chain is to give all processes there an identifiable ID. Thanks, Gastón Kleiman

Review Request 50639: Made all Actor IDs outside of libprocess follow the same schema.

2016-07-31 Thread Gastón Kleiman
`. Thanks, Gastón Kleiman

Re: Review Request 50513: Added missing process IDs in libprocess.

2016-08-02 Thread Gastón Kleiman
k` in OS X and various linux distribution. I also looked at the output of the `__process__` endpoint. After this change, some of the unidentifiable processes have a nicer id now. The goal of this review chain is to give all processes there an identifiable ID. Thanks, Gastón Kleiman

Re: Review Request 50513: Added missing process IDs in libprocess.

2016-08-02 Thread Gastón Kleiman
s an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50513/#review144082 --- On Aug. 2, 2016, 1:45 p.m., Gastón Kleiman wrote: > > --

Re: Review Request 50639: Made all Actor IDs outside of libprocess follow the same schema.

2016-08-02 Thread Gastón Kleiman
distribution, checked the output of `__processes__`. Thanks, Gastón Kleiman

Re: Review Request 50513: Added missing process IDs in libprocess.

2016-08-02 Thread Gastón Kleiman
> On July 29, 2016, 12:44 p.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/src/authenticator.cpp, line 57 > > <https://reviews.apache.org/r/50513/diff/1/?file=1455330#file1455330line57> > > > > Why this one is enclosed in underscores? > > Gastó

Re: Review Request 50637: Added id to the libprocess Sequence constructor.

2016-08-02 Thread Gastón Kleiman
c39865 src/v1/resources.cpp 3f67e32519600a22f07a66ebfe05a7b647cb2c61 Diff: https://reviews.apache.org/r/50637/diff/ Testing --- make check Thanks, Gastón Kleiman

Re: Review Request 50519: Made libprocess Actor IDs consistent.

2016-08-02 Thread Gastón Kleiman
/src/reap.cpp ac60c6d769076912293950432266c956d6c7e705 Diff: https://reviews.apache.org/r/50519/diff/ Testing --- `make check` in OS X and various linux distribution, checked the output of `__processes__`. Thanks, Gastón Kleiman

Re: Review Request 50515: Added missing process IDs.

2016-08-02 Thread Gastón Kleiman
s` above. Fixed here and in `src/linux/cgroups.cpp`. - Gastón --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50515/#review144087 ------- On Aug. 2

Re: Review Request 50563: Made HierarchicalAllocatorProcess specify a process ID.

2016-08-02 Thread Gastón Kleiman
`. Thanks, Gastón Kleiman

Re: Review Request 50637: Added id to the libprocess Sequence constructor.

2016-08-02 Thread Gastón Kleiman
endpoint to them. This change makes it possible to give sequence processes a distinguishable ID. Diffs (updated) - 3rdparty/libprocess/include/process/sequence.hpp 78ccdb95422d0f7e12f1d66b4456c4f0014f4ffe Diff: https://reviews.apache.org/r/50637/diff/ Testing --- make check Thanks, Gastón Kleiman

Re: Review Request 50515: Added missing process IDs.

2016-08-02 Thread Gastón Kleiman
int. After this change, most of the unidentifiable processes have a nicer ID. The goal of this review chain is to give all processes there an identifiable ID. Thanks, Gastón Kleiman

Re: Review Request 50515: Added missing process IDs.

2016-08-02 Thread Gastón Kleiman
----- On Aug. 2, 2016, 1:47 p.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50515/ >

Re: Review Request 50521: Added "zookeeper" prefix to ZK process IDs.

2016-08-02 Thread Gastón Kleiman
tender.cpp 320658182c85d333539be3c20534ad889a2ce99e src/zookeeper/detector.cpp f1def68740e4941a5daa3a889bc14278d7ea7366 src/zookeeper/group.cpp 7438eccf285ad2ab6462c0bb091966179d6906d5 Diff: https://reviews.apache.org/r/50521/diff/ Testing --- `make check` in OS X and various linux distribution. Thanks, Gastón Kleiman

Re: Review Request 50078: Cleaned the arguments passed to health checker in docker executor.

2016-07-15 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50078/#review142377 --- Ship it! Ship It! - Gastón Kleiman On July 15, 2016, 2:13

Review Request 49958: Fixed Docker daemon args generation.

2016-07-12 Thread Gastón Kleiman
00ccc2fc54a984f3a67573ce6fe7aee3e60ce3a2 Diff: https://reviews.apache.org/r/49958/diff/ Testing --- Added two new tests and they pass. Thanks, Gastón Kleiman

Re: Review Request 50812: Updated `HealthCheck` protobuf for HTTP and TCP health check.

2016-08-05 Thread Gastón Kleiman
org/r/50812/#comment211083> What do you think about calling this message `TCP` instead of `Socket`? - Gastón Kleiman On Aug. 4, 2016, 5:30 p.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail.

Review Request 50897: Added compiler information to the output of ./configure.

2016-08-08 Thread Gastón Kleiman
with: `GCC 4.8.1, 4.8.4, 4.8.5, 4.9.2, 5.3.1` `Clang 7.0.2` Thanks, Gastón Kleiman

Re: Review Request 50920: Removed extra spaces in docker.cpp.

2016-08-09 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50920/#review145194 --- Ship it! Ship It! - Gastón Kleiman On Aug. 9, 2016, 10:18

Re: Review Request 50897: Added compiler information to the output of ./configure.

2016-08-09 Thread Gastón Kleiman
checking for C++ compiler vendor... (cached) clang ``` Which is why I'm discarding this RR. Sorry for the inconvenience! - Gastón Kleiman On Aug. 8, 2016, 3:57 p.m., Gastón Kleiman wrote: > > --- > This is an automatically generat

Re: Review Request 50921: Fixed the incorrect order of header files in executor.cpp.

2016-08-09 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50921/#review145196 --- Ship it! Ship It! - Gastón Kleiman On Aug. 9, 2016, 10:18

Re: Review Request 50853: Resolved C++11-related TODO in master/master.cpp.

2016-08-08 Thread Gastón Kleiman
ly, visit: https://reviews.apache.org/r/50853/#review145061 --- On Aug. 5, 2016, 3:30 p.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To rep

Re: Review Request 50812: Updated `HealthCheck` protobuf for HTTP and TCP health check.

2016-08-08 Thread Gastón Kleiman
will be applied) include/mesos/mesos.proto (line 337) <https://reviews.apache.org/r/50812/#comment211277> Ideally this should be an enum, but I guess that we want to be consistent with `message URL`. - Gastón Kleiman On Aug. 5, 2016, 4:19 p.m., haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-03 Thread Gastón Kleiman
ate, then using `localhost` for the domain might be a problem. - Gastón Kleiman On Aug. 1, 2016, 1:15 p.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https:/

Re: Review Request 49360: Supported TCP check in health check.

2016-08-03 Thread Gastón Kleiman
ker.cpp (lines 56 - 72) <https://reviews.apache.org/r/49360/#comment210640> This can be simplified if you used the "union trick" suggested by AlexR in https://reviews.apache.org/r/36816 - Gastón Kleiman On Aug. 1,

Review Request 56288: Improved the wording of what's logged on command health check timeouts.

2017-02-03 Thread Gastón Kleiman
: https://reviews.apache.org/r/56288/diff/ Testing --- None, not a functional change. Thanks, Gastón Kleiman

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-03 Thread Gastón Kleiman
8418cd91484fd26734de16255b37f3ebf574f5eb Diff: https://reviews.apache.org/r/55901/diff/ Testing --- Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It passes on Linux, but not on macOS. Thanks, Gastón Kleiman

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-03 Thread Gastón Kleiman
Diff: https://reviews.apache.org/r/55901/diff/ Testing --- Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It passes on Linux, but not on macOS. Thanks, Gastón Kleiman

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-03 Thread Gastón Kleiman
------------ This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55901/#review164117 --- On Feb. 3, 2017, 5:15 p.m., Gastón Kleiman wrote: > >

Re: Review Request 55322: Used process::after instead of process::RateLimiter.

2017-02-02 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55322/#review163957 --- Ship it! Ship It! - Gastón Kleiman On Jan. 8, 2017, 7:49

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-01 Thread Gastón Kleiman
t; > you are calling `post()` helper here which expects a non-streaming response? > > > > probably one of the reasons why your test is hanging. > > Gastón Kleiman wrote: > The `post()` helper delegates to `process::http::request()`, which takes > boolean flag (

Re: Review Request 55321: Introduced process::after.

2017-02-01 Thread Gastón Kleiman
tps://reviews.apache.org/r/55321/#comment235320> s/let's/lets/ - Gastón Kleiman On Jan. 8, 2017, 7:49 a.m., Benjamin Hindman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 56208: Updated checks library with general check support.

2017-02-08 Thread Gastón Kleiman
g/r/56208/#comment236446> s/taskID/taskId/ We don't use the name `taskID` anywhere else in Mesos. src/checks/checker.hpp (line 95) <https://reviews.apache.org/r/56208/#comment236447> ditto capitalization of the name. - Gastón Kleiman On Feb. 8, 2017, 4:56 p.m

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-07 Thread Gastón Kleiman
> On Jan. 28, 2017, 1:17 a.m., Vinod Kone wrote: > > src/checks/health_checker.cpp, lines 426-429 > > <https://reviews.apache.org/r/55901/diff/1/?file=1613998#file1613998line426> > > > > why a `.repair()` here? > > Gastón Kleiman wrote: > To fai

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-07 Thread Gastón Kleiman
7b6a803a28b2e4f6c27e9a0c4f668350ec2d5a81 Diff: https://reviews.apache.org/r/55901/diff/ Testing --- Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It passes on Linux, but not on macOS. Thanks, Gastón Kleiman

Re: Review Request 56408: Fixed a regression in the execute CLI command.

2017-02-07 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56408/#review164572 --- Ship it! Ship It! - Gastón Kleiman On Feb. 7, 2017, 10:28

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-07 Thread Gastón Kleiman
7b6a803a28b2e4f6c27e9a0c4f668350ec2d5a81 Diff: https://reviews.apache.org/r/55901/diff/ Testing --- Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It passes on Linux, but not on macOS. Thanks, Gastón Kleiman

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-07 Thread Gastón Kleiman
iff/5/?file=1623742#file1623742line585> > > > > Mention `taskId`? Updated all the comments I could find. - Gastón --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.or

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-07 Thread Gastón Kleiman
nline` > > > > This looks like a utility function not really related to the health > > checker library. Maybe put it into "src/slave/api_utils.{hpp|cpp}"? > > Gastón Kleiman wrote: > I renamed the function. Vinod suggested to make it `inline` what'

Re: Review Request 55321: Introduced process::after.

2017-02-02 Thread Gastón Kleiman
tps://reviews.apache.org/r/55321/#comment235475> Shouldn't we update the cmake lists as well? - Gastón Kleiman On Jan. 8, 2017, 7:49 a.m., Benjamin Hindman wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-08 Thread Gastón Kleiman
Diff: https://reviews.apache.org/r/55901/diff/ Testing --- Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It passes on Linux, but not on macOS. Thanks, Gastón Kleiman

Re: Review Request 56250: Cleaned up headers in DRF sorter.

2017-02-03 Thread Gastón Kleiman
/sorter.cpp (line 32) <https://reviews.apache.org/r/56250/#comment235694> According to recent discussions and to our code style guide, this should be the first include in the file. - Gastón Kleiman On Feb. 2, 2017, 10:59 p.m., Neil Conway

Re: Review Request 55900: Improved style in `HealthChecker`.

2017-02-02 Thread Gastón Kleiman
(updated) - src/checks/health_checker.hpp 95da1ff7dd6b222a93076633eb3757ec9aa43cf6 src/checks/health_checker.cpp 58380dc18896f659aa9c4fb4bb567a55bba97f6b Diff: https://reviews.apache.org/r/55900/diff/ Testing --- `make check` (macOS and Linux) Thanks, Gastón Kleiman

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-02 Thread Gastón Kleiman
8418cd91484fd26734de16255b37f3ebf574f5eb Diff: https://reviews.apache.org/r/55901/diff/ Testing --- Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It passes on Linux, but not on macOS. Thanks, Gastón Kleiman

Re: Review Request 55901: [WIP] Added support for command health checks to the default executor.

2017-01-28 Thread Gastón Kleiman
------- On Jan. 25, 2017, 12:33 a.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55901/ > --

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-01-28 Thread Gastón Kleiman
. Thanks, Gastón Kleiman

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-01-28 Thread Gastón Kleiman
> > const & ? This connection is passed to `nestedCommandHealthCheckTimedOut()`, which calls the non-const method `connection.disconnect()`. - Gastón --- This is an automatically generated e-mail. To re

Re: Review Request 55901: [WIP] Added support for command health checks to the default executor.

2017-01-25 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55901/#review162972 --- It looks like there's a deadlock in the cleanup path of the

Re: Review Request 55900: Improved style in `HealthChecker`.

2017-01-25 Thread Gastón Kleiman
/health_checker.hpp 6e558f2061a9e31157c47d31cb64b3a8568aace3 src/checks/health_checker.cpp 50aa2858e807b27bbab58a3618f5200cfe4eca9e Diff: https://reviews.apache.org/r/55900/diff/ Testing --- `make check` (macOS and Linux) Thanks, Gastón Kleiman

Re: Review Request 55900: Improved style in `HealthChecker`.

2017-01-25 Thread Gastón Kleiman
eviews.apache.org/r/55900/#review162982 --- On Jan. 25, 2017, 6:15 p.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: &g

Review Request 55901: [WIP] Added support for command health checks to the default executor.

2017-01-24 Thread Gastón Kleiman
. Thanks, Gastón Kleiman

Review Request 55899: Renamed `taskID` to `taskId` in `HealthChecker`.

2017-01-24 Thread Gastón Kleiman
/checks/health_checker.hpp 6e558f2061a9e31157c47d31cb64b3a8568aace3 src/checks/health_checker.cpp 50aa2858e807b27bbab58a3618f5200cfe4eca9e Diff: https://reviews.apache.org/r/55899/diff/ Testing --- `make check` (macOS and Linux) Thanks, Gastón Kleiman

Review Request 55900: Improved style in `HealthChecker`.

2017-01-24 Thread Gastón Kleiman
6e558f2061a9e31157c47d31cb64b3a8568aace3 src/checks/health_checker.cpp 50aa2858e807b27bbab58a3618f5200cfe4eca9e Diff: https://reviews.apache.org/r/55900/diff/ Testing --- `make check` (macOS and Linux) Thanks, Gastón Kleiman

Re: Review Request 56962: Windows: Undefined `ACL` macro defined in Zookeeper headers.

2017-02-23 Thread Gastón Kleiman
! - Gastón Kleiman On Feb. 23, 2017, 1:24 a.m., Joseph Wu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 56212: Added support for general checks to command executor.

2017-02-14 Thread Gastón Kleiman
o keep it at least for `TASK_KILLING`. It would also make sense to keep the `health` flag in that case. - Gastón Kleiman On Feb. 8, 2017, 4:56 p.m., Alexander Rukletsov wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 56208: Updated checks library with general check support.

2017-02-14 Thread Gastón Kleiman
pp (line 497) <https://reviews.apache.org/r/56208/#comment237393> Ditto `taskId`. src/checks/checker.cpp (line 570) <https://reviews.apache.org/r/56208/#comment237395> Ditto `taskId`. - Gastón Kleiman On Feb. 9, 2017, 12:56 p.m., Alexander Rukletsov wrote: > >

Re: Review Request 56210: Reused previous task status to generate a new one in command executor.

2017-02-14 Thread Gastón Kleiman
org/r/56210/#comment237405> s/Rewrite/Overwrite/ or Update? - Gastón Kleiman On Feb. 9, 2017, 12:56 p.m., Alexander Rukletsov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://re

Re: Review Request 54846: Removed `docker exec` when perform health checks in docker executor.

2017-02-14 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54846/#review165493 --- Ship it! Ship It! - Gastón Kleiman On Dec. 18, 2016, 5:30

Re: Review Request 56601: Used single quotes instead of backticks in error message.

2017-02-14 Thread Gastón Kleiman
o src/master/validation.cpp (line 961) <https://reviews.apache.org/r/56601/#comment237343> ditto src/master/validation.cpp (line 1239) <https://reviews.apache.org/r/56601/#comment237344> ditto - Gastón K

Re: Review Request 56448: Shortened continuation name in default executor.

2017-02-09 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56448/#review165017 --- Ship it! Ship It! - Gastón Kleiman On Feb. 8, 2017, 4:56

Re: Review Request 56211: Renamed health checker in command executor for clarity.

2017-02-09 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56211/#review165018 --- Ship it! Ship It! - Gastón Kleiman On Feb. 8, 2017, 4:56

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-10 Thread Gastón Kleiman
/health_check_tests.cpp 7b6a803a28b2e4f6c27e9a0c4f668350ec2d5a81 Diff: https://reviews.apache.org/r/55901/diff/ Testing --- Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It passes on Linux, but not on macOS, because of MESOS-7050. Thanks, Gastón Kleiman

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-10 Thread Gastón Kleiman
; It's not clear what "this future" means. Fixed. - Gastón ------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55901/#review164919

Re: Review Request 56213: Added check tests for command executor.

2017-02-14 Thread Gastón Kleiman
- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56213/ > ------- > > (Updated Feb. 8, 2017, 4:56 p.m.) > > > Review request for mesos, Gastón Kleiman and Vinod Kone

Re: Review Request 56449: Moved health checker closer to container in default executor.

2017-02-15 Thread Gastón Kleiman
tps://reviews.apache.org/r/56449/#comment237555> Looks like making `healthChecker` an `Option` doesn't gain us much, since we still have to be defensive and do `CHECK_NOTNULL` before using it. - Gastón Kleiman On Feb. 8, 2017, 4:56 p.m., Alexander Rukletsov

Re: Review Request 56213: Added check tests for command executor.

2017-02-15 Thread Gastón Kleiman
<https://reviews.apache.org/r/56213/#comment237554> s/http/HTTP/ - Gastón Kleiman On Feb. 8, 2017, 4:56 p.m., Alexander Rukletsov wrote: > > --- > This is an automatically generated e-mail. To reply

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-09 Thread Gastón Kleiman
://reviews.apache.org/r/55901/#review163363 --- On Feb. 8, 2017, 1:27 p.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-10 Thread Gastón Kleiman
Diff: https://reviews.apache.org/r/55901/diff/ Testing (updated) --- Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It passes on Linux, but not on macOS, because of MESOS-7050. Thanks, Gastón Kleiman

Re: Review Request 56288: Improved the wording of what's logged on command health check timeouts.

2017-02-10 Thread Gastón Kleiman
(updated) - src/checks/health_checker.cpp a5225ff1f4b071ed4182d41fa8ecc705fa4dbe00 Diff: https://reviews.apache.org/r/56288/diff/ Testing --- None, not a functional change. Thanks, Gastón Kleiman

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-10 Thread Gastón Kleiman
er.cpp (line 667) <https://reviews.apache.org/r/55901/#comment236789> It depends on what containerizer is being used. With the `MesosContainerizer`, we get the exit status as returned by `waitpid`. I added a comment in the header file. - Gastón Kleiman On Feb. 8, 2017, 1:27 p.m., Ga

Re: Review Request 56213: Added check tests for command executor.

2017-02-16 Thread Gastón Kleiman
> > > we are not doing this change yet. > > Gastón Kleiman wrote: > We do this in `health_check_tests.cpp`: > > ``` > $ grep "agent = " health_check_tests.cpp | wc -l > 15 > $ grep "slave = " health_check_

Re: Review Request 56601: Used single quotes instead of backticks in error message.

2017-02-16 Thread Gastón Kleiman
> On Feb. 14, 2017, 1:48 p.m., Gastón Kleiman wrote: > > I think that we should also update `checks/checker.cpp` and > > `checks/health_checker.cpp`: > > > > ``` > > $ ag --cpp --nomultiline '^\s*[^/"]+".*`' > > checks/checker.cpp > > 60:

Review Request 56413: WIP Support pausing/resuming health checks.

2017-02-16 Thread Gastón Kleiman
/ Testing --- Thanks, Gastón Kleiman

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-16 Thread Gastón Kleiman
wait until MESOS-7120 is resolved, and then update this patch to not re-use the ContainerID. - Gastón --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55901/#review165630 --

Re: Review Request 56288: Improved the wording of what's logged on command health check timeouts.

2017-02-16 Thread Gastón Kleiman
/checks/health_checker.cpp 6c97369fd9a567ba16dd92085bf142d43f71eaf1 Diff: https://reviews.apache.org/r/56288/diff/ Testing --- None, not a functional change. Thanks, Gastón Kleiman

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-16 Thread Gastón Kleiman
476e04a12f885e5060a6e838b82ee599594c96f7 Diff: https://reviews.apache.org/r/55901/diff/ Testing --- Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It passes on Linux, but not on macOS, because of MESOS-7050. Thanks, Gastón Kleiman

Re: Review Request 56213: Added check tests for command executor.

2017-02-15 Thread Gastón Kleiman
org/r/56213/#comment237576> We should add: ``` ASSERT_EQ(TaskStatus::REASON_TASK_CHECK_STATUS_UPDATED, checkResult.reason()); ``` We should do something like this in all tests each time we get an update. - Gastón Kleiman On Feb. 8, 2017, 4:56 p.m., Ale

Re: Review Request 56217: Added support for general checks to default executor.

2017-02-15 Thread Gastón Kleiman
ybe the best way would be to implement the `ostream::operator<<` for `CheckStatusInfo`? - Gastón Kleiman On Feb. 8, 2017, 4:57 p.m., Alexander Rukletsov wrote: > > --- > This is an automatically generated e-mai

Re: Review Request 56215: Reused previous task status to generate a new one in default executor.

2017-02-15 Thread Gastón Kleiman
ent237559> If we keep this as-is, then I'd replace `Rewrite` with `Update` or `Overwrite`. - Gastón Kleiman On Feb. 9, 2017, 12:56 p.m., Alexander Rukletsov wrote: > > --- > This is an automatically generated e-mail. To

Re: Review Request 56218: Added some check tests for default executor.

2017-02-15 Thread Gastón Kleiman
re, you should also follow it here. Drop this issue otherwise =). src/tests/check_tests.cpp (line 914) <https://reviews.apache.org/r/56218/#comment237569> ditto - Gastón Kleiman On Feb. 9, 201

Re: Review Request 55768: Updated mesos-execute to show check's status.

2017-01-24 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55768/#review162803 --- Ship it! Ship It! - Gastón Kleiman On Jan. 20, 2017, 2:52

Re: Review Request 55876: Avoided shadowing in `Slave::run()`.

2017-01-24 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55876/#review162824 --- Ship it! Ship It! - Gastón Kleiman On Jan. 24, 2017, 2:54

Re: Review Request 55901: Added support for command health checks to the default executor.

2017-02-17 Thread Gastón Kleiman
Diff: https://reviews.apache.org/r/55901/diff/ Testing --- Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It passes on Linux, but not on macOS, because of MESOS-7050. Thanks, Gastón Kleiman

Re: Review Request 56413: WIP Support pausing/resuming health checks.

2017-02-17 Thread Gastón Kleiman
d9417ce1d5b108f5292a66010eab80f11552a969 Diff: https://reviews.apache.org/r/56413/diff/ Testing --- Thanks, Gastón Kleiman

Re: Review Request 55679: Unified the way in which the Slave API handlers perform AuthZ.

2017-01-19 Thread Gastón Kleiman
(updated) - src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee Diff: https://reviews.apache.org/r/55679/diff/ Testing --- `make check` in macOS and Linux. Thanks, Gastón Kleiman

Re: Review Request 55678: Renamed `locateExecutor` to `getExecutor`.

2017-01-19 Thread Gastón Kleiman
/diff/ Testing --- `make check` in macOS and Linux. Thanks, Gastón Kleiman

Re: Review Request 55677: Made `AttachContainerOutput/Input` tests use an existing container ID.

2017-01-19 Thread Gastón Kleiman
://reviews.apache.org/r/55677/diff/ Testing --- `make check` in macOS and Linux. Thanks, Gastón Kleiman

Re: Review Request 55676: Moved `getRootContainerId` to `protobuf_utils`.

2017-01-19 Thread Gastón Kleiman
/utils.hpp a54106dc4893bb222f42ede936ac9029e817faf9 src/slave/containerizer/mesos/utils.cpp 4e2a01495909c07f320af416ff4dc59f7328c710 Diff: https://reviews.apache.org/r/55676/diff/ Testing --- `make check` in macOS and Linux. Thanks, Gastón Kleiman

Re: Review Request 55464: Made the Agent API able to handle containers nested at arbitrary levels.

2017-01-18 Thread Gastón Kleiman
--- `GTEST_FILTER="" make -j 8 check` in macOS. `make check` in Linux. I also did some manual testing using a proof of concept that makes the `DefaultExecutor` leverage this change to perform CMD health checks. Thanks, Gastón Kleiman

Review Request 55679: Unified the way in which the Slave API handlers perform AuthZ.

2017-01-18 Thread Gastón Kleiman
Diff: https://reviews.apache.org/r/55679/diff/ Testing --- `make check` in macOS and Linux. Thanks, Gastón Kleiman

Re: Review Request 55464: Made the Agent API able to handle containers nested at arbitrary levels.

2017-01-18 Thread Gastón Kleiman
otContainerId()` in > > `slave/containerizer/mesos/utils.hpp`? > > > > You might have to pull it out though first and then re-use it in this > > review? Strictly speaking, this method is pretty generic and should not be > > a part of the Mes

Review Request 55677: Made `AttachContainerOutput/Input` tests use an existing container ID.

2017-01-18 Thread Gastón Kleiman
and Linux. Thanks, Gastón Kleiman

Review Request 55676: Moved `getRootContainerId` to `protobuf_utils`.

2017-01-18 Thread Gastón Kleiman
/55676/diff/ Testing --- `make check` in macOS and Linux. Thanks, Gastón Kleiman

  1   2   3   4   5   6   7   8   >