Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-17 Thread Dmitriy Shirchenko
> On Dec. 16, 2015, 10:14 p.m., Joshua Cohen wrote: > > src/test/python/apache/aurora/client/test_config.py, line 187 > > > > > > Kill this comment? It seems superfluous. > > Dmitriy Shirchenko wrote: > I added

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-17 Thread Bill Farner
> On Dec. 17, 2015, 12:31 p.m., Bill Farner wrote: > > Shape of the API LGTM, end-to-end tests pass. I'm going to commit this. Maxim - if you have any remaining issues, i'm happy to address follow-up comments on Dmitriy's behalf - Bill

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-17 Thread Dmitriy Shirchenko
> On Dec. 17, 2015, 1:16 a.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/executor/common/health_checker.py, line 214 > > > > > > Rename this var too? `_end` was never great, but now it's lost all > >

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-17 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41428/#review111034 --- Ship it! Shape of the API LGTM, end-to-end tests pass. - Bill

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-17 Thread Dmitriy Shirchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41428/ --- (Updated Dec. 17, 2015, 7:21 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-16 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41428/#review110727 --- Ship it! Master (fb8155d) is green with this patch.

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-16 Thread Maxim Khutornenko
> On Dec. 16, 2015, 4:22 p.m., Joshua Cohen wrote: > > I'd like to propose an alternate configuration schema for consideration. I > > feel like the version presented in this review is redundant and tries to > > shoehorn the "endpoint" concept onto a shell health checker where it > > doesn't

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-16 Thread John Sirois
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41428/#review110692 --- Ship it! Master (fb8155d) is green with this patch. true I

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-16 Thread Bill Farner
> On Dec. 16, 2015, 8:22 a.m., Joshua Cohen wrote: > > I'd like to propose an alternate configuration schema for consideration. I > > feel like the version presented in this review is redundant and tries to > > shoehorn the "endpoint" concept onto a shell health checker where it > > doesn't

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-16 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41428/#review110716 --- I won't be able to give a review in a reasonable period of time.

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-16 Thread Dmitriy Shirchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41428/ --- (Updated Dec. 16, 2015, 7:08 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-16 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41428/#review110722 --- Ship it! Master (fb8155d) is green with this patch.

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-16 Thread Joshua Cohen
> On Dec. 16, 2015, 4:22 p.m., Joshua Cohen wrote: > > I'd like to propose an alternate configuration schema for consideration. I > > feel like the version presented in this review is redundant and tries to > > shoehorn the "endpoint" concept onto a shell health checker where it > > doesn't

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-16 Thread Dmitriy Shirchenko
> On Dec. 16, 2015, 4:22 p.m., Joshua Cohen wrote: > > I'd like to propose an alternate configuration schema for consideration. I > > feel like the version presented in this review is redundant and tries to > > shoehorn the "endpoint" concept onto a shell health checker where it > > doesn't

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-16 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41428/#review110741 --- src/main/python/apache/aurora/client/config.py (line 103)

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-16 Thread Dmitriy Shirchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41428/ --- (Updated Dec. 16, 2015, 10:43 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-16 Thread Dmitriy Shirchenko
> On Dec. 16, 2015, 10:14 p.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/client/config.py, lines 111-113 > > > > > > Nit, could probably inline all of this: > > > > if not

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-16 Thread Joshua Cohen
> On Dec. 16, 2015, 10:14 p.m., Joshua Cohen wrote: > > src/test/python/apache/aurora/client/test_config.py, line 187 > > > > > > Kill this comment? It seems superfluous. > > Dmitriy Shirchenko wrote: > I added

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-16 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41428/#review110828 --- src/main/python/apache/aurora/executor/common/health_checker.py

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-16 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41428/#review110672 --- I'd like to propose an alternate configuration schema for

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-15 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41428/#review110612 --- Looks good overall! A few comments below.

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-15 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41428/#review110598 --- Master (fb8155d) is red with this patch.

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-15 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41428/#review110603 --- Ship it! Master (fb8155d) is green with this patch.

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-15 Thread Dmitriy Shirchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41428/ --- (Updated Dec. 16, 2015, 2:13 a.m.) Review request for Aurora, Maxim

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-15 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41428/#review110635 --- Ship it! Master (fb8155d) is green with this patch.