Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review110417 --- Ship it! Ship It! - Maxim Khutornenko On Dec. 15, 2015, 4:57 a.

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review110412 --- Ship it! Master (f91ecd1) is green with this patch. ./build-supp

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/ --- (Updated Dec. 15, 2015, 4:57 a.m.) Review request for Aurora, Maxim Khutornenko

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review110405 --- Master (f91ecd1) is red with this patch. ./build-support/jenkins/

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review110404 --- @ReviewBot retry - Dmitriy Shirchenko On Dec. 15, 2015, 3:56 a.m

Re: Review Request 41154: Add support for performing health checks with a shell command.

> On Dec. 15, 2015, 4:06 a.m., Aurora ReviewBot wrote: > > Master (f91ecd1) is red with this patch. > > ./build-support/jenkins/build.sh > > > > virtualenv-12.1.1/virtualenv_support/ > > virtualenv-12.1.1/virtualenv_support/__init__.py > > virtualenv-12.1.1/virtualenv_support/pip-6.1.1-py2.py3

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review110396 --- Master (f91ecd1) is red with this patch. ./build-support/jenkins/

Re: Review Request 41154: Add support for performing health checks with a shell command.

> On Dec. 12, 2015, 12:06 a.m., Maxim Khutornenko wrote: > > src/main/python/apache/aurora/config/schema/base.py, line 51 > > > > > > Please, don't use 'type' for a field name. It's too generic and clashes > > with p

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/ --- (Updated Dec. 15, 2015, 3:56 a.m.) Review request for Aurora, Maxim Khutornenko

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review110285 --- Ship it! So long as a blocking ticket is filed for AURORA-1525 I'm

Re: Review Request 41154: Add support for performing health checks with a shell command.

> On Dec. 12, 2015, 12:06 a.m., Maxim Khutornenko wrote: > > src/main/python/apache/aurora/config/schema/base.py, line 50 > > > > > > I second Zameer's concerns about logical conflict between the new > > `shell_comma

Re: Review Request 41154: Add support for performing health checks with a shell command.

> On Dec. 11, 2015, 7:43 p.m., Zameer Manji wrote: > > Overall this LGTM. The only thing holding back my shipit is a possible > > addition to the e2e test suite. > > > > Bill, Maxim, Josh: Do you think this change requires an addition to the e2e > > suite? I'm on the fence. It seems overkill f

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review110056 --- Ship it! Ship It! - Bill Farner On Dec. 10, 2015, 11:25 p.m., D

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review110055 --- Ship it! LGTM. I'm on board with a follow up to address schema cha

Re: Review Request 41154: Add support for performing health checks with a shell command.

> On Dec. 11, 2015, 4:06 p.m., Maxim Khutornenko wrote: > > src/main/python/apache/aurora/config/schema/base.py, line 50 > > > > > > I second Zameer's concerns about logical conflict between the new > > `shell_comman

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review110040 --- src/main/python/apache/aurora/config/schema/base.py (line 46)

Re: Review Request 41154: Add support for performing health checks with a shell command.

> On Dec. 11, 2015, 11:43 a.m., Zameer Manji wrote: > > Overall this LGTM. The only thing holding back my shipit is a possible > > addition to the e2e test suite. > > > > Bill, Maxim, Josh: Do you think this change requires an addition to the e2e > > suite? I'm on the fence. It seems overkill

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review109992 --- Overall this LGTM. The only thing holding back my shipit is a possi

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/ --- (Updated Dec. 11, 2015, 7:25 a.m.) Review request for Aurora, Maxim Khutornenko

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review109928 --- Ship it! Master (6933a71) is green with this patch. ./build-supp

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/ --- (Updated Dec. 11, 2015, 7:06 a.m.) Review request for Aurora, Maxim Khutornenko

Re: Review Request 41154: Add support for performing health checks with a shell command.

