> On Nov. 16, 2016, 7:35 a.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/executor/common/health_checker.py, line 309 > > <https://reviews.apache.org/r/53590/diff/4/?file=1565046#file1565046line309> > > > > nit: it's not for us to judge ;). Drop the "trivially"? (I.e. "No > > health-check defined, task is assumed healthy.")
Done. > On Nov. 16, 2016, 7:35 a.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/executor/aurora_executor.py, line 178 > > <https://reviews.apache.org/r/53590/diff/4/?file=1565044#file1565044line178> > > > > I kind of disagree with this change. The point of adding the callback > > to the `from_assigned_task` signature in the base `StatusCheckerProvider` > > interface was to avoid the need for the `isinstance` checks. > > > > If we're concerned about passing the `signal_running` callback into a > > non-health check related status provider, then we should figure out a way > > to define the callback in an implementation-agnostic way (i.e. add another > > method to the interface that returns the callback). > > > > Alternately, we can just call the callback what it is in the signature, > > document its purpose and drop the `isinstance` check here with the > > assumption that individual status providers are written to only invoke the > > callback as appropriate. > > > > Perhaps the best solution of all is to drop `callback` from the > > interface entirely and replace it w/ `**kwargs`, and then only the > > `HealthCheckerProvider` will pull it out and pay it any attention? As a middle ground between generic interface and readable code, dropping the `isinstance` check here. - Santhosh Kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53590/#review156069 ----------------------------------------------------------- On Nov. 16, 2016, 12:23 a.m., Santhosh Kumar Shanmugham wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53590/ > ----------------------------------------------------------- > > (Updated Nov. 16, 2016, 12:23 a.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/cli/test_inspect.py > 7ef682d010660c9429d94d881dfd42d94f0ce5fd > src/test/python/apache/aurora/client/test_config.py > 5cf68a5145ddf9478baa30453c0bcb73136fa7eb > src/test/python/apache/aurora/executor/common/test_announcer.py > 58ca3a8d1ee248f050bab3d10d465c3260c15df3 > src/test/python/apache/aurora/executor/common/test_health_checker.py > da0c56ca084b65427a6122f22f251af8637772d1 > > src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py > fe74bd1d36666ecd89fca1b5b2251202cbbc0f24 > 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 > >
