> On Nov. 15, 2016, 9:23 a.m., Joshua Cohen wrote:
> > docs/features/job-updates.md, lines 52-53
> > <https://reviews.apache.org/r/53590/diff/2/?file=1562696#file1562696line52>
> >
> >     I know this is not your doing, but since you're in here... this part is 
> > not really accurate. The update is aborted based on the criteria specified 
> > in the UpdateConfig, not just some blanket percentage of health checks 
> > having failed. Mind updating?

Done.


> On Nov. 15, 2016, 9:23 a.m., Joshua Cohen wrote:
> > docs/features/services.md, lines 93-94
> > <https://reviews.apache.org/r/53590/diff/2/?file=1562697#file1562697line93>
> >
> >     "by introducing a `min_consecutive_successess` parameter .... This 
> > parameter represents the number of ..."

Done.


> On Nov. 15, 2016, 9:23 a.m., Joshua Cohen wrote:
> > docs/features/services.md, lines 95-96
> > <https://reviews.apache.org/r/53590/diff/2/?file=1562697#file1562697line95>
> >
> >     "Tasks that do not have enough successful health checks..."

Done.


> On Nov. 15, 2016, 9:23 a.m., Joshua Cohen wrote:
> > docs/features/services.md, line 102
> > <https://reviews.apache.org/r/53590/diff/2/?file=1562697#file1562697line102>
> >
> >     "...worst-case update time, it can instead be set to 0."

Done.


> On Nov. 15, 2016, 9:23 a.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/client/config.py, line 123
> > <https://reviews.apache.org/r/53590/diff/2/?file=1562701#file1562701line123>
> >
> >     given that the values being iterated are tuples and not key value 
> > pairs, probably better to explicitly name these vars. Something like `for 
> > value, name in params`?

Done.


> On Nov. 15, 2016, 9:23 a.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/executor/aurora_executor.py, lines 175-177
> > <https://reviews.apache.org/r/53590/diff/2/?file=1562703#file1562703line175>
> >
> >     indentation should be 4 spaces

Done. (I really need to find some style-checker)


> On Nov. 15, 2016, 9:23 a.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/executor/common/health_checker.py, line 181
> > <https://reviews.apache.org/r/53590/diff/2/?file=1562705#file1562705line181>
> >
> >     add a space after `attempt:`

Done.


> On Nov. 15, 2016, 9:23 a.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/executor/common/health_checker.py, line 202
> > <https://reviews.apache.org/r/53590/diff/2/?file=1562705#file1562705line202>
> >
> >     I can't think of any good reason not to keep incrementing attempts 
> > (e.g. numbers in Python don't overflow, memory utilization for even an 
> > exorbitantly large number is still trivial), but it stood out as a 
> > potential problem vector, so wanted to raise the question in case there's 
> > anything I'm missing: for a long lived service, is there any downside to 
> > continuing to increment attempts after the callback has been dispatched?

I opted this to avoid one extra conditional (since numbers do not overflow in 
python). But its probably not worth it, since every access to `attempts` is 
already gated. Changed it now.


- Santhosh Kumar


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


On Nov. 13, 2016, 9:09 p.m., Santhosh Kumar Shanmugham wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53590/
> -----------------------------------------------------------
> 
> (Updated Nov. 13, 2016, 9:09 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1225
>     https://issues.apache.org/jira/browse/AURORA-1225
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Make RUNNING a first class state to indicate that the task is running
> and is healthy. It is achieved by introducing a new configuration
> parameter `min_consecutive_successes`, which will dictate when to move
> a task into RUNNING state.
> 
> With this change, it is possible to set the `watch_secs` to 0, so that
> updates are purely based on the task's health, rather than relying on
> watching the task to in RUNNING state for a pre-determined timeout.
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md 392496284b7f046a5dd29a5a3f175305c28a5afe 
>   docs/development/design-documents.md 
> 6bfc679c841ef1b0861758ccc12a5150f1bdb5e3 
>   docs/features/job-updates.md 792f2ae5fd14b1ea9af8be000629ce5a7fc2fe8f 
>   docs/features/services.md b6bfc9de550a714b4243cf033e9f3f03ae676af2 
>   docs/reference/configuration.md f2a0b1873f31e91f3bf0cac6f8448e8130fae688 
>   docs/reference/task-lifecycle.md 4dcb48149e156ef1927e63f5a9fa6b53b174e755 
>   src/main/python/apache/aurora/client/api/updater_util.py 
> c649316edb876565c92cc90c9f030e153c008924 
>   src/main/python/apache/aurora/client/config.py 
> 0186af52f0d7d7e3981ec59bf6a01aafee2bcfb1 
>   src/main/python/apache/aurora/config/schema/base.py 
> 845163043b0b7b2f9e7aca14677ca9f094658551 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> aee5e56ae66ec7a6acbe97d94d6187fd8646ec9a 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 12d8e0af95d4c24c1b7a3b07d01b59cec4a15894 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 3c7c09d173c4adbd89eddaee22cffcdd8d268378 
>   src/main/python/apache/aurora/executor/common/resource_manager.py 
> b7dc40d8973ec2e5998ab4f6ff988051a70bb1ab 
>   src/main/python/apache/aurora/executor/common/status_checker.py 
> 795dae2d6b661fc528d952c2315196d94127961f 
>   src/test/python/apache/aurora/client/test_config.py 
> 5cf68a5145ddf9478baa30453c0bcb73136fa7eb 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> da0c56ca084b65427a6122f22f251af8637772d1 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> 3f82165333665d127e1ace765a7dada7348a4b91 
>   src/test/sh/org/apache/aurora/e2e/http/http_example.aurora 
> c9dae28b83d751cfdc7c09dc2d83ac3ee4852720 
>   src/test/sh/org/apache/aurora/e2e/http/http_example_bad_healthcheck.aurora 
> b85ace4bc33bda9483045732024302eb0725c89f 
>   src/test/sh/org/apache/aurora/e2e/http/http_example_updated.aurora 
> b3caa4186217dbf17282a5780791ce3157f4389b 
>   src/test/sh/org/apache/aurora/e2e/http_example.py 
> 675ece85e2cf8e45f2c6e079e39b0e095075a3fa 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> 9cc6cec6633ee5973d8c9faa98ae24ab10cbeca4 
> 
> Diff: https://reviews.apache.org/r/53590/diff/
> 
> 
> Testing
> -------
> 
> buils-support/jenkins/build.sh
> sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>

Reply via email to