Re: Review Request 52094: Add min_consecutive_health_checks in HealthCheckConfig

2016-09-30 Thread Zameer Manji 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52094/#review151078
---


Ship it!




Ship It!

- Zameer Manji


On Sept. 29, 2016, 4:43 p.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52094/
> ---
> 
> (Updated Sept. 29, 2016, 4:43 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Bugs: AURORA-1224
> https://issues.apache.org/jira/browse/AURORA-1224
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add min_consecutive_health_checks to HealthCheckConfig.
> 
> [Summary]
> HealthCheckConfig should accept a new configuration value that will tell how 
> many positive consecutive health checks an instance requires to move from 
> STARTING to RUNNING.
> 
> [Background]
> This review depends on the executor change(AURORA-1225). Please see 
> https://reviews.apache.org/r/51876/ for more details and background.
> 
> [Change List]
> 1. Add a configuration value "min_consecutive_health_checks"(default=1) to 
> HealthCheckConfig struct.
> 2. Modify the default value of watch_secs to be 0.
> 3. Add a client-side constraint: 
> initial_interval_secs >= min_consecutive_health_checks * interval_secs
> 4. Update the unit tests for health check config in client/config.py, skip 
> unit tests related to watch_secs.
> 
> 
> Diffs
> -
> 
>   docs/reference/configuration.md f2a0b1873f31e91f3bf0cac6f8448e8130fae688 
>   src/main/python/apache/aurora/client/config.py 
> 0186af52f0d7d7e3981ec59bf6a01aafee2bcfb1 
>   src/main/python/apache/aurora/config/schema/base.py 
> 845163043b0b7b2f9e7aca14677ca9f094658551 
>   src/test/python/apache/aurora/client/test_config.py 
> 5cf68a5145ddf9478baa30453c0bcb73136fa7eb 
> 
> Diff: https://reviews.apache.org/r/52094/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> ./pants test.pytest src/test/python/apache/aurora/client::
> 
> ./pants test.pytest src/test/python/apache/aurora/config::
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 52094: Add min_consecutive_health_checks in HealthCheckConfig

2016-09-29 Thread Aurora ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52094/#review150970
---


Ship it!




Master (655105d) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Sept. 29, 2016, 11:43 p.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52094/
> ---
> 
> (Updated Sept. 29, 2016, 11:43 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Bugs: AURORA-1224
> https://issues.apache.org/jira/browse/AURORA-1224
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add min_consecutive_health_checks to HealthCheckConfig.
> 
> [Summary]
> HealthCheckConfig should accept a new configuration value that will tell how 
> many positive consecutive health checks an instance requires to move from 
> STARTING to RUNNING.
> 
> [Background]
> This review depends on the executor change(AURORA-1225). Please see 
> https://reviews.apache.org/r/51876/ for more details and background.
> 
> [Change List]
> 1. Add a configuration value "min_consecutive_health_checks"(default=1) to 
> HealthCheckConfig struct.
> 2. Modify the default value of watch_secs to be 0.
> 3. Add a client-side constraint: 
> initial_interval_secs >= min_consecutive_health_checks * interval_secs
> 4. Update the unit tests for health check config in client/config.py, skip 
> unit tests related to watch_secs.
> 
> 
> Diffs
> -
> 
>   docs/reference/configuration.md f2a0b1873f31e91f3bf0cac6f8448e8130fae688 
>   src/main/python/apache/aurora/client/config.py 
> 0186af52f0d7d7e3981ec59bf6a01aafee2bcfb1 
>   src/main/python/apache/aurora/config/schema/base.py 
> 845163043b0b7b2f9e7aca14677ca9f094658551 
>   src/test/python/apache/aurora/client/test_config.py 
> 5cf68a5145ddf9478baa30453c0bcb73136fa7eb 
> 
> Diff: https://reviews.apache.org/r/52094/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> ./pants test.pytest src/test/python/apache/aurora/client::
> 
> ./pants test.pytest src/test/python/apache/aurora/config::
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 52094: Add min_consecutive_health_checks in HealthCheckConfig

2016-09-29 Thread Kai Huang 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52094/
---

(Updated Sept. 29, 2016, 11:43 p.m.)


Review request for Aurora, Joshua Cohen and Zameer Manji.


Changes
---

Add validation for min_consecutive_successes in HealthCheckConfig.


Bugs: AURORA-1224
https://issues.apache.org/jira/browse/AURORA-1224


Repository: aurora


Description
---

Add min_consecutive_health_checks to HealthCheckConfig.

[Summary]
HealthCheckConfig should accept a new configuration value that will tell how 
many positive consecutive health checks an instance requires to move from 
STARTING to RUNNING.

[Background]
This review depends on the executor change(AURORA-1225). Please see 
https://reviews.apache.org/r/51876/ for more details and background.

[Change List]
1. Add a configuration value "min_consecutive_health_checks"(default=1) to 
HealthCheckConfig struct.
2. Modify the default value of watch_secs to be 0.
3. Add a client-side constraint: 
initial_interval_secs >= min_consecutive_health_checks * interval_secs
4. Update the unit tests for health check config in client/config.py, skip unit 
tests related to watch_secs.


Diffs (updated)
-

  docs/reference/configuration.md f2a0b1873f31e91f3bf0cac6f8448e8130fae688 
  src/main/python/apache/aurora/client/config.py 
0186af52f0d7d7e3981ec59bf6a01aafee2bcfb1 
  src/main/python/apache/aurora/config/schema/base.py 
845163043b0b7b2f9e7aca14677ca9f094658551 
  src/test/python/apache/aurora/client/test_config.py 
5cf68a5145ddf9478baa30453c0bcb73136fa7eb 

Diff: https://reviews.apache.org/r/52094/diff/


Testing
---

./build-support/jenkins/build.sh

./pants test.pytest src/test/python/apache/aurora/client::

./pants test.pytest src/test/python/apache/aurora/config::


Thanks,

Kai Huang

Re: Review Request 52094: Add min_consecutive_health_checks in HealthCheckConfig

2016-09-29 Thread Zameer Manji 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52094/#review150942
---



one data valiadation issue, everything else LGTM.


src/main/python/apache/aurora/client/config.py (line 112)


We should validate that this number is positive and non-zero.


- Zameer Manji


On Sept. 28, 2016, 10 a.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52094/
> ---
> 
> (Updated Sept. 28, 2016, 10 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Bugs: AURORA-1224
> https://issues.apache.org/jira/browse/AURORA-1224
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add min_consecutive_health_checks to HealthCheckConfig.
> 
> [Summary]
> HealthCheckConfig should accept a new configuration value that will tell how 
> many positive consecutive health checks an instance requires to move from 
> STARTING to RUNNING.
> 
> [Background]
> This review depends on the executor change(AURORA-1225). Please see 
> https://reviews.apache.org/r/51876/ for more details and background.
> 
> [Change List]
> 1. Add a configuration value "min_consecutive_health_checks"(default=1) to 
> HealthCheckConfig struct.
> 2. Modify the default value of watch_secs to be 0.
> 3. Add a client-side constraint: 
> initial_interval_secs >= min_consecutive_health_checks * interval_secs
> 4. Update the unit tests for health check config in client/config.py, skip 
> unit tests related to watch_secs.
> 
> 
> Diffs
> -
> 
>   docs/reference/configuration.md f2a0b1873f31e91f3bf0cac6f8448e8130fae688 
>   src/main/python/apache/aurora/client/config.py 
> 0186af52f0d7d7e3981ec59bf6a01aafee2bcfb1 
>   src/main/python/apache/aurora/config/schema/base.py 
> 845163043b0b7b2f9e7aca14677ca9f094658551 
>   src/test/python/apache/aurora/client/test_config.py 
> 5cf68a5145ddf9478baa30453c0bcb73136fa7eb 
> 
> Diff: https://reviews.apache.org/r/52094/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> ./pants test.pytest src/test/python/apache/aurora/client::
> 
> ./pants test.pytest src/test/python/apache/aurora/config::
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 52094: Add min_consecutive_health_checks in HealthCheckConfig

2016-09-28 Thread Joshua Cohen 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52094/#review150759
---


Ship it!




Ship It!

- Joshua Cohen


On Sept. 28, 2016, 5 p.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52094/
> ---
> 
> (Updated Sept. 28, 2016, 5 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Bugs: AURORA-1224
> https://issues.apache.org/jira/browse/AURORA-1224
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add min_consecutive_health_checks to HealthCheckConfig.
> 
> [Summary]
> HealthCheckConfig should accept a new configuration value that will tell how 
> many positive consecutive health checks an instance requires to move from 
> STARTING to RUNNING.
> 
> [Background]
> This review depends on the executor change(AURORA-1225). Please see 
> https://reviews.apache.org/r/51876/ for more details and background.
> 
> [Change List]
> 1. Add a configuration value "min_consecutive_health_checks"(default=1) to 
> HealthCheckConfig struct.
> 2. Modify the default value of watch_secs to be 0.
> 3. Add a client-side constraint: 
> initial_interval_secs >= min_consecutive_health_checks * interval_secs
> 4. Update the unit tests for health check config in client/config.py, skip 
> unit tests related to watch_secs.
> 
> 
> Diffs
> -
> 
>   docs/reference/configuration.md f2a0b1873f31e91f3bf0cac6f8448e8130fae688 
>   src/main/python/apache/aurora/client/config.py 
> 0186af52f0d7d7e3981ec59bf6a01aafee2bcfb1 
>   src/main/python/apache/aurora/config/schema/base.py 
> 845163043b0b7b2f9e7aca14677ca9f094658551 
>   src/test/python/apache/aurora/client/test_config.py 
> 5cf68a5145ddf9478baa30453c0bcb73136fa7eb 
> 
> Diff: https://reviews.apache.org/r/52094/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> ./pants test.pytest src/test/python/apache/aurora/client::
> 
> ./pants test.pytest src/test/python/apache/aurora/config::
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 52094: Add min_consecutive_health_checks in HealthCheckConfig

2016-09-27 Thread Aurora ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52094/#review150679
---


Ship it!




Master (69cba78) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Sept. 28, 2016, 4:08 a.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52094/
> ---
> 
> (Updated Sept. 28, 2016, 4:08 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
> 
> 
> Bugs: AURORA-1224
> https://issues.apache.org/jira/browse/AURORA-1224
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add min_consecutive_health_checks to HealthCheckConfig.
> 
> [Summary]
> HealthCheckConfig should accept a new configuration value that will tell how 
> many positive consecutive health checks an instance requires to move from 
> STARTING to RUNNING.
> 
> [Background]
> This review depends on the executor change(AURORA-1225). Please see 
> https://reviews.apache.org/r/51876/ for more details and background.
> 
> [Change List]
> 1. Add a configuration value "min_consecutive_health_checks"(default=1) to 
> HealthCheckConfig struct.
> 2. Modify the default value of watch_secs to be 0.
> 3. Add a client-side constraint: 
> initial_interval_secs >= min_consecutive_health_checks * interval_secs
> 4. Update the unit tests for health check config in client/config.py, skip 
> unit tests related to watch_secs.
> 
> 
> Diffs
> -
> 
>   docs/reference/configuration.md f2a0b1873f31e91f3bf0cac6f8448e8130fae688 
>   src/main/python/apache/aurora/client/config.py 
> 0186af52f0d7d7e3981ec59bf6a01aafee2bcfb1 
>   src/main/python/apache/aurora/config/schema/base.py 
> 845163043b0b7b2f9e7aca14677ca9f094658551 
>   src/test/python/apache/aurora/client/test_config.py 
> 5cf68a5145ddf9478baa30453c0bcb73136fa7eb 
> 
> Diff: https://reviews.apache.org/r/52094/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> ./pants test.pytest src/test/python/apache/aurora/client::
> 
> ./pants test.pytest src/test/python/apache/aurora/config::
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 52094: Add min_consecutive_health_checks in HealthCheckConfig

2016-09-27 Thread Kai Huang 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52094/
---

(Updated Sept. 28, 2016, 4:08 a.m.)


Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.


Changes
---

Removed deprecated unit tests for watch_secs.

Resumed default value of watch_secs.


Bugs: AURORA-1224
https://issues.apache.org/jira/browse/AURORA-1224


Repository: aurora


Description
---

Add min_consecutive_health_checks to HealthCheckConfig.

[Summary]
HealthCheckConfig should accept a new configuration value that will tell how 
many positive consecutive health checks an instance requires to move from 
STARTING to RUNNING.

[Background]
This review depends on the executor change(AURORA-1225). Please see 
https://reviews.apache.org/r/51876/ for more details and background.

[Change List]
1. Add a configuration value "min_consecutive_health_checks"(default=1) to 
HealthCheckConfig struct.
2. Modify the default value of watch_secs to be 0.
3. Add a client-side constraint: 
initial_interval_secs >= min_consecutive_health_checks * interval_secs
4. Update the unit tests for health check config in client/config.py, skip unit 
tests related to watch_secs.


Diffs (updated)
-

  docs/reference/configuration.md f2a0b1873f31e91f3bf0cac6f8448e8130fae688 
  src/main/python/apache/aurora/client/config.py 
0186af52f0d7d7e3981ec59bf6a01aafee2bcfb1 
  src/main/python/apache/aurora/config/schema/base.py 
845163043b0b7b2f9e7aca14677ca9f094658551 
  src/test/python/apache/aurora/client/test_config.py 
5cf68a5145ddf9478baa30453c0bcb73136fa7eb 

Diff: https://reviews.apache.org/r/52094/diff/


Testing
---

./build-support/jenkins/build.sh

./pants test.pytest src/test/python/apache/aurora/client::

./pants test.pytest src/test/python/apache/aurora/config::


Thanks,

Kai Huang

Re: Review Request 52094: Add min_consecutive_health_checks in HealthCheckConfig

2016-09-27 Thread Joshua Cohen 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52094/#review150671
---




docs/reference/configuration.md (line 368)


I thought Zameer's proposal was to allow `watch_secs` to be set to 0, in 
which case users are opting in to the new health check driven updates, but keep 
the default where it currently is.

Is that no longer the case?



src/main/python/apache/aurora/client/config.py (line 101)


nit: "decreate the interval_secs or decrease min_consecutive_successes." 
reads a bit better.



src/test/python/apache/aurora/client/test_config.py (line 195)


Why mark these tests as skipped? If we don't want them to run based on the 
new behavior then just remove them?


- Joshua Cohen


On Sept. 27, 2016, 11:33 p.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52094/
> ---
> 
> (Updated Sept. 27, 2016, 11:33 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
> 
> 
> Bugs: AURORA-1224
> https://issues.apache.org/jira/browse/AURORA-1224
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add min_consecutive_health_checks to HealthCheckConfig.
> 
> [Summary]
> HealthCheckConfig should accept a new configuration value that will tell how 
> many positive consecutive health checks an instance requires to move from 
> STARTING to RUNNING.
> 
> [Background]
> This review depends on the executor change(AURORA-1225). Please see 
> https://reviews.apache.org/r/51876/ for more details and background.
> 
> [Change List]
> 1. Add a configuration value "min_consecutive_health_checks"(default=1) to 
> HealthCheckConfig struct.
> 2. Modify the default value of watch_secs to be 0.
> 3. Add a client-side constraint: 
> initial_interval_secs >= min_consecutive_health_checks * interval_secs
> 4. Update the unit tests for health check config in client/config.py, skip 
> unit tests related to watch_secs.
> 
> 
> Diffs
> -
> 
>   docs/reference/configuration.md f2a0b1873f31e91f3bf0cac6f8448e8130fae688 
>   src/main/python/apache/aurora/client/config.py 
> 0186af52f0d7d7e3981ec59bf6a01aafee2bcfb1 
>   src/main/python/apache/aurora/config/schema/base.py 
> 845163043b0b7b2f9e7aca14677ca9f094658551 
>   src/test/python/apache/aurora/client/test_config.py 
> 5cf68a5145ddf9478baa30453c0bcb73136fa7eb 
> 
> Diff: https://reviews.apache.org/r/52094/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> ./pants test.pytest src/test/python/apache/aurora/client::
> 
> ./pants test.pytest src/test/python/apache/aurora/config::
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 52094: Add min_consecutive_health_checks in HealthCheckConfig

2016-09-27 Thread Aurora ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52094/#review150649
---


Ship it!




Master (69cba78) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Sept. 27, 2016, 11:33 p.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52094/
> ---
> 
> (Updated Sept. 27, 2016, 11:33 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
> 
> 
> Bugs: AURORA-1224
> https://issues.apache.org/jira/browse/AURORA-1224
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add min_consecutive_health_checks to HealthCheckConfig.
> 
> [Summary]
> HealthCheckConfig should accept a new configuration value that will tell how 
> many positive consecutive health checks an instance requires to move from 
> STARTING to RUNNING.
> 
> [Background]
> This review depends on the executor change(AURORA-1225). Please see 
> https://reviews.apache.org/r/51876/ for more details and background.
> 
> [Change List]
> 1. Add a configuration value "min_consecutive_health_checks"(default=1) to 
> HealthCheckConfig struct.
> 2. Modify the default value of watch_secs to be 0.
> 3. Add a client-side constraint: 
> initial_interval_secs >= min_consecutive_health_checks * interval_secs
> 4. Update the unit tests for health check config in client/config.py, skip 
> unit tests related to watch_secs.
> 
> 
> Diffs
> -
> 
>   docs/reference/configuration.md f2a0b1873f31e91f3bf0cac6f8448e8130fae688 
>   src/main/python/apache/aurora/client/config.py 
> 0186af52f0d7d7e3981ec59bf6a01aafee2bcfb1 
>   src/main/python/apache/aurora/config/schema/base.py 
> 845163043b0b7b2f9e7aca14677ca9f094658551 
>   src/test/python/apache/aurora/client/test_config.py 
> 5cf68a5145ddf9478baa30453c0bcb73136fa7eb 
> 
> Diff: https://reviews.apache.org/r/52094/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> ./pants test.pytest src/test/python/apache/aurora/client::
> 
> ./pants test.pytest src/test/python/apache/aurora/config::
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 52094: Add min_consecutive_health_checks in HealthCheckConfig

2016-09-27 Thread Kai Huang 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52094/
---

(Updated Sept. 27, 2016, 11:33 p.m.)


Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.


Changes
---

Renamed "min_consecutive_health_checks" to "min_consecutive_successes", in 
accordance with executor change.

Added documentaton for the HealthCheckConfig change.


Bugs: AURORA-1224
https://issues.apache.org/jira/browse/AURORA-1224


Repository: aurora


Description
---

Add min_consecutive_health_checks to HealthCheckConfig.

[Summary]
HealthCheckConfig should accept a new configuration value that will tell how 
many positive consecutive health checks an instance requires to move from 
STARTING to RUNNING.

[Background]
This review depends on the executor change(AURORA-1225). Please see 
https://reviews.apache.org/r/51876/ for more details and background.

[Change List]
1. Add a configuration value "min_consecutive_health_checks"(default=1) to 
HealthCheckConfig struct.
2. Modify the default value of watch_secs to be 0.
3. Add a client-side constraint: 
initial_interval_secs >= min_consecutive_health_checks * interval_secs
4. Update the unit tests for health check config in client/config.py, skip unit 
tests related to watch_secs.


Diffs (updated)
-

  docs/reference/configuration.md f2a0b1873f31e91f3bf0cac6f8448e8130fae688 
  src/main/python/apache/aurora/client/config.py 
0186af52f0d7d7e3981ec59bf6a01aafee2bcfb1 
  src/main/python/apache/aurora/config/schema/base.py 
845163043b0b7b2f9e7aca14677ca9f094658551 
  src/test/python/apache/aurora/client/test_config.py 
5cf68a5145ddf9478baa30453c0bcb73136fa7eb 

Diff: https://reviews.apache.org/r/52094/diff/


Testing
---

./build-support/jenkins/build.sh

./pants test.pytest src/test/python/apache/aurora/client::

./pants test.pytest src/test/python/apache/aurora/config::


Thanks,

Kai Huang

Re: Review Request 52094: Add min_consecutive_health_checks in HealthCheckConfig

2016-09-27 Thread Kai Huang 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52094/
---

(Updated Sept. 27, 2016, 11:30 p.m.)


Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.


Changes
---

Rename "min_consecutive_health_checks" to "min_consecutive_successes", in 
accordance with executor change. Add documentations for config change.


Bugs: AURORA-1224
https://issues.apache.org/jira/browse/AURORA-1224


Repository: aurora


Description
---

Add min_consecutive_health_checks to HealthCheckConfig.

[Summary]
HealthCheckConfig should accept a new configuration value that will tell how 
many positive consecutive health checks an instance requires to move from 
STARTING to RUNNING.

[Background]
This review depends on the executor change(AURORA-1225). Please see 
https://reviews.apache.org/r/51876/ for more details and background.

[Change List]
1. Add a configuration value "min_consecutive_health_checks"(default=1) to 
HealthCheckConfig struct.
2. Modify the default value of watch_secs to be 0.
3. Add a client-side constraint: 
initial_interval_secs >= min_consecutive_health_checks * interval_secs
4. Update the unit tests for health check config in client/config.py, skip unit 
tests related to watch_secs.


Diffs (updated)
-

  docs/reference/configuration.md f2a0b1873f31e91f3bf0cac6f8448e8130fae688 
  src/main/python/apache/aurora/client/config.py 
0186af52f0d7d7e3981ec59bf6a01aafee2bcfb1 
  src/main/python/apache/aurora/config/schema/base.py 
845163043b0b7b2f9e7aca14677ca9f094658551 
  src/test/python/apache/aurora/client/test_config.py 
5cf68a5145ddf9478baa30453c0bcb73136fa7eb 

Diff: https://reviews.apache.org/r/52094/diff/


Testing
---

./build-support/jenkins/build.sh

./pants test.pytest src/test/python/apache/aurora/client::

./pants test.pytest src/test/python/apache/aurora/config::


Thanks,

Kai Huang

Re: Review Request 52094: Add min_consecutive_health_checks in HealthCheckConfig

2016-09-23 Thread Aurora ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52094/#review150161
---


Ship it!




Master (4ead189) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Sept. 23, 2016, 4:34 a.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52094/
> ---
> 
> (Updated Sept. 23, 2016, 4:34 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
> 
> 
> Bugs: AURORA-1224
> https://issues.apache.org/jira/browse/AURORA-1224
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add min_consecutive_health_checks to HealthCheckConfig.
> 
> [Summary]
> HealthCheckConfig should accept a new configuration value that will tell how 
> many positive consecutive health checks an instance requires to move from 
> STARTING to RUNNING.
> 
> [Background]
> This review depends on the executor change(AURORA-1225). Please see 
> https://reviews.apache.org/r/51876/ for more details and background.
> 
> [Change List]
> 1. Add a configuration value "min_consecutive_health_checks"(default=1) to 
> HealthCheckConfig struct.
> 2. Modify the default value of watch_secs to be 0.
> 3. Add a client-side constraint: 
> initial_interval_secs >= min_consecutive_health_checks * interval_secs
> 4. Update the unit tests for health check config in client/config.py, skip 
> unit tests related to watch_secs.
> 
> 
> Diffs
> -
> 
>   docs/reference/configuration.md f2a0b1873f31e91f3bf0cac6f8448e8130fae688 
>   src/main/python/apache/aurora/client/config.py 
> 0186af52f0d7d7e3981ec59bf6a01aafee2bcfb1 
>   src/main/python/apache/aurora/config/schema/base.py 
> 845163043b0b7b2f9e7aca14677ca9f094658551 
>   src/test/python/apache/aurora/client/test_config.py 
> 5cf68a5145ddf9478baa30453c0bcb73136fa7eb 
> 
> Diff: https://reviews.apache.org/r/52094/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> ./pants test.pytest src/test/python/apache/aurora/client::
> 
> ./pants test.pytest src/test/python/apache/aurora/config::
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 52094: Add min_consecutive_health_checks in HealthCheckConfig

2016-09-22 Thread Kai Huang 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52094/#review150134
---




docs/reference/configuration.md (line 368)


In addition to documentation, we also need to notify the user:
1. For task with health check, they can leave out watch_secs and relies on 
initial_interval_secs.
2. For task without health check, they still have to provide watch_secs to 
cover the warming up period.

I'm thinking that whether we should incorporate this information as a 
client warning message to prevent misuse of the configuration?

I'll wait until Maxim to weigh in. But thanks for your feedback, Dmitriy!


- Kai Huang


On Sept. 23, 2016, 4:34 a.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52094/
> ---
> 
> (Updated Sept. 23, 2016, 4:34 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
> 
> 
> Bugs: AURORA-1224
> https://issues.apache.org/jira/browse/AURORA-1224
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add min_consecutive_health_checks to HealthCheckConfig.
> 
> [Summary]
> HealthCheckConfig should accept a new configuration value that will tell how 
> many positive consecutive health checks an instance requires to move from 
> STARTING to RUNNING.
> 
> [Background]
> This review depends on the executor change(AURORA-1225). Please see 
> https://reviews.apache.org/r/51876/ for more details and background.
> 
> [Change List]
> 1. Add a configuration value "min_consecutive_health_checks"(default=1) to 
> HealthCheckConfig struct.
> 2. Modify the default value of watch_secs to be 0.
> 3. Add a client-side constraint: 
> initial_interval_secs >= min_consecutive_health_checks * interval_secs
> 4. Update the unit tests for health check config in client/config.py, skip 
> unit tests related to watch_secs.
> 
> 
> Diffs
> -
> 
>   docs/reference/configuration.md f2a0b1873f31e91f3bf0cac6f8448e8130fae688 
>   src/main/python/apache/aurora/client/config.py 
> 0186af52f0d7d7e3981ec59bf6a01aafee2bcfb1 
>   src/main/python/apache/aurora/config/schema/base.py 
> 845163043b0b7b2f9e7aca14677ca9f094658551 
>   src/test/python/apache/aurora/client/test_config.py 
> 5cf68a5145ddf9478baa30453c0bcb73136fa7eb 
> 
> Diff: https://reviews.apache.org/r/52094/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> ./pants test.pytest src/test/python/apache/aurora/client::
> 
> ./pants test.pytest src/test/python/apache/aurora/config::
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 52094: Add min_consecutive_health_checks in HealthCheckConfig

2016-09-22 Thread Kai Huang 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52094/
---

(Updated Sept. 23, 2016, 4:34 a.m.)


Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.


Changes
---

Add documentation for min_consecutive_health_checks, update the documentaion of 
watch_secs and initial_interval_secs.


Bugs: AURORA-1224
https://issues.apache.org/jira/browse/AURORA-1224


Repository: aurora


Description
---

Add min_consecutive_health_checks to HealthCheckConfig.

[Summary]
HealthCheckConfig should accept a new configuration value that will tell how 
many positive consecutive health checks an instance requires to move from 
STARTING to RUNNING.

[Background]
This review depends on the executor change(AURORA-1225). Please see 
https://reviews.apache.org/r/51876/ for more details and background.

[Change List]
1. Add a configuration value "min_consecutive_health_checks"(default=1) to 
HealthCheckConfig struct.
2. Modify the default value of watch_secs to be 0.
3. Add a client-side constraint: 
initial_interval_secs >= min_consecutive_health_checks * interval_secs
4. Update the unit tests for health check config in client/config.py, skip unit 
tests related to watch_secs.


Diffs (updated)
-

  docs/reference/configuration.md f2a0b1873f31e91f3bf0cac6f8448e8130fae688 
  src/main/python/apache/aurora/client/config.py 
0186af52f0d7d7e3981ec59bf6a01aafee2bcfb1 
  src/main/python/apache/aurora/config/schema/base.py 
845163043b0b7b2f9e7aca14677ca9f094658551 
  src/test/python/apache/aurora/client/test_config.py 
5cf68a5145ddf9478baa30453c0bcb73136fa7eb 

Diff: https://reviews.apache.org/r/52094/diff/


Testing
---

./build-support/jenkins/build.sh

./pants test.pytest src/test/python/apache/aurora/client::

./pants test.pytest src/test/python/apache/aurora/config::


Thanks,

Kai Huang

Re: Review Request 52094: Add min_consecutive_health_checks in HealthCheckConfig

2016-09-22 Thread Kai Huang 


> On Sept. 23, 2016, 1:16 a.m., Dmitriy Shirchenko wrote:
> > src/main/python/apache/aurora/client/config.py, line 113
> > 
> >
> > why was this configuration option deleted? it is still referenced: 
> > https://github.com/apache/aurora/blob/master/docs/reference/configuration.md#healthcheckconfig-objects
> > 
> > if you are introducing a new option it should be documented.

This configuration option is not deleted. We just won't validate the constraint 
between watch_secs, max_consecutive_failures here.


- Kai


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52094/#review150118
---


On Sept. 20, 2016, 7:43 p.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52094/
> ---
> 
> (Updated Sept. 20, 2016, 7:43 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
> 
> 
> Bugs: AURORA-1224
> https://issues.apache.org/jira/browse/AURORA-1224
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add min_consecutive_health_checks to HealthCheckConfig.
> 
> [Summary]
> HealthCheckConfig should accept a new configuration value that will tell how 
> many positive consecutive health checks an instance requires to move from 
> STARTING to RUNNING.
> 
> [Background]
> This review depends on the executor change(AURORA-1225). Please see 
> https://reviews.apache.org/r/51876/ for more details and background.
> 
> [Change List]
> 1. Add a configuration value "min_consecutive_health_checks"(default=1) to 
> HealthCheckConfig struct.
> 2. Modify the default value of watch_secs to be 0.
> 3. Add a client-side constraint: 
> initial_interval_secs >= min_consecutive_health_checks * interval_secs
> 4. Update the unit tests for health check config in client/config.py, skip 
> unit tests related to watch_secs.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/config.py 
> 0186af52f0d7d7e3981ec59bf6a01aafee2bcfb1 
>   src/main/python/apache/aurora/config/schema/base.py 
> 845163043b0b7b2f9e7aca14677ca9f094658551 
>   src/test/python/apache/aurora/client/test_config.py 
> 5cf68a5145ddf9478baa30453c0bcb73136fa7eb 
> 
> Diff: https://reviews.apache.org/r/52094/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> ./pants test.pytest src/test/python/apache/aurora/client::
> 
> ./pants test.pytest src/test/python/apache/aurora/config::
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 52094: Add min_consecutive_health_checks in HealthCheckConfig

2016-09-22 Thread Dmitriy Shirchenko 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52094/#review150118
---




src/main/python/apache/aurora/client/config.py (line 112)


why was this configuration option deleted? it is still referenced: 
https://github.com/apache/aurora/blob/master/docs/reference/configuration.md#healthcheckconfig-objects

if you are introducing a new option it should be documented.



src/main/python/apache/aurora/config/schema/base.py (line 32)


new default should changed in configuration.md



src/main/python/apache/aurora/config/schema/base.py (line 63)


also needs to be documented.


- Dmitriy Shirchenko


On Sept. 20, 2016, 7:43 p.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52094/
> ---
> 
> (Updated Sept. 20, 2016, 7:43 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
> 
> 
> Bugs: AURORA-1224
> https://issues.apache.org/jira/browse/AURORA-1224
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add min_consecutive_health_checks to HealthCheckConfig.
> 
> [Summary]
> HealthCheckConfig should accept a new configuration value that will tell how 
> many positive consecutive health checks an instance requires to move from 
> STARTING to RUNNING.
> 
> [Background]
> This review depends on the executor change(AURORA-1225). Please see 
> https://reviews.apache.org/r/51876/ for more details and background.
> 
> [Change List]
> 1. Add a configuration value "min_consecutive_health_checks"(default=1) to 
> HealthCheckConfig struct.
> 2. Modify the default value of watch_secs to be 0.
> 3. Add a client-side constraint: 
> initial_interval_secs >= min_consecutive_health_checks * interval_secs
> 4. Update the unit tests for health check config in client/config.py, skip 
> unit tests related to watch_secs.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/config.py 
> 0186af52f0d7d7e3981ec59bf6a01aafee2bcfb1 
>   src/main/python/apache/aurora/config/schema/base.py 
> 845163043b0b7b2f9e7aca14677ca9f094658551 
>   src/test/python/apache/aurora/client/test_config.py 
> 5cf68a5145ddf9478baa30453c0bcb73136fa7eb 
> 
> Diff: https://reviews.apache.org/r/52094/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> ./pants test.pytest src/test/python/apache/aurora/client::
> 
> ./pants test.pytest src/test/python/apache/aurora/config::
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 52094: Add min_consecutive_health_checks in HealthCheckConfig

2016-09-22 Thread Kai Huang 


> On Sept. 20, 2016, 7:59 p.m., Aurora ReviewBot wrote:
> > Master (a9f4e26) is green with this patch.
> >   ./build-support/jenkins/build.sh
> > 
> > I will refresh this build result if you post a review containing 
> > "@ReviewBot retry"

ping for review: Could any one take a look at this review? Conceptually it 
depends on https://reviews.apache.org/r/51876/.
However, the implementation should be independent from executor change.


- Kai


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52094/#review149719
---


On Sept. 20, 2016, 7:43 p.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52094/
> ---
> 
> (Updated Sept. 20, 2016, 7:43 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
> 
> 
> Bugs: AURORA-1224
> https://issues.apache.org/jira/browse/AURORA-1224
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add min_consecutive_health_checks to HealthCheckConfig.
> 
> [Summary]
> HealthCheckConfig should accept a new configuration value that will tell how 
> many positive consecutive health checks an instance requires to move from 
> STARTING to RUNNING.
> 
> [Background]
> This review depends on the executor change(AURORA-1225). Please see 
> https://reviews.apache.org/r/51876/ for more details and background.
> 
> [Change List]
> 1. Add a configuration value "min_consecutive_health_checks"(default=1) to 
> HealthCheckConfig struct.
> 2. Modify the default value of watch_secs to be 0.
> 3. Add a client-side constraint: 
> initial_interval_secs >= min_consecutive_health_checks * interval_secs
> 4. Update the unit tests for health check config in client/config.py, skip 
> unit tests related to watch_secs.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/config.py 
> 0186af52f0d7d7e3981ec59bf6a01aafee2bcfb1 
>   src/main/python/apache/aurora/config/schema/base.py 
> 845163043b0b7b2f9e7aca14677ca9f094658551 
>   src/test/python/apache/aurora/client/test_config.py 
> 5cf68a5145ddf9478baa30453c0bcb73136fa7eb 
> 
> Diff: https://reviews.apache.org/r/52094/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> ./pants test.pytest src/test/python/apache/aurora/client::
> 
> ./pants test.pytest src/test/python/apache/aurora/config::
> 
> 
> Thanks,
> 
> Kai Huang
> 
>