> On Dec. 11, 2015, 1:28 a.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/client/config.py, lines 98-110 > > > > > > I feel like this validation, while absolutely important on the client, > > should also b

Re: Review Request 41154: Add support for performing health checks with a shell command.

> On Dec. 11, 2015, 4:30 a.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/common/health_check/shell.py, line 47 > > > > > > Should we use `shlex.split` here instead? Done. - Dmitriy ---

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review109914 --- src/main/python/apache/aurora/common/health_check/shell.py (line 4

Re: Review Request 41154: Add support for performing health checks with a shell command.

> On Dec. 11, 2015, 1:28 a.m., Joshua Cohen wrote: > > src/test/python/apache/aurora/common/BUILD, line 26 > > > > > > I don't think we need this dependency here. The tests under > > health_check will be picked up wh

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review109911 --- Ship it! Master (6933a71) is green with this patch. ./build-supp

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/ --- (Updated Dec. 11, 2015, 2:36 a.m.) Review request for Aurora, Maxim Khutornenko

Re: Review Request 41154: Add support for performing health checks with a shell command.

> On Dec. 11, 2015, 1:28 a.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/client/config.py, lines 98-110 > > > > > > I feel like this validation, while absolutely important on the client, > > should also b

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review109901 --- docs/configuration-reference.md (lines 387 - 389)

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review109895 --- Ship it! Master (6933a71) is green with this patch. ./build-supp

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/ --- (Updated Dec. 11, 2015, 12:08 a.m.) Review request for Aurora, Maxim Khutornenk

Re: Review Request 41154: Add support for performing health checks with a shell command.

> On Dec. 10, 2015, 9:54 p.m., Zameer Manji wrote: > > src/main/python/apache/aurora/executor/common/health_checker.py, line 212 > > > > > > It seems like a logical error to both set "shell_command" and have the > >

Re: Review Request 41154: Add support for performing health checks with a shell command.

> On Dec. 10, 2015, 10:11 p.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/common/health_check/BUILD, lines 1-28 > > > > > > I don't think this BUILD file should exist. Per our [python build > > conventions

Re: Review Request 41154: Add support for performing health checks with a shell command.

> On Dec. 10, 2015, 10:11 p.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/common/health_check/shell.py, line 49 > > > > > > I second Steve's concern about adding a dependency on subprocess32. Can > > you

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review109870 --- src/main/python/apache/aurora/common/health_check/BUILD (lines 1 -

Re: Review Request 41154: Add support for performing health checks with a shell command.

> On Dec. 10, 2015, 1:23 p.m., Steve Niemitz wrote: > > Overall comment here: is there a really good reason to introduce another > > third party library (subprocess32)? Especially because its not pure python > > (there's a C extension). I'd prefer to see this done without introducing > > oth

Re: Review Request 41154: Add support for performing health checks with a shell command.

> On Dec. 10, 2015, 9:23 p.m., Steve Niemitz wrote: > > Overall comment here: is there a really good reason to introduce another > > third party library (subprocess32)? Especially because its not pure python > > (there's a C extension). I'd prefer to see this done without introducing > > oth

Re: Review Request 41154: Add support for performing health checks with a shell command.

> On Dec. 10, 2015, 1:23 p.m., Steve Niemitz wrote: > > Overall comment here: is there a really good reason to introduce another > > third party library (subprocess32)? Especially because its not pure python > > (there's a C extension). I'd prefer to see this done without introducing > > oth

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review109859 --- I have not looked at the tests yet but the implementation looks ok

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review109857 --- Master (6933a71) is red with this patch. ./build-support/jenkins/

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review109855 --- Overall comment here: is there a really good reason to introduce an

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/ --- (Updated Dec. 10, 2015, 9:16 p.m.) Review request for Aurora, Maxim Khutornenko

Re: Review Request 41154: Add support for performing health checks with a shell command.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/ --- (Updated Dec. 10, 2015, 9:15 p.m.) Review request for Aurora, Maxim Khutornenko