Re: Review Request 30695: Implements log rotation in the Thermos runner.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30695/#review109834 --- LGTM mod some top-level ergonomics. Happy to ship once we resolve

Re: Review Request 41201: Preserve env variables for tasks in docker

> On Dec. 10, 2015, 4:38 a.m., Stephan Erb wrote: > > src/main/python/apache/aurora/executor/thermos_task_runner.py, line 364 > > > > > > Is it really necessary that this is implemented docker specific? I > >

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

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: code version of health check

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41154/#review109847 --- Overall this looks great! Focusing on 2 nits in this pass -

Re: Review Request 30695: Implements log rotation in the Thermos runner.

> On Dec. 10, 2015, 11:12 a.m., Bill Farner wrote: > > docs/deploying-aurora-scheduler.md, line 182 > > > > > > This has diverged from the actual arg in the code. > > George Sirois wrote: > Yeah, see my comment

Re: Review Request 30695: Implements log rotation in the Thermos runner.

> On Dec. 10, 2015, 7:12 p.m., Bill Farner wrote: > > src/main/python/apache/aurora/executor/bin/thermos_executor_main.py, line > > 110 > > > > > > What is the unit? FWIW i'm typically fond when the arg calls out

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

Re: Review Request 41154: code version of health check

> On Dec. 10, 2015, 8:25 p.m., Bill Farner wrote: > > Also, meta - please re-title the review to something that is more readable > > in the commit log than 'code version [...]'. Perhaps 'Add support for > > performing health checks with a shell command.' 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/#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.

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

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.

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

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.

Review Request 41201: Preserve env variables for tasks in docker

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41201/ --- Review request for Aurora. Repository: aurora Description --- Preserve

Re: Review Request 41181: Convert test sources to use lambdas throughout.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41181/#review109803 --- Ship it! Ship It! - Maxim Khutornenko On Dec. 10, 2015, 2:32

Re: Review Request 41226: Handling task event race in updater.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41226/ --- (Updated Dec. 11, 2015, 1:33 a.m.) Review request for Aurora and Bill Farner.

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

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

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.

Re: Review Request 41226: Handling task event race in updater.

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

Re: Review Request 41201: Preserve env variables for tasks in docker

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41201/#review109748 --- src/main/python/apache/aurora/executor/thermos_task_runner.py

Re: Review Request 41201: Preserve env variables for tasks in docker

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41201/#review109749 --- Master (8613f7b) is green with this patch.

Re: Review Request 41201: Preserve env variables for tasks in docker

> On Dec. 10, 2015, 12:38 p.m., Stephan Erb wrote: > > src/main/python/apache/aurora/executor/thermos_task_runner.py, line 364 > > > > > > Is it really necessary that this is implemented docker specific? I > >

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

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.

> 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

Re: Review Request 30695: Implements log rotation in the Thermos runner.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30695/#review109767 --- I've manually tested patch on running mesos/aurora cluster. Mesos