Re: Review Request 26328: Improve handling of unknown errors in the aurora client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26328/ --- (Updated Oct. 8, 2014, 10:42 a.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Changes --- Rebase. Bugs: aurora-779 https://issues.apache.org/jira/browse/aurora-779 Repository: aurora Description --- Improve handling of unknown errors in the aurora client. Instead of dumping the stack on the user's terminal, or absorbing the error and generating a brief error message, the client now writes detailed information about the error is written into an error log file, and the user is given a clean error message referring them to that file for details. Diffs (updated) - src/main/python/apache/aurora/client/cli/__init__.py e0c3050151bca1128ed7e476ec5133407a20f6c2 src/main/python/apache/aurora/client/cli/standalone_client.py 7c0975ce9d415b19b6704b9a772ee2619ac9a2af src/test/python/apache/aurora/client/cli/test_api_from_cli.py 78f21d2f20cf71fa2dfe0614885d44d2948decd2 src/test/python/apache/aurora/client/cli/test_create.py 6e55188bdfc576506848605debb391288e696fe3 Diff: https://reviews.apache.org/r/26328/diff/ Testing --- New unit test. Thanks, Mark Chu-Carroll
Re: Review Request 26328: Improve handling of unknown errors in the aurora client.
On Oct. 8, 2014, 1:58 p.m., Mark Chu-Carroll wrote: src/main/python/apache/aurora/client/cli/__init__.py, line 435 https://reviews.apache.org/r/26328/diff/2/?file=714194#file714194line435 It writes into the current directory where the user is executing the client. This is going to lead to complaints. On the dev list, John wrote: The full backtrace goes off to a file in the user's home dir somewhere and then you can ask them to run a command passing the pill ref to get the full error report without worry of re-running some non-idempotent command, etc. So I really think we should write them to a consistent and predictable location. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26328/#review55794 --- On Oct. 8, 2014, 2:42 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26328/ --- (Updated Oct. 8, 2014, 2:42 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-779 https://issues.apache.org/jira/browse/aurora-779 Repository: aurora Description --- Improve handling of unknown errors in the aurora client. Instead of dumping the stack on the user's terminal, or absorbing the error and generating a brief error message, the client now writes detailed information about the error is written into an error log file, and the user is given a clean error message referring them to that file for details. Diffs - src/main/python/apache/aurora/client/cli/__init__.py e0c3050151bca1128ed7e476ec5133407a20f6c2 src/main/python/apache/aurora/client/cli/standalone_client.py 7c0975ce9d415b19b6704b9a772ee2619ac9a2af src/test/python/apache/aurora/client/cli/test_api_from_cli.py 78f21d2f20cf71fa2dfe0614885d44d2948decd2 src/test/python/apache/aurora/client/cli/test_create.py 6e55188bdfc576506848605debb391288e696fe3 Diff: https://reviews.apache.org/r/26328/diff/ Testing --- New unit test. Thanks, Mark Chu-Carroll
Re: Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.
On Oct. 7, 2014, 9:39 p.m., Mark Chu-Carroll wrote: Looks good. One note on the change description: I'm willing to bet that there is a way to get that branch active in a test. Every single time that I've ever said that something couldn't be tested, or that some branch couldn't be exercised in a test, I've always been wrong. There's a way. Poor wording on my part. I meant to say that it was not possible to enter those branches before this diff, meaning we would continue to loop when an exception was caught. With this diff, those branches are now entered in pracice and in the test cases changed. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26308/#review55716 --- On Oct. 4, 2014, 5:55 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26308/ --- (Updated Oct. 4, 2014, 5:55 p.m.) Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll. Repository: aurora Description --- Fixes two problems: - mocking was incorrect in `test_status_api_failure`. Turns out that Mock objects were being passed around and somehow resulted in the test case passing. - use of `threading.Event()` was broken in scheduler_client.py. I don't think it's possible to enter those branches. Diffs - src/main/python/apache/aurora/client/api/__init__.py 26300792594e4005dacc139a9f89711b8a66ab61 src/main/python/apache/aurora/client/api/command_runner.py a1fed5fc75dde3a79c840515e6daa4741156ef97 src/main/python/apache/aurora/client/api/job_monitor.py 18d5c0381d43fc7b24bae4b2e5e6fdc774a74b52 src/main/python/apache/aurora/client/api/scheduler_client.py 311c954f1db245b75192d00c6aca0721085fbf32 src/main/python/apache/aurora/client/api/updater.py bf608981c2f2e7960b68c3fbda144277a59a3d40 src/main/python/apache/aurora/common/aurora_job_key.py a7ca7b6df6b9566b3ce617283ccac948deb2eb83 src/test/python/apache/aurora/client/api/test_job_monitor.py 5b26539f86a0a82f72753a803a769eda77cbc332 src/test/python/apache/aurora/client/api/test_scheduler_client.py 1835843f1795b0530874ec561582df17acfbce65 src/test/python/apache/aurora/client/api/test_updater.py 6905831b23a84320e7f41843efd62b86da366c0b src/test/python/apache/aurora/client/cli/test_api_from_cli.py 78f21d2f20cf71fa2dfe0614885d44d2948decd2 src/test/python/apache/aurora/client/cli/test_cancel_update.py c15e142930c9474c7873dd931261b6ab4eb5967f src/test/python/apache/aurora/client/cli/test_command_hooks.py 3acd2ba0d8bd8c71d4c0a9d71a035fc974fa20c3 src/test/python/apache/aurora/client/cli/test_create.py 6e55188bdfc576506848605debb391288e696fe3 src/test/python/apache/aurora/client/cli/test_diff.py e1a6f764830e06c73d0bc10a3b5da67219da836c src/test/python/apache/aurora/client/cli/test_kill.py e3a366bf67074e50787394cad58d5e01359b641e src/test/python/apache/aurora/client/cli/test_logging.py 6285fbb07442291c2dc4096e68eb285c98994097 src/test/python/apache/aurora/client/cli/test_plugins.py 7a0a31818cbc57de952d7817f8e7c8fa1e84b25a src/test/python/apache/aurora/client/cli/test_status.py bd30b11022ee0d1fb38da9f6efd4d9c5923b7d13 src/test/python/apache/aurora/client/cli/test_task_run.py 1ce9a632874e818eee71573cd481842affae3615 src/test/python/apache/aurora/client/cli/test_update.py 85b1db19d89967a741bfba7964eeb368426f0b61 src/test/python/apache/aurora/client/commands/test_admin.py 1192556c027dc3adf16bb37adeac7798cf9ef93d src/test/python/apache/aurora/client/commands/test_cancel_update.py 5f05ef7c0643d189de3de38c75aae58c2a3814a4 src/test/python/apache/aurora/client/commands/test_create.py 7503345ea1c0f224a894ce02cc2c2d8719574e32 src/test/python/apache/aurora/client/commands/test_diff.py 8f5da7d2bca9b0486b635afe49d3885151624e12 src/test/python/apache/aurora/client/commands/test_hooks.py 0861f13b13a8406950ba953efba0ffae186a8253 src/test/python/apache/aurora/client/commands/test_kill.py c0a6fd44c5691cde50746ffdec325bb11a33469a src/test/python/apache/aurora/client/commands/test_run.py e97b5156609f80a4170028e780bcd891e56983ff src/test/python/apache/aurora/client/commands/test_ssh.py c5ca66e378bfc97c40a406a758ae4dfaef8ab2c8 src/test/python/apache/aurora/client/commands/test_status.py bda1f28d544ae48428129f8167d8632ef27f5fba src/test/python/apache/aurora/client/commands/test_update.py af2cbc7f88287201a472ba36902b00d90bc77d3b Diff: https://reviews.apache.org/r/26308/diff/ Testing --- ./pants src/test/python:all -vxs Thanks, Bill Farner
Re: Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.
On Oct. 7, 2014, 9:18 p.m., Kevin Sweeney wrote: src/test/python/apache/aurora/client/cli/test_api_from_cli.py, line 51 https://reviews.apache.org/r/26308/diff/5/?file=713862#file713862line51 Why do these need to be mocks? This would work identically using the Thrift structures directly. If you want to prevent setting a field that doesn't exist you can use the constructor kwargs form of the thrift object, i.e. ```py task = ScheduledTask( key=JobKey(role=..., ...), assignedTask=AssignedTask( slaveHost=..., ... ), ... ) ``` I thought the same way when i came to this code, but had already pulled a long thread with this diff and didn't want to continue further for the sake of reviewer sanity. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26308/#review55712 --- On Oct. 4, 2014, 5:55 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26308/ --- (Updated Oct. 4, 2014, 5:55 p.m.) Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll. Repository: aurora Description --- Fixes two problems: - mocking was incorrect in `test_status_api_failure`. Turns out that Mock objects were being passed around and somehow resulted in the test case passing. - use of `threading.Event()` was broken in scheduler_client.py. I don't think it's possible to enter those branches. Diffs - src/main/python/apache/aurora/client/api/__init__.py 26300792594e4005dacc139a9f89711b8a66ab61 src/main/python/apache/aurora/client/api/command_runner.py a1fed5fc75dde3a79c840515e6daa4741156ef97 src/main/python/apache/aurora/client/api/job_monitor.py 18d5c0381d43fc7b24bae4b2e5e6fdc774a74b52 src/main/python/apache/aurora/client/api/scheduler_client.py 311c954f1db245b75192d00c6aca0721085fbf32 src/main/python/apache/aurora/client/api/updater.py bf608981c2f2e7960b68c3fbda144277a59a3d40 src/main/python/apache/aurora/common/aurora_job_key.py a7ca7b6df6b9566b3ce617283ccac948deb2eb83 src/test/python/apache/aurora/client/api/test_job_monitor.py 5b26539f86a0a82f72753a803a769eda77cbc332 src/test/python/apache/aurora/client/api/test_scheduler_client.py 1835843f1795b0530874ec561582df17acfbce65 src/test/python/apache/aurora/client/api/test_updater.py 6905831b23a84320e7f41843efd62b86da366c0b src/test/python/apache/aurora/client/cli/test_api_from_cli.py 78f21d2f20cf71fa2dfe0614885d44d2948decd2 src/test/python/apache/aurora/client/cli/test_cancel_update.py c15e142930c9474c7873dd931261b6ab4eb5967f src/test/python/apache/aurora/client/cli/test_command_hooks.py 3acd2ba0d8bd8c71d4c0a9d71a035fc974fa20c3 src/test/python/apache/aurora/client/cli/test_create.py 6e55188bdfc576506848605debb391288e696fe3 src/test/python/apache/aurora/client/cli/test_diff.py e1a6f764830e06c73d0bc10a3b5da67219da836c src/test/python/apache/aurora/client/cli/test_kill.py e3a366bf67074e50787394cad58d5e01359b641e src/test/python/apache/aurora/client/cli/test_logging.py 6285fbb07442291c2dc4096e68eb285c98994097 src/test/python/apache/aurora/client/cli/test_plugins.py 7a0a31818cbc57de952d7817f8e7c8fa1e84b25a src/test/python/apache/aurora/client/cli/test_status.py bd30b11022ee0d1fb38da9f6efd4d9c5923b7d13 src/test/python/apache/aurora/client/cli/test_task_run.py 1ce9a632874e818eee71573cd481842affae3615 src/test/python/apache/aurora/client/cli/test_update.py 85b1db19d89967a741bfba7964eeb368426f0b61 src/test/python/apache/aurora/client/commands/test_admin.py 1192556c027dc3adf16bb37adeac7798cf9ef93d src/test/python/apache/aurora/client/commands/test_cancel_update.py 5f05ef7c0643d189de3de38c75aae58c2a3814a4 src/test/python/apache/aurora/client/commands/test_create.py 7503345ea1c0f224a894ce02cc2c2d8719574e32 src/test/python/apache/aurora/client/commands/test_diff.py 8f5da7d2bca9b0486b635afe49d3885151624e12 src/test/python/apache/aurora/client/commands/test_hooks.py 0861f13b13a8406950ba953efba0ffae186a8253 src/test/python/apache/aurora/client/commands/test_kill.py c0a6fd44c5691cde50746ffdec325bb11a33469a src/test/python/apache/aurora/client/commands/test_run.py e97b5156609f80a4170028e780bcd891e56983ff src/test/python/apache/aurora/client/commands/test_ssh.py c5ca66e378bfc97c40a406a758ae4dfaef8ab2c8 src/test/python/apache/aurora/client/commands/test_status.py bda1f28d544ae48428129f8167d8632ef27f5fba src/test/python/apache/aurora/client/commands/test_update.py af2cbc7f88287201a472ba36902b00d90bc77d3b Diff:
Review Request 26445: Fix error in incorrect deprecation warning on v1 ssh command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26445/ --- Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-804 https://issues.apache.org/jira/browse/aurora-804 Repository: aurora Description --- The error message incorrectly displayed a list-value for the --tunnels parameter, when in fact it should expand the list into multiple instances of --tunnels. Diffs - src/main/python/apache/aurora/client/commands/ssh.py 9ee94a0bdfd8bbc030ae978e2ac3fe39571b4ce2 src/test/python/apache/aurora/client/commands/test_ssh.py c5ca66e378bfc97c40a406a758ae4dfaef8ab2c8 Diff: https://reviews.apache.org/r/26445/diff/ Testing --- New unit test. Thanks, Mark Chu-Carroll
Re: Review Request 26424: Disable requests http connection logging.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26424/#review55805 --- src/main/python/apache/aurora/common/transport.py https://reviews.apache.org/r/26424/#comment96183 I find it odd to extract a function for this. If i were a newcomer, i would immediately inline this function for a 3-1 line code savings. If you feel strongly about extracting a function, a comment is needed to explain the thought process. - Bill Farner On Oct. 7, 2014, 8:32 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26424/ --- (Updated Oct. 7, 2014, 8:32 p.m.) Review request for Aurora, Joe Smith and Bill Farner. Bugs: AURORA-770 https://issues.apache.org/jira/browse/AURORA-770 Repository: aurora Description --- Disable requests http connection logging. Diffs - src/main/python/apache/aurora/common/transport.py 6f7c355d725b5e537cc4ae471170eaa8431da326 src/test/python/apache/aurora/common/test_transport.py c722eae2d04dec90e9c772f49c578184a2bdf76c Diff: https://reviews.apache.org/r/26424/diff/ Testing --- Added hokey test, ran hokey test. Also verified the lack of http connection logs when running client commands directly. Thanks, Joshua Cohen
Re: Review Request 26432: Reject new GC tasks when shutting down
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26432/#review55810 --- Ship it! Ship It! - Bill Farner On Oct. 8, 2014, 12:14 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26432/ --- (Updated Oct. 8, 2014, 12:14 a.m.) Review request for Aurora and Brian Wickman. Bugs: AURORA-807 https://issues.apache.org/jira/browse/AURORA-807 Repository: aurora Description --- Reject new GC tasks when shutting down Diffs - src/main/python/apache/aurora/executor/gc_executor.py 80ce6dc93e17f01c95a84ee8bef2ce87cafa88c0 src/test/python/apache/aurora/executor/test_gc_executor.py d90dc68ec1526110a484cfc2b6c4658abdc2e871 Diff: https://reviews.apache.org/r/26432/diff/ Testing --- ./pants src/test/python/apache/aurora/executor:gc_executor Thanks, Kevin Sweeney
Re: Review Request 26424: Disable requests http connection logging.
On Oct. 8, 2014, 4:28 p.m., Bill Farner wrote: src/main/python/apache/aurora/common/transport.py, line 28 https://reviews.apache.org/r/26424/diff/2/?file=714839#file714839line28 I find it odd to extract a function for this. If i were a newcomer, i would immediately inline this function for a 3-1 line code savings. If you feel strongly about extracting a function, a comment is needed to explain the thought process. Joshua Cohen wrote: My thought process was, it's already weird for initializing the transport to have a side effect of globally changing the log level for requests, this way if something needs to change requests logging without instantiating a transport, they could just invoke this function. Does that sound reasonable enough to keep the function? If so, I can add a comment, if not I can inline. To me, the otherwise-unused function only adds to the weirdness of it all :-) I say YAGNI - inline, add a comment that we don't like doing this, but want quiet output. Side note: is there no other wsy to do this? It's surprising that this ia a global setting in the library. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26424/#review55805 --- On Oct. 7, 2014, 8:32 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26424/ --- (Updated Oct. 7, 2014, 8:32 p.m.) Review request for Aurora, Joe Smith and Bill Farner. Bugs: AURORA-770 https://issues.apache.org/jira/browse/AURORA-770 Repository: aurora Description --- Disable requests http connection logging. Diffs - src/main/python/apache/aurora/common/transport.py 6f7c355d725b5e537cc4ae471170eaa8431da326 src/test/python/apache/aurora/common/test_transport.py c722eae2d04dec90e9c772f49c578184a2bdf76c Diff: https://reviews.apache.org/r/26424/diff/ Testing --- Added hokey test, ran hokey test. Also verified the lack of http connection logs when running client commands directly. Thanks, Joshua Cohen
Re: Review Request 26383: Health Check Disabler
On Oct. 6, 2014, 10:55 p.m., Joshua Cohen wrote: src/main/python/apache/aurora/config/schema/base.py, line 69 https://reviews.apache.org/r/26383/diff/1/?file=714259#file714259line69 Do we need to make this path configurable? I'm trying to think of a reason that someone might want to change it, and I'm coming up blank, however I could envision a scenario where someone does something malicious (intentionally or not) like sets this path to something that shouldn't be removed (e.g. the artifact being run). +1, knob uneeded - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26383/#review55590 --- On Oct. 6, 2014, 9:24 p.m., David Pan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26383/ --- (Updated Oct. 6, 2014, 9:24 p.m.) Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji. Repository: aurora Description --- The health check disabler allows health checks for a job to be snoozed temporarily by touching a snooze file in the job's sandbox. The path of the snooze file and the snooze duration can be set in the HealthCheckConfig. The appropriate unit tests were modified/added. The corresponding JIRA ticket is the following: https://issues.apache.org/jira/browse/AURORA-795 Diffs - docs/configuration-reference.md 5166d45ddf95ae5d8afe39dd3b00654ac91857ec docs/configuration-tutorial.md 67998e9dab6ac429d96d7c0d2df959336b767f32 src/main/python/apache/aurora/config/schema/base.py f12634f103c3eb20e43f37c25d9b0fc3e3d228ec src/main/python/apache/aurora/executor/common/health_checker.py 4980411c847d12655cbb363404707ebd9f0bd163 src/test/python/apache/aurora/executor/common/BUILD c7f7a003c865d479ba6e3cd7b5349322f884f653 src/test/python/apache/aurora/executor/common/test_health_checker.py aa36415fa891fc523a3a376ffeca5d3cd5ceabec Diff: https://reviews.apache.org/r/26383/diff/ Testing --- On vagrant in ~/aurora, I ran ./pants src/test/python/apache/aurora/executor:: Thanks, David Pan
Re: Review Request 26328: Improve handling of unknown errors in the aurora client.
On Oct. 8, 2014, 1:05 a.m., Maxim Khutornenko wrote: I see this got committed even though there are still open questions in this RB. Any chance these can get answered? - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26328/#review55752 --- On Oct. 8, 2014, 2:42 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26328/ --- (Updated Oct. 8, 2014, 2:42 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-779 https://issues.apache.org/jira/browse/aurora-779 Repository: aurora Description --- Improve handling of unknown errors in the aurora client. Instead of dumping the stack on the user's terminal, or absorbing the error and generating a brief error message, the client now writes detailed information about the error is written into an error log file, and the user is given a clean error message referring them to that file for details. Diffs - src/main/python/apache/aurora/client/cli/__init__.py e0c3050151bca1128ed7e476ec5133407a20f6c2 src/main/python/apache/aurora/client/cli/standalone_client.py 7c0975ce9d415b19b6704b9a772ee2619ac9a2af src/test/python/apache/aurora/client/cli/test_api_from_cli.py 78f21d2f20cf71fa2dfe0614885d44d2948decd2 src/test/python/apache/aurora/client/cli/test_create.py 6e55188bdfc576506848605debb391288e696fe3 Diff: https://reviews.apache.org/r/26328/diff/ Testing --- New unit test. Thanks, Mark Chu-Carroll
Re: Review Request 26383: Health Check Disabler
On Oct. 6, 2014, 10:40 p.m., Brian Wickman wrote: docs/configuration-reference.md, lines 359-360 https://reviews.apache.org/r/26383/diff/1/?file=714257#file714257line359 Is there any reason this needs to be configurable? Why not just hardcode the filename as '.healthchecksnooze' and then allow the user to specify the snooze at runtime, e.g. echo '600' .healthchecksnooze to sleep for 600 seconds. (And if the value is malformed, just unlink and don't snooze.) David Pan wrote: A few questions: 1. In the case if the value is malformed, do we want to force fail the health check in order to alert the user the fact that they failed at setting the snooze duration. Otherwise, the user might not notice the mistake. 2. Should we read the value from the file only the first time the snooze file is created. Or, should we allow the user change the value on the fly. Brian Wickman wrote: 1. I think force failing the health check would be counterproductive -- if you accidentally fat finger, you don't want to kill the task which might already be a unicorn. Instead just unlinking right away or perhaps using a default snooze value. 2. Maybe do mtime + time in the file as the snooze expiration, and re-read if the mtime changed. This means at least doing a stat() each health check interval, but will allow you to change the snooze on the fly. How about removing the time control altogether, and let presence of the file serve as the snooze? It's easier to add the time control later than to remove it if we decide it's unneeded. This allows us to sidestep the malformed file content discussion. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26383/#review55592 --- On Oct. 6, 2014, 9:24 p.m., David Pan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26383/ --- (Updated Oct. 6, 2014, 9:24 p.m.) Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji. Repository: aurora Description --- The health check disabler allows health checks for a job to be snoozed temporarily by touching a snooze file in the job's sandbox. The path of the snooze file and the snooze duration can be set in the HealthCheckConfig. The appropriate unit tests were modified/added. The corresponding JIRA ticket is the following: https://issues.apache.org/jira/browse/AURORA-795 Diffs - docs/configuration-reference.md 5166d45ddf95ae5d8afe39dd3b00654ac91857ec docs/configuration-tutorial.md 67998e9dab6ac429d96d7c0d2df959336b767f32 src/main/python/apache/aurora/config/schema/base.py f12634f103c3eb20e43f37c25d9b0fc3e3d228ec src/main/python/apache/aurora/executor/common/health_checker.py 4980411c847d12655cbb363404707ebd9f0bd163 src/test/python/apache/aurora/executor/common/BUILD c7f7a003c865d479ba6e3cd7b5349322f884f653 src/test/python/apache/aurora/executor/common/test_health_checker.py aa36415fa891fc523a3a376ffeca5d3cd5ceabec Diff: https://reviews.apache.org/r/26383/diff/ Testing --- On vagrant in ~/aurora, I ran ./pants src/test/python/apache/aurora/executor:: Thanks, David Pan
Re: Review Request 26424: Disable requests http connection logging.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26424/#review55818 --- Ship it! Ship It! - Bill Farner On Oct. 8, 2014, 4:54 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26424/ --- (Updated Oct. 8, 2014, 4:54 p.m.) Review request for Aurora, Joe Smith and Bill Farner. Bugs: AURORA-770 https://issues.apache.org/jira/browse/AURORA-770 Repository: aurora Description --- . Diffs - src/main/python/apache/aurora/common/transport.py 6f7c355d725b5e537cc4ae471170eaa8431da326 src/test/python/apache/aurora/common/test_transport.py c722eae2d04dec90e9c772f49c578184a2bdf76c Diff: https://reviews.apache.org/r/26424/diff/ Testing --- Added hokey test, ran hokey test. Also verified the lack of http connection logs when running client commands directly. Thanks, Joshua Cohen
Re: Review Request 26383: Health Check Disabler
I'm +1 on removing the time control as well. If you need to extend the snooze you could always touch -m the snooze file? On Wed, Oct 8, 2014 at 9:51 AM, Bill Farner wfar...@apache.org wrote: On Oct. 6, 2014, 10:40 p.m., Brian Wickman wrote: docs/configuration-reference.md, lines 359-360 https://reviews.apache.org/r/26383/diff/1/?file=714257#file714257line359 Is there any reason this needs to be configurable? Why not just hardcode the filename as '.healthchecksnooze' and then allow the user to specify the snooze at runtime, e.g. echo '600' .healthchecksnooze to sleep for 600 seconds. (And if the value is malformed, just unlink and don't snooze.) David Pan wrote: A few questions: 1. In the case if the value is malformed, do we want to force fail the health check in order to alert the user the fact that they failed at setting the snooze duration. Otherwise, the user might not notice the mistake. 2. Should we read the value from the file only the first time the snooze file is created. Or, should we allow the user change the value on the fly. Brian Wickman wrote: 1. I think force failing the health check would be counterproductive -- if you accidentally fat finger, you don't want to kill the task which might already be a unicorn. Instead just unlinking right away or perhaps using a default snooze value. 2. Maybe do mtime + time in the file as the snooze expiration, and re-read if the mtime changed. This means at least doing a stat() each health check interval, but will allow you to change the snooze on the fly. How about removing the time control altogether, and let presence of the file serve as the snooze? It's easier to add the time control later than to remove it if we decide it's unneeded. This allows us to sidestep the malformed file content discussion. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26383/#review55592 --- On Oct. 6, 2014, 9:24 p.m., David Pan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26383/ --- (Updated Oct. 6, 2014, 9:24 p.m.) Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji. Repository: aurora Description --- The health check disabler allows health checks for a job to be snoozed temporarily by touching a snooze file in the job's sandbox. The path of the snooze file and the snooze duration can be set in the HealthCheckConfig. The appropriate unit tests were modified/added. The corresponding JIRA ticket is the following: https://issues.apache.org/jira/browse/AURORA-795 Diffs - docs/configuration-reference.md 5166d45ddf95ae5d8afe39dd3b00654ac91857ec docs/configuration-tutorial.md 67998e9dab6ac429d96d7c0d2df959336b767f32 src/main/python/apache/aurora/config/schema/base.py f12634f103c3eb20e43f37c25d9b0fc3e3d228ec src/main/python/apache/aurora/executor/common/health_checker.py 4980411c847d12655cbb363404707ebd9f0bd163 src/test/python/apache/aurora/executor/common/BUILD c7f7a003c865d479ba6e3cd7b5349322f884f653 src/test/python/apache/aurora/executor/common/test_health_checker.py aa36415fa891fc523a3a376ffeca5d3cd5ceabec Diff: https://reviews.apache.org/r/26383/diff/ Testing --- On vagrant in ~/aurora, I ran ./pants src/test/python/apache/aurora/executor:: Thanks, David Pan
Re: Review Request 26300: Don't kill GC Executor after period of inactivity
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26300/#review55821 --- Ship it! Ship It! - David McLaughlin On Oct. 3, 2014, 1:59 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26300/ --- (Updated Oct. 3, 2014, 1:59 a.m.) Review request for Aurora, Ben Mahler, Vinod Kone, and Brian Wickman. Bugs: AURORA-788 https://issues.apache.org/jira/browse/AURORA-788 Repository: aurora Description --- The GC executor is configured to exit after 15 minutes of inactivity. This leads to a race where the mesos slave gets a launchTask message for a GC executor just as the executor has exited, causing TASK_LOST noise. This also increases the risk that a slave will lose its GC executor due to the scheduler not being able to find a slot for it (since GC executors will have a higher churn rate). Cluster operators will still be able to deploy new versions of the GC executor as the 24-hour max lifetime limit is still in place. This patch only removes the inactivity limit. Diffs - src/main/python/apache/aurora/executor/gc_executor.py 44eb0da984a126536f0d277da3da128089201a47 src/test/python/apache/aurora/executor/test_gc_executor.py 774c9ba0d5c31fc4c46dbe257579e013460fa943 Diff: https://reviews.apache.org/r/26300/diff/ Testing --- ./pants src/test/python/apache/aurora/executor:gc_executor Thanks, Kevin Sweeney
Re: Review Request 26320: Skip checkstyle on python file in 3rdparty.
On Oct. 6, 2014, 4:33 p.m., Joshua Cohen wrote: *ping* Kevin. *ping* again. Once this ships the static assets changes from https://github.com/jcohen/incubator-aurora/commits/jcohen/static-assets (reviewed at https://reviews.apache.org/r/25835/) should be mergeable. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26320/#review55511 --- On Oct. 3, 2014, 4:51 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26320/ --- (Updated Oct. 3, 2014, 4:51 p.m.) Review request for Aurora, Kevin Sweeney and Brian Wickman. Bugs: AURORA-780 https://issues.apache.org/jira/browse/AURORA-780 Repository: aurora Description --- Skip checkstyle on python file in 3rdparty. Diffs - 3rdparty/javascript/bower_components/bootstrap/test-infra/s3_cache.py 472963a1e4a6c9ace6044273c7f728812aa8458b Diff: https://reviews.apache.org/r/26320/diff/ Testing --- Committed file w/ precommit hook in place. Thanks, Joshua Cohen
Re: Review Request 26422: Drop syncrhonized from JobUpdateEventSubscriber
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26422/ --- (Updated Oct. 8, 2014, 10:27 a.m.) Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji. Bugs: AURORA-801 https://issues.apache.org/jira/browse/AURORA-801 Repository: aurora Description --- Drop syncrhonized from JobUpdateEventSubscriber This fixes a startup deadlock. Diffs - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriber.java 49d8b7a6c4adc4c58049c439bd09019c9e6885b1 Diff: https://reviews.apache.org/r/26422/diff/ Testing --- ./gradlew -Pq build Manually verified that all delegated calls to the JobUpdateController are already protected by the storage write-lock. Rather than add a potentially-flaky regression test (like the one added in https://reviews.apache.org/r/25556/) I'd prefer to prioritize adding runtime deadlock detection (https://issues.apache.org/jira/browse/AURORA-800). Thanks, Kevin Sweeney
Re: Review Request 26432: Reject new GC tasks when shutting down
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26432/ --- (Updated Oct. 8, 2014, 10:28 a.m.) Review request for Aurora, David McLaughlin, Bill Farner, and Brian Wickman. Bugs: AURORA-807 https://issues.apache.org/jira/browse/AURORA-807 Repository: aurora Description --- Reject new GC tasks when shutting down Diffs - src/main/python/apache/aurora/executor/gc_executor.py 80ce6dc93e17f01c95a84ee8bef2ce87cafa88c0 src/test/python/apache/aurora/executor/test_gc_executor.py d90dc68ec1526110a484cfc2b6c4658abdc2e871 Diff: https://reviews.apache.org/r/26432/diff/ Testing --- ./pants src/test/python/apache/aurora/executor:gc_executor Thanks, Kevin Sweeney
Re: Review Request 26432: Reject new GC tasks when shutting down
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26432/#review55828 --- Ship it! Ship It! - David McLaughlin On Oct. 8, 2014, 5:28 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26432/ --- (Updated Oct. 8, 2014, 5:28 p.m.) Review request for Aurora, David McLaughlin, Bill Farner, and Brian Wickman. Bugs: AURORA-807 https://issues.apache.org/jira/browse/AURORA-807 Repository: aurora Description --- Reject new GC tasks when shutting down Diffs - src/main/python/apache/aurora/executor/gc_executor.py 80ce6dc93e17f01c95a84ee8bef2ce87cafa88c0 src/test/python/apache/aurora/executor/test_gc_executor.py d90dc68ec1526110a484cfc2b6c4658abdc2e871 Diff: https://reviews.apache.org/r/26432/diff/ Testing --- ./pants src/test/python/apache/aurora/executor:gc_executor Thanks, Kevin Sweeney
Re: Review Request 26422: Drop syncrhonized from JobUpdateEventSubscriber
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26422/#review55827 --- Ship it! Ship It! - David McLaughlin On Oct. 8, 2014, 5:27 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26422/ --- (Updated Oct. 8, 2014, 5:27 p.m.) Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji. Bugs: AURORA-801 https://issues.apache.org/jira/browse/AURORA-801 Repository: aurora Description --- Drop syncrhonized from JobUpdateEventSubscriber This fixes a startup deadlock. Diffs - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriber.java 49d8b7a6c4adc4c58049c439bd09019c9e6885b1 Diff: https://reviews.apache.org/r/26422/diff/ Testing --- ./gradlew -Pq build Manually verified that all delegated calls to the JobUpdateController are already protected by the storage write-lock. Rather than add a potentially-flaky regression test (like the one added in https://reviews.apache.org/r/25556/) I'd prefer to prioritize adding runtime deadlock detection (https://issues.apache.org/jira/browse/AURORA-800). Thanks, Kevin Sweeney
Re: Review Request 26422: Drop syncrhonized from JobUpdateEventSubscriber
On Oct. 8, 2014, 9:19 a.m., Bill Farner wrote: While our minds are on deadlock risks, it's a good idea to assess other potential vulnerabilities. A quick filter to find other potential sources deserving a glance: $ grep -Rl synchronized src/main/java | xargs grep -l Storage src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java src/main/java/org/apache/aurora/scheduler/async/Preemptor.java src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java src/main/java/org/apache/aurora/scheduler/TaskVars.java src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java My proposal is to add runtime deadlock detection for these cases via CycleDetectingLockFactory. I have runtime evidence that this deadlock exists and would like to keep this change small in scope. Happy to add this as a followup item to AURORA-800. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26422/#review55803 --- On Oct. 8, 2014, 10:27 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26422/ --- (Updated Oct. 8, 2014, 10:27 a.m.) Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji. Bugs: AURORA-801 https://issues.apache.org/jira/browse/AURORA-801 Repository: aurora Description --- Drop syncrhonized from JobUpdateEventSubscriber This fixes a startup deadlock. Diffs - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriber.java 49d8b7a6c4adc4c58049c439bd09019c9e6885b1 Diff: https://reviews.apache.org/r/26422/diff/ Testing --- ./gradlew -Pq build Manually verified that all delegated calls to the JobUpdateController are already protected by the storage write-lock. Rather than add a potentially-flaky regression test (like the one added in https://reviews.apache.org/r/25556/) I'd prefer to prioritize adding runtime deadlock detection (https://issues.apache.org/jira/browse/AURORA-800). Thanks, Kevin Sweeney
Re: Review Request 26432: Reject new GC tasks when shutting down
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26432/#review55836 --- Ship it! Ship It! - Brian Wickman On Oct. 8, 2014, 5:28 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26432/ --- (Updated Oct. 8, 2014, 5:28 p.m.) Review request for Aurora, David McLaughlin, Bill Farner, and Brian Wickman. Bugs: AURORA-807 https://issues.apache.org/jira/browse/AURORA-807 Repository: aurora Description --- Reject new GC tasks when shutting down Diffs - src/main/python/apache/aurora/executor/gc_executor.py 80ce6dc93e17f01c95a84ee8bef2ce87cafa88c0 src/test/python/apache/aurora/executor/test_gc_executor.py d90dc68ec1526110a484cfc2b6c4658abdc2e871 Diff: https://reviews.apache.org/r/26432/diff/ Testing --- ./pants src/test/python/apache/aurora/executor:gc_executor Thanks, Kevin Sweeney
Re: Review Request 26363: Make the large-update check in the client update command consider instance parameters.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26363/ --- (Updated Oct. 8, 2014, 1:40 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Changes --- Remove debug prints. Bugs: aurora-792 https://issues.apache.org/jira/browse/aurora-792 Repository: aurora Description --- Make the large-update check in the client update command consider instance parameters. Diffs (updated) - src/main/python/apache/aurora/client/cli/context.py 301531fcb443297facb78d87a18073c8b7fd4064 src/main/python/apache/aurora/client/cli/jobs.py 103fb53cb5101d3e025d8712e3887bccdfbb6aeb src/test/python/apache/aurora/client/cli/BUILD 8ce6bd5b7faa1579372fb6935180ea982af64af8 src/test/python/apache/aurora/client/cli/test_update.py 85b1db19d89967a741bfba7964eeb368426f0b61 Diff: https://reviews.apache.org/r/26363/diff/ Testing --- New unit tests added, all test pass. Thanks, Mark Chu-Carroll
Re: Review Request 26448: Move the error-log seed file to a user specified directory.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26448/#review55838 --- Ship it! Ship It! - Zameer Manji On Oct. 8, 2014, 10:32 a.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26448/ --- (Updated Oct. 8, 2014, 10:32 a.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-779 https://issues.apache.org/jira/browse/aurora-779 Repository: aurora Description --- Instead of dumping error traces into the users current directory, dump them into a user specified directory, with a benign default. Diffs - src/main/python/apache/aurora/client/cli/__init__.py 5c0688f5ea184e2c0b1f8e1b54a109d8260a12fa src/main/python/apache/aurora/client/cli/standalone_client.py 4fdcf6df493f63aae672e0834214dad208cf4110 src/test/python/apache/aurora/client/cli/test_create.py 8dc0ccb78539f8b0c7829ec2927da93d4afa9ada src/test/python/apache/aurora/client/cli/test_inspect.py fd35fe0185c1850910bd18d6a1e5831cd3effa6f src/test/python/apache/aurora/client/cli/test_plugins.py 7a0a31818cbc57de952d7817f8e7c8fa1e84b25a src/test/python/apache/aurora/client/cli/util.py ff7eda20dbba073c8b24fbe3f4389292aab2d128 Diff: https://reviews.apache.org/r/26448/diff/ Testing --- Added new unit test; verified all tests pass. Thanks, Mark Chu-Carroll
Re: Review Request 26445: Fix error in incorrect deprecation warning on v1 ssh command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26445/#review55840 --- Ship it! Ship It! - Zameer Manji On Oct. 8, 2014, 8:58 a.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26445/ --- (Updated Oct. 8, 2014, 8:58 a.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-804 https://issues.apache.org/jira/browse/aurora-804 Repository: aurora Description --- The error message incorrectly displayed a list-value for the --tunnels parameter, when in fact it should expand the list into multiple instances of --tunnels. Diffs - src/main/python/apache/aurora/client/commands/ssh.py 9ee94a0bdfd8bbc030ae978e2ac3fe39571b4ce2 src/test/python/apache/aurora/client/commands/test_ssh.py c5ca66e378bfc97c40a406a758ae4dfaef8ab2c8 Diff: https://reviews.apache.org/r/26445/diff/ Testing --- New unit test. Thanks, Mark Chu-Carroll
Re: Review Request 26432: Reject new GC tasks when shutting down
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26432/#review55839 --- Ship it! Awesome - Joe Smith On Oct. 8, 2014, 10:28 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26432/ --- (Updated Oct. 8, 2014, 10:28 a.m.) Review request for Aurora, David McLaughlin, Bill Farner, and Brian Wickman. Bugs: AURORA-807 https://issues.apache.org/jira/browse/AURORA-807 Repository: aurora Description --- Reject new GC tasks when shutting down Diffs - src/main/python/apache/aurora/executor/gc_executor.py 80ce6dc93e17f01c95a84ee8bef2ce87cafa88c0 src/test/python/apache/aurora/executor/test_gc_executor.py d90dc68ec1526110a484cfc2b6c4658abdc2e871 Diff: https://reviews.apache.org/r/26432/diff/ Testing --- ./pants src/test/python/apache/aurora/executor:gc_executor Thanks, Kevin Sweeney
Re: Review Request 25835: Serve HTTP assets out of a standard classpath root.
On Oct. 3, 2014, 4:43 p.m., Bill Farner wrote: Anything left to do here before Kevin commits? Joshua Cohen wrote: Nope, he tried to commit but was blocked by checkstyle flagging 3rdparty python (https://issues.apache.org/jira/browse/AURORA-780). I sent a review out to address that this morning, so once that ships he should be able to merge this. Kevin - anything else blocking this? - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25835/#review55353 --- On Sept. 19, 2014, 9:39 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25835/ --- (Updated Sept. 19, 2014, 9:39 p.m.) Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner. Bugs: AURORA-678 https://issues.apache.org/jira/browse/AURORA-678 Repository: aurora Description --- Serve HTTP assets out of a standard claspath root. There's a lot of moved files in this diff, so apologies for it being essentially unreadable. ReviewBoard actually seems to choke about half the time just loading the individual pages. For the sake of convenience/sanity, the actual meat of the changes can be found in: build.gradle: https://reviews.apache.org/r/25835/diff/#336 src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java: https://reviews.apache.org/r/25835/diff/1/?page=17#338 src/main/resources/scheduler/assets/index.html: https://reviews.apache.org/r/25835/diff/1/?page=18#363 Everything else is just a rename/move. Diffs - 3rdparty/javascript/bower_components/angular-bootstrap/.bower.json 3rdparty/javascript/bower_components/angular-bootstrap/bower.json 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.js 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.min.js 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.js 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.min.js 3rdparty/javascript/bower_components/angular-route/.bower.json 3rdparty/javascript/bower_components/angular-route/README.md 3rdparty/javascript/bower_components/angular-route/angular-route.js 3rdparty/javascript/bower_components/angular-route/angular-route.min.js 3rdparty/javascript/bower_components/angular-route/angular-route.min.js.map 3rdparty/javascript/bower_components/angular-route/bower.json 3rdparty/javascript/bower_components/angular/.bower.json 3rdparty/javascript/bower_components/angular/README.md 3rdparty/javascript/bower_components/angular/angular-csp.css 3rdparty/javascript/bower_components/angular/angular.js 3rdparty/javascript/bower_components/angular/angular.min.js 3rdparty/javascript/bower_components/angular/angular.min.js.gzip 3rdparty/javascript/bower_components/angular/angular.min.js.map 3rdparty/javascript/bower_components/angular/bower.json 3rdparty/javascript/bower_components/bootstrap/.bower.json 3rdparty/javascript/bower_components/bootstrap/Gruntfile.js 3rdparty/javascript/bower_components/bootstrap/LICENSE 3rdparty/javascript/bower_components/bootstrap/README.md 3rdparty/javascript/bower_components/bootstrap/bower.json 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css.map 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.min.css 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css.map 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.min.css 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.eot 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.svg 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.ttf 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.woff 3rdparty/javascript/bower_components/bootstrap/dist/js/bootstrap.js 3rdparty/javascript/bower_components/bootstrap/dist/js/bootstrap.min.js 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.eot 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.svg 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.ttf
Re: Review Request 26422: Drop syncrhonized from JobUpdateEventSubscriber
On Oct. 8, 2014, 4:19 p.m., Bill Farner wrote: While our minds are on deadlock risks, it's a good idea to assess other potential vulnerabilities. A quick filter to find other potential sources deserving a glance: $ grep -Rl synchronized src/main/java | xargs grep -l Storage src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java src/main/java/org/apache/aurora/scheduler/async/Preemptor.java src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java src/main/java/org/apache/aurora/scheduler/TaskVars.java src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java Kevin Sweeney wrote: My proposal is to add runtime deadlock detection for these cases via CycleDetectingLockFactory. I have runtime evidence that this deadlock exists and would like to keep this change small in scope. Happy to add this as a followup item to AURORA-800. That effort shouldn't cause us to skip due diligence of a skim for other places we're vulnerable. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26422/#review55803 --- On Oct. 8, 2014, 5:27 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26422/ --- (Updated Oct. 8, 2014, 5:27 p.m.) Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji. Bugs: AURORA-801 https://issues.apache.org/jira/browse/AURORA-801 Repository: aurora Description --- Drop syncrhonized from JobUpdateEventSubscriber This fixes a startup deadlock. Diffs - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriber.java 49d8b7a6c4adc4c58049c439bd09019c9e6885b1 Diff: https://reviews.apache.org/r/26422/diff/ Testing --- ./gradlew -Pq build Manually verified that all delegated calls to the JobUpdateController are already protected by the storage write-lock. Rather than add a potentially-flaky regression test (like the one added in https://reviews.apache.org/r/25556/) I'd prefer to prioritize adding runtime deadlock detection (https://issues.apache.org/jira/browse/AURORA-800). Thanks, Kevin Sweeney
Re: Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26308/#review55846 --- Ship it! Ship It! - Kevin Sweeney On Oct. 4, 2014, 10:55 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26308/ --- (Updated Oct. 4, 2014, 10:55 a.m.) Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll. Repository: aurora Description --- Fixes two problems: - mocking was incorrect in `test_status_api_failure`. Turns out that Mock objects were being passed around and somehow resulted in the test case passing. - use of `threading.Event()` was broken in scheduler_client.py. I don't think it's possible to enter those branches. Diffs - src/main/python/apache/aurora/client/api/__init__.py 26300792594e4005dacc139a9f89711b8a66ab61 src/main/python/apache/aurora/client/api/command_runner.py a1fed5fc75dde3a79c840515e6daa4741156ef97 src/main/python/apache/aurora/client/api/job_monitor.py 18d5c0381d43fc7b24bae4b2e5e6fdc774a74b52 src/main/python/apache/aurora/client/api/scheduler_client.py 311c954f1db245b75192d00c6aca0721085fbf32 src/main/python/apache/aurora/client/api/updater.py bf608981c2f2e7960b68c3fbda144277a59a3d40 src/main/python/apache/aurora/common/aurora_job_key.py a7ca7b6df6b9566b3ce617283ccac948deb2eb83 src/test/python/apache/aurora/client/api/test_job_monitor.py 5b26539f86a0a82f72753a803a769eda77cbc332 src/test/python/apache/aurora/client/api/test_scheduler_client.py 1835843f1795b0530874ec561582df17acfbce65 src/test/python/apache/aurora/client/api/test_updater.py 6905831b23a84320e7f41843efd62b86da366c0b src/test/python/apache/aurora/client/cli/test_api_from_cli.py 78f21d2f20cf71fa2dfe0614885d44d2948decd2 src/test/python/apache/aurora/client/cli/test_cancel_update.py c15e142930c9474c7873dd931261b6ab4eb5967f src/test/python/apache/aurora/client/cli/test_command_hooks.py 3acd2ba0d8bd8c71d4c0a9d71a035fc974fa20c3 src/test/python/apache/aurora/client/cli/test_create.py 6e55188bdfc576506848605debb391288e696fe3 src/test/python/apache/aurora/client/cli/test_diff.py e1a6f764830e06c73d0bc10a3b5da67219da836c src/test/python/apache/aurora/client/cli/test_kill.py e3a366bf67074e50787394cad58d5e01359b641e src/test/python/apache/aurora/client/cli/test_logging.py 6285fbb07442291c2dc4096e68eb285c98994097 src/test/python/apache/aurora/client/cli/test_plugins.py 7a0a31818cbc57de952d7817f8e7c8fa1e84b25a src/test/python/apache/aurora/client/cli/test_status.py bd30b11022ee0d1fb38da9f6efd4d9c5923b7d13 src/test/python/apache/aurora/client/cli/test_task_run.py 1ce9a632874e818eee71573cd481842affae3615 src/test/python/apache/aurora/client/cli/test_update.py 85b1db19d89967a741bfba7964eeb368426f0b61 src/test/python/apache/aurora/client/commands/test_admin.py 1192556c027dc3adf16bb37adeac7798cf9ef93d src/test/python/apache/aurora/client/commands/test_cancel_update.py 5f05ef7c0643d189de3de38c75aae58c2a3814a4 src/test/python/apache/aurora/client/commands/test_create.py 7503345ea1c0f224a894ce02cc2c2d8719574e32 src/test/python/apache/aurora/client/commands/test_diff.py 8f5da7d2bca9b0486b635afe49d3885151624e12 src/test/python/apache/aurora/client/commands/test_hooks.py 0861f13b13a8406950ba953efba0ffae186a8253 src/test/python/apache/aurora/client/commands/test_kill.py c0a6fd44c5691cde50746ffdec325bb11a33469a src/test/python/apache/aurora/client/commands/test_run.py e97b5156609f80a4170028e780bcd891e56983ff src/test/python/apache/aurora/client/commands/test_ssh.py c5ca66e378bfc97c40a406a758ae4dfaef8ab2c8 src/test/python/apache/aurora/client/commands/test_status.py bda1f28d544ae48428129f8167d8632ef27f5fba src/test/python/apache/aurora/client/commands/test_update.py af2cbc7f88287201a472ba36902b00d90bc77d3b Diff: https://reviews.apache.org/r/26308/diff/ Testing --- ./pants src/test/python:all -vxs Thanks, Bill Farner
Re: Review Request 26320: Skip checkstyle on python file in 3rdparty.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26320/#review55847 --- Apologies for the review delay. Is there a way we can tell checkstyle to avoid this file at all? I'd rather not make it a policy to patch each individual bower component we checkin. - Kevin Sweeney On Oct. 3, 2014, 9:51 a.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26320/ --- (Updated Oct. 3, 2014, 9:51 a.m.) Review request for Aurora, Kevin Sweeney and Brian Wickman. Bugs: AURORA-780 https://issues.apache.org/jira/browse/AURORA-780 Repository: aurora Description --- Skip checkstyle on python file in 3rdparty. Diffs - 3rdparty/javascript/bower_components/bootstrap/test-infra/s3_cache.py 472963a1e4a6c9ace6044273c7f728812aa8458b Diff: https://reviews.apache.org/r/26320/diff/ Testing --- Committed file w/ precommit hook in place. Thanks, Joshua Cohen
Re: Review Request 26445: Fix error in incorrect deprecation warning on v1 ssh command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26445/#review55849 --- Ship it! Ship It! - David McLaughlin On Oct. 8, 2014, 3:58 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26445/ --- (Updated Oct. 8, 2014, 3:58 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-804 https://issues.apache.org/jira/browse/aurora-804 Repository: aurora Description --- The error message incorrectly displayed a list-value for the --tunnels parameter, when in fact it should expand the list into multiple instances of --tunnels. Diffs - src/main/python/apache/aurora/client/commands/ssh.py 9ee94a0bdfd8bbc030ae978e2ac3fe39571b4ce2 src/test/python/apache/aurora/client/commands/test_ssh.py c5ca66e378bfc97c40a406a758ae4dfaef8ab2c8 Diff: https://reviews.apache.org/r/26445/diff/ Testing --- New unit test. Thanks, Mark Chu-Carroll
Re: Review Request 26320: Skip checkstyle on python file in 3rdparty.
On Oct. 8, 2014, 6:02 p.m., Kevin Sweeney wrote: Apologies for the review delay. Is there a way we can tell checkstyle to avoid this file at all? I'd rather not make it a policy to patch each individual bower component we checkin. Not without patching checkstyle alas. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26320/#review55847 --- On Oct. 3, 2014, 4:51 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26320/ --- (Updated Oct. 3, 2014, 4:51 p.m.) Review request for Aurora, Kevin Sweeney and Brian Wickman. Bugs: AURORA-780 https://issues.apache.org/jira/browse/AURORA-780 Repository: aurora Description --- Skip checkstyle on python file in 3rdparty. Diffs - 3rdparty/javascript/bower_components/bootstrap/test-infra/s3_cache.py 472963a1e4a6c9ace6044273c7f728812aa8458b Diff: https://reviews.apache.org/r/26320/diff/ Testing --- Committed file w/ precommit hook in place. Thanks, Joshua Cohen
Re: Review Request 26422: Drop syncrhonized from JobUpdateEventSubscriber
On Oct. 8, 2014, 9:19 a.m., Bill Farner wrote: While our minds are on deadlock risks, it's a good idea to assess other potential vulnerabilities. A quick filter to find other potential sources deserving a glance: $ grep -Rl synchronized src/main/java | xargs grep -l Storage src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java src/main/java/org/apache/aurora/scheduler/async/Preemptor.java src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java src/main/java/org/apache/aurora/scheduler/TaskVars.java src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java Kevin Sweeney wrote: My proposal is to add runtime deadlock detection for these cases via CycleDetectingLockFactory. I have runtime evidence that this deadlock exists and would like to keep this change small in scope. Happy to add this as a followup item to AURORA-800. Bill Farner wrote: That effort shouldn't cause us to skip due diligence of a skim for other places we're vulnerable. A cursory look through doesn't reveal any immediate concerns. Preemptor does acquire the storage lock in a synchronized method; however the only caller of Preemptor always holds the storage write lock. Others just use synchronization to ensure consistent internal state. Note I used 'synchronized ' to avoid synchronizedMap. % grep -Rl 'synchronized ' src/main/java | xargs grep -lE '(.write|.consistentRead|.consistentFetchTasks)' src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java src/main/java/org/apache/aurora/scheduler/async/Preemptor.java src/main/java/org/apache/aurora/scheduler/TaskVars.java src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLog.java Of course, this doesn't reveal cases where a call to a dependency might cause the storage lock to be acquired, nor does it protect against accidental introduction of new deadlocks so AURORA-800 is still relevant. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26422/#review55803 --- On Oct. 8, 2014, 10:27 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26422/ --- (Updated Oct. 8, 2014, 10:27 a.m.) Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji. Bugs: AURORA-801 https://issues.apache.org/jira/browse/AURORA-801 Repository: aurora Description --- Drop syncrhonized from JobUpdateEventSubscriber This fixes a startup deadlock. Diffs - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriber.java 49d8b7a6c4adc4c58049c439bd09019c9e6885b1 Diff: https://reviews.apache.org/r/26422/diff/ Testing --- ./gradlew -Pq build Manually verified that all delegated calls to the JobUpdateController are already protected by the storage write-lock. Rather than add a potentially-flaky regression test (like the one added in https://reviews.apache.org/r/25556/) I'd prefer to prioritize adding runtime deadlock detection (https://issues.apache.org/jira/browse/AURORA-800). Thanks, Kevin Sweeney
Re: Review Request 26422: Drop syncrhonized from JobUpdateEventSubscriber
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26422/#review55854 --- Ship it! Thanks for the extra look! - Bill Farner On Oct. 8, 2014, 5:27 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26422/ --- (Updated Oct. 8, 2014, 5:27 p.m.) Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji. Bugs: AURORA-801 https://issues.apache.org/jira/browse/AURORA-801 Repository: aurora Description --- Drop syncrhonized from JobUpdateEventSubscriber This fixes a startup deadlock. Diffs - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriber.java 49d8b7a6c4adc4c58049c439bd09019c9e6885b1 Diff: https://reviews.apache.org/r/26422/diff/ Testing --- ./gradlew -Pq build Manually verified that all delegated calls to the JobUpdateController are already protected by the storage write-lock. Rather than add a potentially-flaky regression test (like the one added in https://reviews.apache.org/r/25556/) I'd prefer to prioritize adding runtime deadlock detection (https://issues.apache.org/jira/browse/AURORA-800). Thanks, Kevin Sweeney
Re: Review Request 26300: Don't kill GC Executor after period of inactivity
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26300/ --- (Updated Oct. 8, 2014, 11:56 a.m.) Review request for Aurora, Ben Mahler, Vinod Kone, and Brian Wickman. Changes --- rebase Bugs: AURORA-788 https://issues.apache.org/jira/browse/AURORA-788 Repository: aurora Description --- The GC executor is configured to exit after 15 minutes of inactivity. This leads to a race where the mesos slave gets a launchTask message for a GC executor just as the executor has exited, causing TASK_LOST noise. This also increases the risk that a slave will lose its GC executor due to the scheduler not being able to find a slot for it (since GC executors will have a higher churn rate). Cluster operators will still be able to deploy new versions of the GC executor as the 24-hour max lifetime limit is still in place. This patch only removes the inactivity limit. Diffs (updated) - src/main/python/apache/aurora/executor/gc_executor.py 788671e32d2f995b954e906d8866019ed66c970d src/test/python/apache/aurora/executor/test_gc_executor.py 1905fe35de31e667f499f7945952f04dc13aac06 Diff: https://reviews.apache.org/r/26300/diff/ Testing --- ./pants src/test/python/apache/aurora/executor:gc_executor Thanks, Kevin Sweeney
Re: Review Request 26428: Fixing python style violations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26428/#review55857 --- Ship it! Ship It! - Brian Wickman On Oct. 8, 2014, 12:17 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26428/ --- (Updated Oct. 8, 2014, 12:17 a.m.) Review request for Aurora and Brian Wickman. Repository: aurora Description --- Pre-commit hook was no longer working properly. Added target to match jenkins definition. Diffs - build-support/hooks/pre-commit 24646e7b1c43a0177beb0084dd2af4c4b3dc686c src/test/python/apache/aurora/client/api/test_updater.py fc6a057c6c650d9ac9800b009e544dfad0c809bf src/test/python/apache/aurora/client/cli/test_status.py 38ffdb86c5f577ebf3a482128588331a63af15d1 src/test/python/apache/aurora/client/cli/util.py ff7eda20dbba073c8b24fbe3f4389292aab2d128 Diff: https://reviews.apache.org/r/26428/diff/ Testing --- ./build-support/hooks/pre-commit Performing Python import order check. SUCCESS Performing Python checkstyle. SUCCESS $ build-support/python/checkstyle-check src Thanks, Maxim Khutornenko
Re: Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26308/#review55858 --- Ship it! Ship It! - Joe Smith On Oct. 4, 2014, 10:55 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26308/ --- (Updated Oct. 4, 2014, 10:55 a.m.) Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll. Repository: aurora Description --- Fixes two problems: - mocking was incorrect in `test_status_api_failure`. Turns out that Mock objects were being passed around and somehow resulted in the test case passing. - use of `threading.Event()` was broken in scheduler_client.py. I don't think it's possible to enter those branches. Diffs - src/main/python/apache/aurora/client/api/__init__.py 26300792594e4005dacc139a9f89711b8a66ab61 src/main/python/apache/aurora/client/api/command_runner.py a1fed5fc75dde3a79c840515e6daa4741156ef97 src/main/python/apache/aurora/client/api/job_monitor.py 18d5c0381d43fc7b24bae4b2e5e6fdc774a74b52 src/main/python/apache/aurora/client/api/scheduler_client.py 311c954f1db245b75192d00c6aca0721085fbf32 src/main/python/apache/aurora/client/api/updater.py bf608981c2f2e7960b68c3fbda144277a59a3d40 src/main/python/apache/aurora/common/aurora_job_key.py a7ca7b6df6b9566b3ce617283ccac948deb2eb83 src/test/python/apache/aurora/client/api/test_job_monitor.py 5b26539f86a0a82f72753a803a769eda77cbc332 src/test/python/apache/aurora/client/api/test_scheduler_client.py 1835843f1795b0530874ec561582df17acfbce65 src/test/python/apache/aurora/client/api/test_updater.py 6905831b23a84320e7f41843efd62b86da366c0b src/test/python/apache/aurora/client/cli/test_api_from_cli.py 78f21d2f20cf71fa2dfe0614885d44d2948decd2 src/test/python/apache/aurora/client/cli/test_cancel_update.py c15e142930c9474c7873dd931261b6ab4eb5967f src/test/python/apache/aurora/client/cli/test_command_hooks.py 3acd2ba0d8bd8c71d4c0a9d71a035fc974fa20c3 src/test/python/apache/aurora/client/cli/test_create.py 6e55188bdfc576506848605debb391288e696fe3 src/test/python/apache/aurora/client/cli/test_diff.py e1a6f764830e06c73d0bc10a3b5da67219da836c src/test/python/apache/aurora/client/cli/test_kill.py e3a366bf67074e50787394cad58d5e01359b641e src/test/python/apache/aurora/client/cli/test_logging.py 6285fbb07442291c2dc4096e68eb285c98994097 src/test/python/apache/aurora/client/cli/test_plugins.py 7a0a31818cbc57de952d7817f8e7c8fa1e84b25a src/test/python/apache/aurora/client/cli/test_status.py bd30b11022ee0d1fb38da9f6efd4d9c5923b7d13 src/test/python/apache/aurora/client/cli/test_task_run.py 1ce9a632874e818eee71573cd481842affae3615 src/test/python/apache/aurora/client/cli/test_update.py 85b1db19d89967a741bfba7964eeb368426f0b61 src/test/python/apache/aurora/client/commands/test_admin.py 1192556c027dc3adf16bb37adeac7798cf9ef93d src/test/python/apache/aurora/client/commands/test_cancel_update.py 5f05ef7c0643d189de3de38c75aae58c2a3814a4 src/test/python/apache/aurora/client/commands/test_create.py 7503345ea1c0f224a894ce02cc2c2d8719574e32 src/test/python/apache/aurora/client/commands/test_diff.py 8f5da7d2bca9b0486b635afe49d3885151624e12 src/test/python/apache/aurora/client/commands/test_hooks.py 0861f13b13a8406950ba953efba0ffae186a8253 src/test/python/apache/aurora/client/commands/test_kill.py c0a6fd44c5691cde50746ffdec325bb11a33469a src/test/python/apache/aurora/client/commands/test_run.py e97b5156609f80a4170028e780bcd891e56983ff src/test/python/apache/aurora/client/commands/test_ssh.py c5ca66e378bfc97c40a406a758ae4dfaef8ab2c8 src/test/python/apache/aurora/client/commands/test_status.py bda1f28d544ae48428129f8167d8632ef27f5fba src/test/python/apache/aurora/client/commands/test_update.py af2cbc7f88287201a472ba36902b00d90bc77d3b Diff: https://reviews.apache.org/r/26308/diff/ Testing --- ./pants src/test/python:all -vxs Thanks, Bill Farner
Re: Review Request 26431: Moving post_drain script execution into host_maintenance.py
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26431/#review55859 --- src/test/python/apache/aurora/client/commands/test_maintenance.py https://reviews.apache.org/r/26431/#comment96235 can this be a mock that we assert gets called 3 times? (This test is a little bit of an integration test since it goes in and is also testing perform_maintenace, but a full refactor of this test may be too much for now) - Joe Smith On Oct. 7, 2014, 5:13 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26431/ --- (Updated Oct. 7, 2014, 5:13 p.m.) Review request for Aurora, Joe Smith and Mark Chu-Carroll. Bugs: AURORA-806 https://issues.apache.org/jira/browse/AURORA-806 Repository: aurora Description --- Moving post_drain script functionality into host_maintenance.py to support per batch execution. Diffs - src/main/python/apache/aurora/admin/host_maintenance.py 9c2a9f77109791da574e1624d27b6b7096a2678e src/main/python/apache/aurora/client/commands/maintenance.py e465d973e9f764076e18491e1691d44303c0f388 src/test/python/apache/aurora/admin/test_host_maintenance.py 40228df59e43bc6034f2dc651c166a0c4b78aea8 src/test/python/apache/aurora/client/commands/test_maintenance.py d86aaf677804301fa5ddf1f76dba552f4fafb8c3 Diff: https://reviews.apache.org/r/26431/diff/ Testing --- ./pants src/test/python:all Thanks, Maxim Khutornenko
Re: Review Request 26424: Disable requests http connection logging.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26424/#review55860 --- Ship it! Inlining inside `__init__` WFM. thanks! - Joe Smith On Oct. 8, 2014, 9:54 a.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26424/ --- (Updated Oct. 8, 2014, 9:54 a.m.) Review request for Aurora, Joe Smith and Bill Farner. Bugs: AURORA-770 https://issues.apache.org/jira/browse/AURORA-770 Repository: aurora Description --- . Diffs - src/main/python/apache/aurora/common/transport.py 6f7c355d725b5e537cc4ae471170eaa8431da326 src/test/python/apache/aurora/common/test_transport.py c722eae2d04dec90e9c772f49c578184a2bdf76c Diff: https://reviews.apache.org/r/26424/diff/ Testing --- Added hokey test, ran hokey test. Also verified the lack of http connection logs when running client commands directly. Thanks, Joshua Cohen
Re: Review Request 26428: Fixing python style violations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26428/ --- (Updated Oct. 8, 2014, 7:22 p.m.) Review request for Aurora and Brian Wickman. Changes --- Fixing more violations. Repository: aurora Description --- Pre-commit hook was no longer working properly. Added target to match jenkins definition. Diffs (updated) - build-support/hooks/pre-commit 24646e7b1c43a0177beb0084dd2af4c4b3dc686c src/main/python/apache/aurora/executor/gc_executor.py a11feb910cd116f53c9949343dfe7ecbd17d793c src/test/python/apache/aurora/client/api/test_updater.py fc6a057c6c650d9ac9800b009e544dfad0c809bf src/test/python/apache/aurora/client/cli/test_create.py 88d1b350b09012684bb4f7263657cdff083df215 src/test/python/apache/aurora/client/cli/test_inspect.py b64fbf9864693d8758baf01ba2293df525adebb7 src/test/python/apache/aurora/client/cli/test_status.py 38ffdb86c5f577ebf3a482128588331a63af15d1 src/test/python/apache/aurora/client/cli/util.py 4e9b6362728ecfd6e8a6efbc433f63f42aef60ad Diff: https://reviews.apache.org/r/26428/diff/ Testing --- ./build-support/hooks/pre-commit Performing Python import order check. SUCCESS Performing Python checkstyle. SUCCESS $ build-support/python/checkstyle-check src Thanks, Maxim Khutornenko
Re: Review Request 25835: Serve HTTP assets out of a standard classpath root.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25835/#review55862 --- Patch fails to apply: % ./rbt patch -c 25835 Patch is being applied from request 25835 with diff revision 4. Failed to execute command: ['git', 'commit', '-m', uServe HTTP assets out of a standard classpath root.\n\nTesting Done:\n./gradlew build -Pq\n\nConfirmed everything works in scheduler UI in vagrant image.\n\nOne thing I've seen locally is lost tasks in vagrant, but I *think*\nthat's just due to my image being somehow misconfigured, and not\nthese changes. I'm going to destroy and rebuild the image to see if\nthat cleans things up.\n\nBugs closed: AURORA-678\n\nReviewed at https://reviews.apache.org/r/25835/\n;, u'--author=Joshua Cohen jco...@twopensource.com'] Performing Python import order check. SUCCESS Performing Python checkstyle. SUCCESS On branch master - Kevin Sweeney On Sept. 19, 2014, 2:39 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25835/ --- (Updated Sept. 19, 2014, 2:39 p.m.) Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner. Bugs: AURORA-678 https://issues.apache.org/jira/browse/AURORA-678 Repository: aurora Description --- Serve HTTP assets out of a standard claspath root. There's a lot of moved files in this diff, so apologies for it being essentially unreadable. ReviewBoard actually seems to choke about half the time just loading the individual pages. For the sake of convenience/sanity, the actual meat of the changes can be found in: build.gradle: https://reviews.apache.org/r/25835/diff/#336 src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java: https://reviews.apache.org/r/25835/diff/1/?page=17#338 src/main/resources/scheduler/assets/index.html: https://reviews.apache.org/r/25835/diff/1/?page=18#363 Everything else is just a rename/move. Diffs - 3rdparty/javascript/bower_components/angular-bootstrap/.bower.json 3rdparty/javascript/bower_components/angular-bootstrap/bower.json 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.js 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.min.js 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.js 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.min.js 3rdparty/javascript/bower_components/angular-route/.bower.json 3rdparty/javascript/bower_components/angular-route/README.md 3rdparty/javascript/bower_components/angular-route/angular-route.js 3rdparty/javascript/bower_components/angular-route/angular-route.min.js 3rdparty/javascript/bower_components/angular-route/angular-route.min.js.map 3rdparty/javascript/bower_components/angular-route/bower.json 3rdparty/javascript/bower_components/angular/.bower.json 3rdparty/javascript/bower_components/angular/README.md 3rdparty/javascript/bower_components/angular/angular-csp.css 3rdparty/javascript/bower_components/angular/angular.js 3rdparty/javascript/bower_components/angular/angular.min.js 3rdparty/javascript/bower_components/angular/angular.min.js.gzip 3rdparty/javascript/bower_components/angular/angular.min.js.map 3rdparty/javascript/bower_components/angular/bower.json 3rdparty/javascript/bower_components/bootstrap/.bower.json 3rdparty/javascript/bower_components/bootstrap/Gruntfile.js 3rdparty/javascript/bower_components/bootstrap/LICENSE 3rdparty/javascript/bower_components/bootstrap/README.md 3rdparty/javascript/bower_components/bootstrap/bower.json 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css.map 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.min.css 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css.map 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.min.css 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.eot 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.svg 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.ttf 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.woff 3rdparty/javascript/bower_components/bootstrap/dist/js/bootstrap.js
Re: Review Request 25835: Serve HTTP assets out of a standard classpath root.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25835/#review55864 --- Patch fails to apply: % ./rbt patch -c 25835 Patch is being applied from request 25835 with diff revision 4. Failed to execute command: ['git', 'commit', '-m', uServe HTTP assets out of a standard classpath root.\n\nTesting Done:\n./gradlew build -Pq\n\nConfirmed everything works in scheduler UI in vagrant image.\n\nOne thing I've seen locally is lost tasks in vagrant, but I *think*\nthat's just due to my image being somehow misconfigured, and not\nthese changes. I'm going to destroy and rebuild the image to see if\nthat cleans things up.\n\nBugs closed: AURORA-678\n\nReviewed at https://reviews.apache.org/r/25835/\n;, u'--author=Joshua Cohen jco...@twopensource.com'] Performing Python import order check. SUCCESS Performing Python checkstyle. SUCCESS On branch master - Kevin Sweeney On Sept. 19, 2014, 2:39 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25835/ --- (Updated Sept. 19, 2014, 2:39 p.m.) Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner. Bugs: AURORA-678 https://issues.apache.org/jira/browse/AURORA-678 Repository: aurora Description --- Serve HTTP assets out of a standard claspath root. There's a lot of moved files in this diff, so apologies for it being essentially unreadable. ReviewBoard actually seems to choke about half the time just loading the individual pages. For the sake of convenience/sanity, the actual meat of the changes can be found in: build.gradle: https://reviews.apache.org/r/25835/diff/#336 src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java: https://reviews.apache.org/r/25835/diff/1/?page=17#338 src/main/resources/scheduler/assets/index.html: https://reviews.apache.org/r/25835/diff/1/?page=18#363 Everything else is just a rename/move. Diffs - 3rdparty/javascript/bower_components/angular-bootstrap/.bower.json 3rdparty/javascript/bower_components/angular-bootstrap/bower.json 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.js 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.min.js 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.js 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.min.js 3rdparty/javascript/bower_components/angular-route/.bower.json 3rdparty/javascript/bower_components/angular-route/README.md 3rdparty/javascript/bower_components/angular-route/angular-route.js 3rdparty/javascript/bower_components/angular-route/angular-route.min.js 3rdparty/javascript/bower_components/angular-route/angular-route.min.js.map 3rdparty/javascript/bower_components/angular-route/bower.json 3rdparty/javascript/bower_components/angular/.bower.json 3rdparty/javascript/bower_components/angular/README.md 3rdparty/javascript/bower_components/angular/angular-csp.css 3rdparty/javascript/bower_components/angular/angular.js 3rdparty/javascript/bower_components/angular/angular.min.js 3rdparty/javascript/bower_components/angular/angular.min.js.gzip 3rdparty/javascript/bower_components/angular/angular.min.js.map 3rdparty/javascript/bower_components/angular/bower.json 3rdparty/javascript/bower_components/bootstrap/.bower.json 3rdparty/javascript/bower_components/bootstrap/Gruntfile.js 3rdparty/javascript/bower_components/bootstrap/LICENSE 3rdparty/javascript/bower_components/bootstrap/README.md 3rdparty/javascript/bower_components/bootstrap/bower.json 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css.map 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.min.css 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css.map 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.min.css 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.eot 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.svg 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.ttf 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.woff 3rdparty/javascript/bower_components/bootstrap/dist/js/bootstrap.js
Re: Review Request 26425: Fixing quota checking for updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26425/#review55865 --- I'm underwater on reviews and unable to review this promptly - please remove me from the People line. - Kevin Sweeney On Oct. 7, 2014, 3:03 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26425/ --- (Updated Oct. 7, 2014, 3:03 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-802 https://issues.apache.org/jira/browse/AURORA-802 Repository: aurora Description --- Rolling up prod consumption does not work for quota checking during job update intitiation where per-instance filtering is required. Splitting the QuotaManager interface into 2 use cases: - createJob/addInstances - startJobUpdate Reverting the IResourceAggregate abstraction in QuotaManager in favor of task/update objects to simplify handling. Diffs - src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 2442b06f318c8d0cefe60296950baa1b916b42f7 src/main/java/org/apache/aurora/scheduler/quota/QuotaUtil.java d197dd515eb646cb4babadf22e9cf6480a919060 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 5dcae4a6132026504cf02093ad4c68ab521e4ab8 src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java b58c8f363e6e7c72accaf590b2a7cb7bf24275ea src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 02cd8f712fff3d283abf8e3eb1b4dcab1e762ac2 Diff: https://reviews.apache.org/r/26425/diff/ Testing --- ./gradlew -Pq build Also, tested various update scenarios in vagrant. Thanks, Maxim Khutornenko
Re: Review Request 26430: Remove deprecated configuration options.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26430/#review55870 --- src/main/python/apache/aurora/client/config.py https://reviews.apache.org/r/26430/#comment96239 default environments are also deprecated and should be removed. src/main/python/apache/aurora/config/thrift.py https://reviews.apache.org/r/26430/#comment96241 cron_policy shouldn't be empty right? since it's a Default() field. src/main/python/apache/aurora/config/thrift.py https://reviews.apache.org/r/26430/#comment96242 same -- service is a Default(Boolean, False) so has_service() should never be false. - Brian Wickman On Oct. 8, 2014, 12:05 a.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26430/ --- (Updated Oct. 8, 2014, 12:05 a.m.) Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Brian Wickman. Bugs: AURORA-333 https://issues.apache.org/jira/browse/AURORA-333 Repository: aurora Description --- This removes several long deprecated configuration options from aurora. The features removed are cron_policy, daemon, health_check_interval_secs, recipes and the PackerObject Diffs - docs/configuration-reference.md 5166d45ddf95ae5d8afe39dd3b00654ac91857ec src/main/python/apache/aurora/client/binding_helper.py 6d6a06785c6840e4345e304eb4e242682676ac66 src/main/python/apache/aurora/client/config.py e440f587d100ce46b2df85ccc663912c615051ef src/main/python/apache/aurora/config/recipes.py 68b5d252f87a592d4c2f7d52525163829bea2cc9 src/main/python/apache/aurora/config/schema/base.py f12634f103c3eb20e43f37c25d9b0fc3e3d228ec src/main/python/apache/aurora/config/thrift.py 288fb40f65629c8fd4eb7d92c8bf02369237de3b src/test/python/apache/aurora/client/cli/test_status.py 38ffdb86c5f577ebf3a482128588331a63af15d1 src/test/python/apache/aurora/client/cli/util.py ff7eda20dbba073c8b24fbe3f4389292aab2d128 src/test/python/apache/aurora/client/commands/util.py 663f2f4a16113a36826943b7238cad900ae0dcd2 src/test/python/apache/aurora/config/test_thrift.py fd28313df2cfd5a9c7d00f6d329518b4caabacb2 Diff: https://reviews.apache.org/r/26430/diff/ Testing --- ./build-support/jenkins/build.sh Thanks, Zameer Manji
Re: Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26308/ --- (Updated Oct. 8, 2014, 8:57 p.m.) Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll. Changes --- Another rebase cycle, more merge conflicts while waiting for tests to run. Repository: aurora Description --- Fixes two problems: - mocking was incorrect in `test_status_api_failure`. Turns out that Mock objects were being passed around and somehow resulted in the test case passing. - use of `threading.Event()` was broken in scheduler_client.py. I don't think it's possible to enter those branches. Diffs (updated) - src/main/python/apache/aurora/client/api/__init__.py 26300792594e4005dacc139a9f89711b8a66ab61 src/main/python/apache/aurora/client/api/command_runner.py a1fed5fc75dde3a79c840515e6daa4741156ef97 src/main/python/apache/aurora/client/api/job_monitor.py 18d5c0381d43fc7b24bae4b2e5e6fdc774a74b52 src/main/python/apache/aurora/client/api/scheduler_client.py 311c954f1db245b75192d00c6aca0721085fbf32 src/main/python/apache/aurora/client/api/updater.py bb4a7555851341e450da408441975f078f2b5576 src/main/python/apache/aurora/common/aurora_job_key.py a7ca7b6df6b9566b3ce617283ccac948deb2eb83 src/test/python/apache/aurora/client/api/test_job_monitor.py 5b26539f86a0a82f72753a803a769eda77cbc332 src/test/python/apache/aurora/client/api/test_scheduler_client.py 1835843f1795b0530874ec561582df17acfbce65 src/test/python/apache/aurora/client/api/test_updater.py fe27391a7b267e5acf4dc5f1a82a8199777d5b04 src/test/python/apache/aurora/client/cli/test_api_from_cli.py 733327b557df3e81c6d723bf24e753442fa375ca src/test/python/apache/aurora/client/cli/test_cancel_update.py c15e142930c9474c7873dd931261b6ab4eb5967f src/test/python/apache/aurora/client/cli/test_command_hooks.py 3acd2ba0d8bd8c71d4c0a9d71a035fc974fa20c3 src/test/python/apache/aurora/client/cli/test_create.py d17203578438fcae1f12b5e70e78e420b87554e4 src/test/python/apache/aurora/client/cli/test_diff.py e1a6f764830e06c73d0bc10a3b5da67219da836c src/test/python/apache/aurora/client/cli/test_kill.py e3a366bf67074e50787394cad58d5e01359b641e src/test/python/apache/aurora/client/cli/test_logging.py 6285fbb07442291c2dc4096e68eb285c98994097 src/test/python/apache/aurora/client/cli/test_plugins.py 7c79c7a75e77e47544684c0f767d248d2540901a src/test/python/apache/aurora/client/cli/test_status.py d26371249aa4f5b671c9ace9b503e756cb39528a src/test/python/apache/aurora/client/cli/test_task_run.py 1ce9a632874e818eee71573cd481842affae3615 src/test/python/apache/aurora/client/cli/test_update.py 85b1db19d89967a741bfba7964eeb368426f0b61 src/test/python/apache/aurora/client/commands/test_admin.py 1192556c027dc3adf16bb37adeac7798cf9ef93d src/test/python/apache/aurora/client/commands/test_cancel_update.py 5f05ef7c0643d189de3de38c75aae58c2a3814a4 src/test/python/apache/aurora/client/commands/test_create.py 7503345ea1c0f224a894ce02cc2c2d8719574e32 src/test/python/apache/aurora/client/commands/test_diff.py 8f5da7d2bca9b0486b635afe49d3885151624e12 src/test/python/apache/aurora/client/commands/test_hooks.py 0861f13b13a8406950ba953efba0ffae186a8253 src/test/python/apache/aurora/client/commands/test_kill.py c0a6fd44c5691cde50746ffdec325bb11a33469a src/test/python/apache/aurora/client/commands/test_run.py e97b5156609f80a4170028e780bcd891e56983ff src/test/python/apache/aurora/client/commands/test_ssh.py 33262248d14a2cc182d87fde736090f38c322bf4 src/test/python/apache/aurora/client/commands/test_status.py bda1f28d544ae48428129f8167d8632ef27f5fba src/test/python/apache/aurora/client/commands/test_update.py af2cbc7f88287201a472ba36902b00d90bc77d3b Diff: https://reviews.apache.org/r/26308/diff/ Testing --- ./pants src/test/python:all -vxs Thanks, Bill Farner
Review Request 26458: Adding wait loop into host_drain status monitoring.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26458/ --- Review request for Aurora, Joe Smith and Brian Wickman. Bugs: AURORA-820 https://issues.apache.org/jira/browse/AURORA-820 Repository: aurora Description --- Throttling status check calls now at a predefined 5 second interval with a max timeout of 5 minutes. Diffs - src/main/python/apache/aurora/admin/host_maintenance.py 9c2a9f77109791da574e1624d27b6b7096a2678e src/test/python/apache/aurora/admin/test_host_maintenance.py 40228df59e43bc6034f2dc651c166a0c4b78aea8 Diff: https://reviews.apache.org/r/26458/diff/ Testing --- ./pants src/test/python:all Thanks, Maxim Khutornenko
Re: Review Request 26363: Make the large-update check in the client update command consider instance parameters.
On Oct. 6, 2014, 4:44 p.m., Joshua Cohen wrote: src/test/python/apache/aurora/client/cli/test_update.py, lines 342-353 https://reviews.apache.org/r/26363/diff/1/?file=714123#file714123line342 This sequence of mocks and writing config to a file is repeated in... many (all?) of these tests. Can you refactor to remove the repetition? Mark Chu-Carroll wrote: I really think that that should not be done. For tests, I really like to be able to see, in an instant, exactly what mocks are being used by a particular test. I don't want to have to look somewhere else; I don't want to have to mentally combine the stuff that's mocked in a common frame and the different stuff that's mocked and/or reconfigured in a particular test. The test should be as clear as it can be, and anything from the environment that it's mucking with should be done explicitly in the most local-possible context. Not a ship blocker for me, just my perspective, but these mocks are identical in several calls, if many client test cases need to mock the exact same set of things in the exact same way, it seems worthwhile to simplify that process. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26363/#review55512 --- On Oct. 8, 2014, 5:40 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26363/ --- (Updated Oct. 8, 2014, 5:40 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-792 https://issues.apache.org/jira/browse/aurora-792 Repository: aurora Description --- Make the large-update check in the client update command consider instance parameters. Diffs - src/main/python/apache/aurora/client/cli/context.py 301531fcb443297facb78d87a18073c8b7fd4064 src/main/python/apache/aurora/client/cli/jobs.py 103fb53cb5101d3e025d8712e3887bccdfbb6aeb src/test/python/apache/aurora/client/cli/BUILD 8ce6bd5b7faa1579372fb6935180ea982af64af8 src/test/python/apache/aurora/client/cli/test_update.py 85b1db19d89967a741bfba7964eeb368426f0b61 Diff: https://reviews.apache.org/r/26363/diff/ Testing --- New unit tests added, all test pass. Thanks, Mark Chu-Carroll
Re: Review Request 26430: Remove deprecated configuration options.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26430/ --- (Updated Oct. 8, 2014, 3:29 p.m.) Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Brian Wickman. Changes --- Brian's feedback. Bugs: AURORA-333 https://issues.apache.org/jira/browse/AURORA-333 Repository: aurora Description --- This removes several long deprecated configuration options from aurora. The features removed are cron_policy, daemon, health_check_interval_secs, recipes and the PackerObject Diffs (updated) - docs/configuration-reference.md 5166d45ddf95ae5d8afe39dd3b00654ac91857ec src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 865742171c11fbe5cf1469a69dd7258ec1be28c2 src/main/python/apache/aurora/client/binding_helper.py 6d6a06785c6840e4345e304eb4e242682676ac66 src/main/python/apache/aurora/client/config.py e440f587d100ce46b2df85ccc663912c615051ef src/main/python/apache/aurora/config/recipes.py 68b5d252f87a592d4c2f7d52525163829bea2cc9 src/main/python/apache/aurora/config/schema/base.py f12634f103c3eb20e43f37c25d9b0fc3e3d228ec src/main/python/apache/aurora/config/thrift.py 288fb40f65629c8fd4eb7d92c8bf02369237de3b src/main/thrift/org/apache/aurora/gen/api.thrift 8794731f4b3f1033588bdfa33c292e4796319a2a src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java ee9587582bd7c45a446e8afe28930c18a97d2792 src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java d3c90416e48e25d83f28f966b21c018ee2a1ac27 src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java cdd29ea2b6fc92b967571028d299260556e16d42 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 02cd8f712fff3d283abf8e3eb1b4dcab1e762ac2 src/test/python/apache/aurora/client/api/test_scheduler_client.py 1835843f1795b0530874ec561582df17acfbce65 src/test/python/apache/aurora/client/cli/util.py 967b5dcdab76f0581f6d2a518409c30de0800f27 src/test/python/apache/aurora/client/commands/util.py 663f2f4a16113a36826943b7238cad900ae0dcd2 src/test/python/apache/aurora/client/test_config.py 901c3378ed59c44b7e2dea239f186193f1f66355 src/test/python/apache/aurora/config/test_thrift.py fd28313df2cfd5a9c7d00f6d329518b4caabacb2 Diff: https://reviews.apache.org/r/26430/diff/ Testing --- ./build-support/jenkins/build.sh Thanks, Zameer Manji
Re: Review Request 26430: Remove deprecated configuration options.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26430/ --- (Updated Oct. 8, 2014, 3:29 p.m.) Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Brian Wickman. Bugs: AURORA-333 https://issues.apache.org/jira/browse/AURORA-333 Repository: aurora Description (updated) --- This removes several long deprecated configuration options from aurora. The features removed are automatically filling out environment, cron_policy, daemon, health_check_interval_secs, recipes and the PackerObject. Diffs - docs/configuration-reference.md 5166d45ddf95ae5d8afe39dd3b00654ac91857ec src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 865742171c11fbe5cf1469a69dd7258ec1be28c2 src/main/python/apache/aurora/client/binding_helper.py 6d6a06785c6840e4345e304eb4e242682676ac66 src/main/python/apache/aurora/client/config.py e440f587d100ce46b2df85ccc663912c615051ef src/main/python/apache/aurora/config/recipes.py 68b5d252f87a592d4c2f7d52525163829bea2cc9 src/main/python/apache/aurora/config/schema/base.py f12634f103c3eb20e43f37c25d9b0fc3e3d228ec src/main/python/apache/aurora/config/thrift.py 288fb40f65629c8fd4eb7d92c8bf02369237de3b src/main/thrift/org/apache/aurora/gen/api.thrift 8794731f4b3f1033588bdfa33c292e4796319a2a src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java ee9587582bd7c45a446e8afe28930c18a97d2792 src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java d3c90416e48e25d83f28f966b21c018ee2a1ac27 src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java cdd29ea2b6fc92b967571028d299260556e16d42 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 02cd8f712fff3d283abf8e3eb1b4dcab1e762ac2 src/test/python/apache/aurora/client/api/test_scheduler_client.py 1835843f1795b0530874ec561582df17acfbce65 src/test/python/apache/aurora/client/cli/util.py 967b5dcdab76f0581f6d2a518409c30de0800f27 src/test/python/apache/aurora/client/commands/util.py 663f2f4a16113a36826943b7238cad900ae0dcd2 src/test/python/apache/aurora/client/test_config.py 901c3378ed59c44b7e2dea239f186193f1f66355 src/test/python/apache/aurora/config/test_thrift.py fd28313df2cfd5a9c7d00f6d329518b4caabacb2 Diff: https://reviews.apache.org/r/26430/diff/ Testing --- ./build-support/jenkins/build.sh Thanks, Zameer Manji
Re: Review Request 26430: Remove deprecated configuration options.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26430/#review55885 --- Ship it! Ship It! - Kevin Sweeney On Oct. 8, 2014, 3:29 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26430/ --- (Updated Oct. 8, 2014, 3:29 p.m.) Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Brian Wickman. Bugs: AURORA-333 https://issues.apache.org/jira/browse/AURORA-333 Repository: aurora Description --- This removes several long deprecated configuration options from aurora. The features removed are automatically filling out environment, cron_policy, daemon, health_check_interval_secs, recipes and the PackerObject. Diffs - docs/configuration-reference.md 5166d45ddf95ae5d8afe39dd3b00654ac91857ec src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 865742171c11fbe5cf1469a69dd7258ec1be28c2 src/main/python/apache/aurora/client/binding_helper.py 6d6a06785c6840e4345e304eb4e242682676ac66 src/main/python/apache/aurora/client/config.py e440f587d100ce46b2df85ccc663912c615051ef src/main/python/apache/aurora/config/recipes.py 68b5d252f87a592d4c2f7d52525163829bea2cc9 src/main/python/apache/aurora/config/schema/base.py f12634f103c3eb20e43f37c25d9b0fc3e3d228ec src/main/python/apache/aurora/config/thrift.py 288fb40f65629c8fd4eb7d92c8bf02369237de3b src/main/thrift/org/apache/aurora/gen/api.thrift 8794731f4b3f1033588bdfa33c292e4796319a2a src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java ee9587582bd7c45a446e8afe28930c18a97d2792 src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java d3c90416e48e25d83f28f966b21c018ee2a1ac27 src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java cdd29ea2b6fc92b967571028d299260556e16d42 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 02cd8f712fff3d283abf8e3eb1b4dcab1e762ac2 src/test/python/apache/aurora/client/api/test_scheduler_client.py 1835843f1795b0530874ec561582df17acfbce65 src/test/python/apache/aurora/client/cli/util.py 967b5dcdab76f0581f6d2a518409c30de0800f27 src/test/python/apache/aurora/client/commands/util.py 663f2f4a16113a36826943b7238cad900ae0dcd2 src/test/python/apache/aurora/client/test_config.py 901c3378ed59c44b7e2dea239f186193f1f66355 src/test/python/apache/aurora/config/test_thrift.py fd28313df2cfd5a9c7d00f6d329518b4caabacb2 Diff: https://reviews.apache.org/r/26430/diff/ Testing --- ./build-support/jenkins/build.sh Thanks, Zameer Manji
Re: Review Request 26425: Fixing quota checking for updates.
On Oct. 8, 2014, 5:17 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 90 https://reviews.apache.org/r/26425/diff/1/?file=714851#file714851line90 These methods read strangely to me: check the quota of this task config, and check the quota of this update. How about `checkInstanceAddition`, and `checkJobUpdate`? Sure. Done. On Oct. 8, 2014, 5:17 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 148 https://reviews.apache.org/r/26425/diff/1/?file=714851#file714851line148 checkArgument(instances = 0) (technically we could assert 0, but this function should not care if instances == 0) Done. On Oct. 8, 2014, 5:17 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 168 https://reviews.apache.org/r/26425/diff/1/?file=714851#file714851line168 The method names in QuotaInfo make this pretty confusing. It's not obvious why `getQuota()` and `getProdConsumpgion()` are used the way they are. To me this suggests the method names should be re-evaluated to avoid misuse. The getQuota() is exactly what it is. I am not sure I have a better alternative for getProdConsumption() without using something stupidly long. Technically, it is a prod consumption in pure (IJobUpdate absent) or in a adjusted form where a not-yet-saved update is added to the prod mix to simplify handling. I have added javadoc comments for the getQuotaInfo() to hopefully make it clear. On Oct. 8, 2014, 5:17 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 209 https://reviews.apache.org/r/26425/diff/1/?file=714851#file714851line209 I would find this easier if functions were composed rather than combined. e.g.: // Fetch the active production tasks in a role, group by job, compute resources. MapIJobKey, IResourceAggregate getProdTaskResources(String role); // Call getProdTaskResources, combine values. IResourceAggregate getProdConsumption(String role); // Call getProdTaskResources, calculate charge for updated job, combine values. IResourceAggregate getProdConsumptionDuringUpdate(IJobUpdate update); The above would not quite work as the updates must be checked against while the prod consumption is computed to yield the correct value. Hence the combination rather than composition. On Oct. 8, 2014, 5:17 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 317 https://reviews.apache.org/r/26425/diff/1/?file=714851#file714851line317 Avoid encoding type information in the method name. Also, observe Law of Demeter. How about: private static IResourceAggregate toProdResources(IJobUpdateInstructions instructions) Type encoding (Job Update) here is unintentional. Changed the name anyway. On Oct. 8, 2014, 5:17 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 320 https://reviews.apache.org/r/26425/diff/1/?file=714851#file714851line320 This is well-suited for a javadoc. lol, it was actually a javadoc in QuotaUtil. Fixed. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26425/#review55819 --- On Oct. 7, 2014, 10:03 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26425/ --- (Updated Oct. 7, 2014, 10:03 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-802 https://issues.apache.org/jira/browse/AURORA-802 Repository: aurora Description --- Rolling up prod consumption does not work for quota checking during job update intitiation where per-instance filtering is required. Splitting the QuotaManager interface into 2 use cases: - createJob/addInstances - startJobUpdate Reverting the IResourceAggregate abstraction in QuotaManager in favor of task/update objects to simplify handling. Diffs - src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 2442b06f318c8d0cefe60296950baa1b916b42f7 src/main/java/org/apache/aurora/scheduler/quota/QuotaUtil.java d197dd515eb646cb4babadf22e9cf6480a919060 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 5dcae4a6132026504cf02093ad4c68ab521e4ab8 src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java b58c8f363e6e7c72accaf590b2a7cb7bf24275ea
Re: Review Request 26425: Fixing quota checking for updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26425/ --- (Updated Oct. 8, 2014, 10:58 p.m.) Review request for Aurora and Bill Farner. Changes --- CR comments. -kevints per request. Bugs: AURORA-802 https://issues.apache.org/jira/browse/AURORA-802 Repository: aurora Description --- Rolling up prod consumption does not work for quota checking during job update intitiation where per-instance filtering is required. Splitting the QuotaManager interface into 2 use cases: - createJob/addInstances - startJobUpdate Reverting the IResourceAggregate abstraction in QuotaManager in favor of task/update objects to simplify handling. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 2442b06f318c8d0cefe60296950baa1b916b42f7 src/main/java/org/apache/aurora/scheduler/quota/QuotaUtil.java d197dd515eb646cb4babadf22e9cf6480a919060 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 5dcae4a6132026504cf02093ad4c68ab521e4ab8 src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java b58c8f363e6e7c72accaf590b2a7cb7bf24275ea src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 02cd8f712fff3d283abf8e3eb1b4dcab1e762ac2 Diff: https://reviews.apache.org/r/26425/diff/ Testing --- ./gradlew -Pq build Also, tested various update scenarios in vagrant. Thanks, Maxim Khutornenko
Re: Review Request 26383: Health Check Disabler
On Oct. 6, 2014, 10:55 p.m., Joshua Cohen wrote: src/main/python/apache/aurora/executor/common/health_checker.py, lines 157-158 https://reviews.apache.org/r/26383/diff/1/?file=714260#file714260line157 I know you didn't originate this pattern, but it seems odd for these defaults to be repeated on both the HealthChecker and the ThreadedHealthChecker. Perhaps they should be constants, or maybe ThreadedHealthChecker shouldn't have defaults at all? I actually did originate this pattern. The reason why ThreadedHealthChecker has defaults is because some tests don't care about all the parameters to ThreadedHealthChecker. For example, if my test doesn't care about the interval_secs, the defaults allow me to ignore that parameter altogether, and focus on the parameters that I actually care about. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26383/#review55590 --- On Oct. 6, 2014, 9:24 p.m., David Pan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26383/ --- (Updated Oct. 6, 2014, 9:24 p.m.) Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji. Repository: aurora Description --- The health check disabler allows health checks for a job to be snoozed temporarily by touching a snooze file in the job's sandbox. The path of the snooze file and the snooze duration can be set in the HealthCheckConfig. The appropriate unit tests were modified/added. The corresponding JIRA ticket is the following: https://issues.apache.org/jira/browse/AURORA-795 Diffs - docs/configuration-reference.md 5166d45ddf95ae5d8afe39dd3b00654ac91857ec docs/configuration-tutorial.md 67998e9dab6ac429d96d7c0d2df959336b767f32 src/main/python/apache/aurora/config/schema/base.py f12634f103c3eb20e43f37c25d9b0fc3e3d228ec src/main/python/apache/aurora/executor/common/health_checker.py 4980411c847d12655cbb363404707ebd9f0bd163 src/test/python/apache/aurora/executor/common/BUILD c7f7a003c865d479ba6e3cd7b5349322f884f653 src/test/python/apache/aurora/executor/common/test_health_checker.py aa36415fa891fc523a3a376ffeca5d3cd5ceabec Diff: https://reviews.apache.org/r/26383/diff/ Testing --- On vagrant in ~/aurora, I ran ./pants src/test/python/apache/aurora/executor:: Thanks, David Pan
Re: Review Request 26376: Implementing non-prod MTTA/R SLA metrics.
On Oct. 8, 2014, 5:32 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java, line 81 https://reviews.apache.org/r/26376/diff/1/?file=714219#file714219line81 Seems like we're moving towards identical calculations for prod and non-prod tasks. Should we just do that now and avoid the independent groupings? I don't necessarily see prods and non-prods converging any time soon. Doing that now would require more work to exclude non-prods from platform uptime calculations and would add redundant job uptime metrics. I'd rather keep them separate at least for now and see if it ever becomes a trend. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26376/#review55829 --- On Oct. 6, 2014, 7:10 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26376/ --- (Updated Oct. 6, 2014, 7:10 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-774 https://issues.apache.org/jira/browse/AURORA-774 Repository: aurora Description --- Implementing non-prod MTTA/R SLA metrics. Diffs - src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java dca680415426be2bc760875d5774c9e9399ea94b src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java 33b6bbe4b39d516595276128bbfa0686d0bb9cbd src/test/java/org/apache/aurora/scheduler/sla/MetricCalculatorTest.java aeb90bbb822254cbe4691e45092b9581596ad800 src/test/java/org/apache/aurora/scheduler/sla/SlaTestUtil.java 96a04389e6b14c4d1c0bdb4d06abc046e7ea2151 Diff: https://reviews.apache.org/r/26376/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 26464: Blacklist org/apache/aurora/scheduler/http/api/ApiBeta$2 which has no coverage.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26464/ --- Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-819 https://issues.apache.org/jira/browse/AURORA-819 Repository: aurora Description --- The org/apache/aurora/scheduler/http/api/ApiBeta$2 class has no coverage so we blacklist it. Diffs - build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 Diff: https://reviews.apache.org/r/26464/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 26394: Deprecating Identity struct (renaming fields).
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26394/ --- (Updated Oct. 8, 2014, 11:39 p.m.) Review request for Aurora, David McLaughlin, Mark Chu-Carroll, and Bill Farner. Changes --- Rebased. Ping. Bugs: AURORA-84 https://issues.apache.org/jira/browse/AURORA-84 Repository: aurora Description (updated) --- Part 1 of Identity struct deprecation: renaming fields. Diffs (updated) - examples/jobs/hello_world.aurora fc7877c3a60e56e301d9ee1fabd73446afca7236 src/main/java/org/apache/aurora/scheduler/TaskIdGenerator.java 5c75cc8cae53edfa069c85c37ebad34774682081 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java e9f251508257cd7287ff00773e0073a3cd130df8 src/main/java/org/apache/aurora/scheduler/base/JobKeys.java a76c3fac71b35115064fba6644cff0066fd9e630 src/main/java/org/apache/aurora/scheduler/base/Query.java eded7a59eb394748b93d7fbc085a1bdf64b043cc src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 865742171c11fbe5cf1469a69dd7258ec1be28c2 src/main/java/org/apache/aurora/scheduler/http/Utilization.java a0cb7bf56aeb7edd92b25d8d69a739d87452777a src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 8c20ab6f2bebf1d1c0f91fed3f1e48361cdf45d6 src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 37176237fac336413267f3c8bb4e1b9a6255150c src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 5dcae4a6132026504cf02093ad4c68ab521e4ab8 src/main/python/apache/aurora/client/api/instance_watcher.py b390aa8993205f1f6938f8c295e3c16a0bf4df6d src/main/python/apache/aurora/client/api/sla.py b9b64680b15f5395ed6aca681b9b1c30ffe2822c src/main/python/apache/aurora/client/cli/task.py c41484bdc27266443bc4e139e1ebb362a59be0f9 src/main/python/apache/aurora/client/commands/admin.py deee0250f3ba9837feeb92acc654f5b3b68b4e0f src/main/python/apache/aurora/client/commands/core.py 58f419e674f1a9a0ae9da6faa2e39c8167bab597 src/main/python/apache/aurora/client/commands/ssh.py d2b8bf675556b924d3d63b545d036dc48a081486 src/main/python/apache/aurora/config/thrift.py 288fb40f65629c8fd4eb7d92c8bf02369237de3b src/main/python/apache/aurora/executor/aurora_executor.py 2c6423d096656f426a4385f4edef6875ebad7049 src/main/python/apache/aurora/executor/common/announcer.py 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 src/main/python/apache/aurora/executor/thermos_task_runner.py 7b346e253677ee9b42c57782f7f67ff63b6a0083 src/main/resources/scheduler/assets/js/controllers.js 7e9037ee921b009dc2b7c5adcf057bedebb01632 src/main/resources/scheduler/assets/js/services.js b744f375411e09b7f776e4a05ee5961227143439 src/main/thrift/org/apache/aurora/gen/api.thrift 8794731f4b3f1033588bdfa33c292e4796319a2a src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 371ae87f5954fa5f092db1f6d21e2291d7576173 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 606c4434b7158220ccf1403b6deac939021fee31 src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java f2d153f446247032ad9d8d173fb70870dbfdcca1 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/async/RescheduleCalculatorImplTest.java 131bd826dfe47f40f3c27f29c095ed42953e316c src/test/java/org/apache/aurora/scheduler/async/TaskGroupsTest.java efdde15939b2a851e38be53cceab395cc2cd82a1 src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java 53d2c6bb78ad08a84639c1ecd48ba64d17c3f9fc src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 6534329a92bf005223fa8907cbe4a8a3a511e142 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 9970948bace4c0ecbc51d6fc79270d77fb17bf87 src/test/java/org/apache/aurora/scheduler/base/TaskTestUtil.java b8b9c1b8cdf7b641976c583a71fdd9b0c14e6e5f src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java ee9587582bd7c45a446e8afe28930c18a97d2792 src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java f3c7c5bd53df759432beda4fa46db49fd0514b42 src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java d2d3e86bb5acf3402f55188b9ae440412ef14b5a src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 62154045f49c5b23949dc739d735c3e5d3680b89 src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java b58c8f363e6e7c72accaf590b2a7cb7bf24275ea
Re: Review Request 26464: Blacklist org/apache/aurora/scheduler/http/api/ApiBeta$2 which has no coverage.
On Oct. 8, 2014, 11:31 p.m., Bill Farner wrote: Ship It! This is now on master: $ git log -1 --abbrev-commit origin/master commit 18ae905 Author: Zameer Manji zma...@twopensource.com Date: Wed Oct 8 16:33:42 2014 -0700 Blacklist org/apache/aurora/scheduler/http/api/ApiBeta$2 which has no coverage. Bugs closed: AURORA-819 Reviewed at https://reviews.apache.org/r/26464/ - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26464/#review55894 --- On Oct. 8, 2014, 11:09 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26464/ --- (Updated Oct. 8, 2014, 11:09 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-819 https://issues.apache.org/jira/browse/AURORA-819 Repository: aurora Description --- The org/apache/aurora/scheduler/http/api/ApiBeta$2 class has no coverage so we blacklist it. Diffs - build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 Diff: https://reviews.apache.org/r/26464/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 26431: Moving post_drain script execution into host_maintenance.py
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26431/ --- (Updated Oct. 8, 2014, 11:46 p.m.) Review request for Aurora, Joe Smith and Mark Chu-Carroll. Changes --- CR comments. Bugs: AURORA-806 https://issues.apache.org/jira/browse/AURORA-806 Repository: aurora Description --- Moving post_drain script functionality into host_maintenance.py to support per batch execution. Diffs (updated) - src/main/python/apache/aurora/admin/host_maintenance.py 9c2a9f77109791da574e1624d27b6b7096a2678e src/main/python/apache/aurora/client/commands/maintenance.py e465d973e9f764076e18491e1691d44303c0f388 src/test/python/apache/aurora/admin/test_host_maintenance.py 40228df59e43bc6034f2dc651c166a0c4b78aea8 Diff: https://reviews.apache.org/r/26431/diff/ Testing --- ./pants src/test/python:all Thanks, Maxim Khutornenko
Re: Review Request 26431: Moving post_drain script execution into host_maintenance.py
On Oct. 8, 2014, 7:08 p.m., Joe Smith wrote: src/test/python/apache/aurora/client/commands/test_maintenance.py, line 117 https://reviews.apache.org/r/26431/diff/1/?file=715028#file715028line117 can this be a mock that we assert gets called 3 times? (This test is a little bit of an integration test since it goes in and is also testing perform_maintenace, but a full refactor of this test may be too much for now) Sure, reverted it. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26431/#review55859 --- On Oct. 8, 2014, 12:13 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26431/ --- (Updated Oct. 8, 2014, 12:13 a.m.) Review request for Aurora, Joe Smith and Mark Chu-Carroll. Bugs: AURORA-806 https://issues.apache.org/jira/browse/AURORA-806 Repository: aurora Description --- Moving post_drain script functionality into host_maintenance.py to support per batch execution. Diffs - src/main/python/apache/aurora/admin/host_maintenance.py 9c2a9f77109791da574e1624d27b6b7096a2678e src/main/python/apache/aurora/client/commands/maintenance.py e465d973e9f764076e18491e1691d44303c0f388 src/test/python/apache/aurora/admin/test_host_maintenance.py 40228df59e43bc6034f2dc651c166a0c4b78aea8 src/test/python/apache/aurora/client/commands/test_maintenance.py d86aaf677804301fa5ddf1f76dba552f4fafb8c3 Diff: https://reviews.apache.org/r/26431/diff/ Testing --- ./pants src/test/python:all Thanks, Maxim Khutornenko
Re: Review Request 26464: Blacklist org/apache/aurora/scheduler/http/api/ApiBeta$2 which has no coverage.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26464/#review55900 --- This doesn't actually fail when I run it locally. I'm worried that we're just fixing a symptom of the problem, and not the problem itself (i.e. we're going to be adding random classes to the legacy list despite their actually having coverage). - Joshua Cohen On Oct. 8, 2014, 11:09 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26464/ --- (Updated Oct. 8, 2014, 11:09 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-819 https://issues.apache.org/jira/browse/AURORA-819 Repository: aurora Description --- The org/apache/aurora/scheduler/http/api/ApiBeta$2 class has no coverage so we blacklist it. Diffs - build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 Diff: https://reviews.apache.org/r/26464/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 26363: Make the large-update check in the client update command consider instance parameters.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26363/ --- (Updated Oct. 8, 2014, 7:56 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Changes --- rebase Bugs: aurora-792 https://issues.apache.org/jira/browse/aurora-792 Repository: aurora Description --- Make the large-update check in the client update command consider instance parameters. Diffs (updated) - src/main/python/apache/aurora/client/cli/context.py 4e94a4e3017f3b2ace38d6d8ce68c3c778c8cd0e src/main/python/apache/aurora/client/cli/jobs.py 0277cbed4b7eb927d6e5823b4b41d90b366f81d0 src/test/python/apache/aurora/client/cli/BUILD d33e86643a59879c115876c98bd1dc19aa7ae61c src/test/python/apache/aurora/client/cli/test_update.py cff1b6578aec6f5bcc1e610e58b47af233f32b41 Diff: https://reviews.apache.org/r/26363/diff/ Testing --- New unit tests added, all test pass. Thanks, Mark Chu-Carroll
Re: Review Request 26464: Blacklist org/apache/aurora/scheduler/http/api/ApiBeta$2 which has no coverage.
On Oct. 8, 2014, 11:50 p.m., Joshua Cohen wrote: This doesn't actually fail when I run it locally. I'm worried that we're just fixing a symptom of the problem, and not the problem itself (i.e. we're going to be adding random classes to the legacy list despite their actually having coverage). I suspect you might be right. I want to get to the bottom of this, since i think the no-coverage check is really useful. The next time we notice one of these, let's remember to capture `dist/reports/jacoco/test/jacocoTestReport.xml`. That's what the check uses, and it will help us determine if the check is faulty, or jacoco is doing something odd. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26464/#review55900 --- On Oct. 8, 2014, 11:09 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26464/ --- (Updated Oct. 8, 2014, 11:09 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-819 https://issues.apache.org/jira/browse/AURORA-819 Repository: aurora Description --- The org/apache/aurora/scheduler/http/api/ApiBeta$2 class has no coverage so we blacklist it. Diffs - build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 Diff: https://reviews.apache.org/r/26464/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 26394: Deprecating Identity struct (renaming fields).
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26394/#review55907 --- Thanks again for leading this - i'm very happy to see momentum on removing `Identity`. Stepping back - i wonder if we should re-evaluate the way we do field deprecations now that we've established a protocol with JIRA and releases. This might mean we don't need to do the `DEPRECATED` mangling. What do you think? - Bill Farner On Oct. 8, 2014, 11:39 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26394/ --- (Updated Oct. 8, 2014, 11:39 p.m.) Review request for Aurora, David McLaughlin, Mark Chu-Carroll, and Bill Farner. Bugs: AURORA-84 https://issues.apache.org/jira/browse/AURORA-84 Repository: aurora Description --- Part 1 of Identity struct deprecation: renaming fields. Diffs - examples/jobs/hello_world.aurora fc7877c3a60e56e301d9ee1fabd73446afca7236 src/main/java/org/apache/aurora/scheduler/TaskIdGenerator.java 5c75cc8cae53edfa069c85c37ebad34774682081 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java e9f251508257cd7287ff00773e0073a3cd130df8 src/main/java/org/apache/aurora/scheduler/base/JobKeys.java a76c3fac71b35115064fba6644cff0066fd9e630 src/main/java/org/apache/aurora/scheduler/base/Query.java eded7a59eb394748b93d7fbc085a1bdf64b043cc src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 865742171c11fbe5cf1469a69dd7258ec1be28c2 src/main/java/org/apache/aurora/scheduler/http/Utilization.java a0cb7bf56aeb7edd92b25d8d69a739d87452777a src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 8c20ab6f2bebf1d1c0f91fed3f1e48361cdf45d6 src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 37176237fac336413267f3c8bb4e1b9a6255150c src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 5dcae4a6132026504cf02093ad4c68ab521e4ab8 src/main/python/apache/aurora/client/api/instance_watcher.py b390aa8993205f1f6938f8c295e3c16a0bf4df6d src/main/python/apache/aurora/client/api/sla.py b9b64680b15f5395ed6aca681b9b1c30ffe2822c src/main/python/apache/aurora/client/cli/task.py c41484bdc27266443bc4e139e1ebb362a59be0f9 src/main/python/apache/aurora/client/commands/admin.py deee0250f3ba9837feeb92acc654f5b3b68b4e0f src/main/python/apache/aurora/client/commands/core.py 58f419e674f1a9a0ae9da6faa2e39c8167bab597 src/main/python/apache/aurora/client/commands/ssh.py d2b8bf675556b924d3d63b545d036dc48a081486 src/main/python/apache/aurora/config/thrift.py 288fb40f65629c8fd4eb7d92c8bf02369237de3b src/main/python/apache/aurora/executor/aurora_executor.py 2c6423d096656f426a4385f4edef6875ebad7049 src/main/python/apache/aurora/executor/common/announcer.py 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 src/main/python/apache/aurora/executor/thermos_task_runner.py 7b346e253677ee9b42c57782f7f67ff63b6a0083 src/main/resources/scheduler/assets/js/controllers.js 7e9037ee921b009dc2b7c5adcf057bedebb01632 src/main/resources/scheduler/assets/js/services.js b744f375411e09b7f776e4a05ee5961227143439 src/main/thrift/org/apache/aurora/gen/api.thrift 8794731f4b3f1033588bdfa33c292e4796319a2a src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 371ae87f5954fa5f092db1f6d21e2291d7576173 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 606c4434b7158220ccf1403b6deac939021fee31 src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java f2d153f446247032ad9d8d173fb70870dbfdcca1 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/async/RescheduleCalculatorImplTest.java 131bd826dfe47f40f3c27f29c095ed42953e316c src/test/java/org/apache/aurora/scheduler/async/TaskGroupsTest.java efdde15939b2a851e38be53cceab395cc2cd82a1 src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java 53d2c6bb78ad08a84639c1ecd48ba64d17c3f9fc src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 6534329a92bf005223fa8907cbe4a8a3a511e142 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 9970948bace4c0ecbc51d6fc79270d77fb17bf87 src/test/java/org/apache/aurora/scheduler/base/TaskTestUtil.java b8b9c1b8cdf7b641976c583a71fdd9b0c14e6e5f src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
Re: Review Request 26458: Adding wait loop into host_drain status monitoring.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26458/#review55899 --- src/main/python/apache/aurora/admin/host_maintenance.py https://reviews.apache.org/r/26458/#comment96275 Is this provided so external modules can interact with the maintenance? src/main/python/apache/aurora/admin/host_maintenance.py https://reviews.apache.org/r/26458/#comment96274 move out to its own method? src/main/python/apache/aurora/admin/host_maintenance.py https://reviews.apache.org/r/26458/#comment96277 Maybe just raise a normal exception instead? :) (and maybe log which hosts weren't moved?) src/test/python/apache/aurora/admin/test_host_maintenance.py https://reviews.apache.org/r/26458/#comment96279 can you assert on how many times this was called? - Joe Smith On Oct. 8, 2014, 2:40 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26458/ --- (Updated Oct. 8, 2014, 2:40 p.m.) Review request for Aurora, Joe Smith and Brian Wickman. Bugs: AURORA-820 https://issues.apache.org/jira/browse/AURORA-820 Repository: aurora Description --- Throttling status check calls now at a predefined 5 second interval with a max timeout of 5 minutes. Diffs - src/main/python/apache/aurora/admin/host_maintenance.py 9c2a9f77109791da574e1624d27b6b7096a2678e src/test/python/apache/aurora/admin/test_host_maintenance.py 40228df59e43bc6034f2dc651c166a0c4b78aea8 Diff: https://reviews.apache.org/r/26458/diff/ Testing --- ./pants src/test/python:all Thanks, Maxim Khutornenko
Re: Review Request 26458: Adding wait loop into host_drain status monitoring.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26458/#review55909 --- src/main/python/apache/aurora/admin/host_maintenance.py https://reviews.apache.org/r/26458/#comment96280 Will this abort aurora_admin? If so, writing these to the failed set is more desirable, along with continuing. - Tobias Weingartner On Oct. 8, 2014, 9:40 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26458/ --- (Updated Oct. 8, 2014, 9:40 p.m.) Review request for Aurora, Joe Smith and Brian Wickman. Bugs: AURORA-820 https://issues.apache.org/jira/browse/AURORA-820 Repository: aurora Description --- Throttling status check calls now at a predefined 5 second interval with a max timeout of 5 minutes. Diffs - src/main/python/apache/aurora/admin/host_maintenance.py 9c2a9f77109791da574e1624d27b6b7096a2678e src/test/python/apache/aurora/admin/test_host_maintenance.py 40228df59e43bc6034f2dc651c166a0c4b78aea8 Diff: https://reviews.apache.org/r/26458/diff/ Testing --- ./pants src/test/python:all Thanks, Maxim Khutornenko
Re: Review Request 26464: Blacklist org/apache/aurora/scheduler/http/api/ApiBeta$2 which has no coverage.
On Oct. 8, 2014, 11:50 p.m., Joshua Cohen wrote: This doesn't actually fail when I run it locally. I'm worried that we're just fixing a symptom of the problem, and not the problem itself (i.e. we're going to be adding random classes to the legacy list despite their actually having coverage). Bill Farner wrote: I suspect you might be right. I want to get to the bottom of this, since i think the no-coverage check is really useful. The next time we notice one of these, let's remember to capture `dist/reports/jacoco/test/jacocoTestReport.xml`. That's what the check uses, and it will help us determine if the check is faulty, or jacoco is doing something odd. In this case I think it is uncovering an actual case where coverage was lost (because the static asset changes serve the html directly without going through ApiBeta, I'll have a review out shortly to kill that code). That being said, this check never failed for me locally which was super strange. I had to run the tests locally in a debugger and confirm the breakpoint was not hit. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26464/#review55900 --- On Oct. 8, 2014, 11:09 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26464/ --- (Updated Oct. 8, 2014, 11:09 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-819 https://issues.apache.org/jira/browse/AURORA-819 Repository: aurora Description --- The org/apache/aurora/scheduler/http/api/ApiBeta$2 class has no coverage so we blacklist it. Diffs - build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 Diff: https://reviews.apache.org/r/26464/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Review Request 26469: Kill code to serve ApiBeta help pages that's no longer used now that the content is served directly.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26469/ --- Review request for Aurora, Bill Farner and Zameer Manji. Repository: aurora Description --- Kill code to serve ApiBeta help pages that's no longer used now that the content is served directly. Diffs - build.gradle 8f7eed0b58a00f8e5e0521512975087d5788b5b6 src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java eaf63382f689a045f837847736ef24fa75dee874 src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 62154045f49c5b23949dc739d735c3e5d3680b89 Diff: https://reviews.apache.org/r/26469/diff/ Testing --- ./gradlew build -Pq $ curl -I http://192.168.33.7:8081/apibeta/help/method/setQuota.html HTTP/1.1 200 OK Content-Type: text/html Vary: Accept-Encoding Content-Length: 0 Server: Jetty(7.6.15.v20140411) Thanks, Joshua Cohen
Re: Review Request 26464: Blacklist org/apache/aurora/scheduler/http/api/ApiBeta$2 which has no coverage.
On Oct. 8, 2014, 11:50 p.m., Joshua Cohen wrote: This doesn't actually fail when I run it locally. I'm worried that we're just fixing a symptom of the problem, and not the problem itself (i.e. we're going to be adding random classes to the legacy list despite their actually having coverage). Bill Farner wrote: I suspect you might be right. I want to get to the bottom of this, since i think the no-coverage check is really useful. The next time we notice one of these, let's remember to capture `dist/reports/jacoco/test/jacocoTestReport.xml`. That's what the check uses, and it will help us determine if the check is faulty, or jacoco is doing something odd. Joshua Cohen wrote: In this case I think it is uncovering an actual case where coverage was lost (because the static asset changes serve the html directly without going through ApiBeta, I'll have a review out shortly to kill that code). That being said, this check never failed for me locally which was super strange. I had to run the tests locally in a debugger and confirm the breakpoint was not hit. https://reviews.apache.org/r/26469/ - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26464/#review55900 --- On Oct. 8, 2014, 11:09 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26464/ --- (Updated Oct. 8, 2014, 11:09 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-819 https://issues.apache.org/jira/browse/AURORA-819 Repository: aurora Description --- The org/apache/aurora/scheduler/http/api/ApiBeta$2 class has no coverage so we blacklist it. Diffs - build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 Diff: https://reviews.apache.org/r/26464/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 26394: Deprecating Identity struct (renaming fields).
On Oct. 9, 2014, 12:05 a.m., Bill Farner wrote: Thanks again for leading this - i'm very happy to see momentum on removing `Identity`. Stepping back - i wonder if we should re-evaluate the way we do field deprecations now that we've established a protocol with JIRA and releases. This might mean we don't need to do the `DEPRECATED` mangling. What do you think? I still think visual code reminder is quite beneficial in avoiding the use of deprecated fields. The between-release time is just too long to remember what needs to be avoided when coding far away from thrift schema. Besides, it helps validate the concept and will facilitate later removal. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26394/#review55907 --- On Oct. 8, 2014, 11:39 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26394/ --- (Updated Oct. 8, 2014, 11:39 p.m.) Review request for Aurora, David McLaughlin, Mark Chu-Carroll, and Bill Farner. Bugs: AURORA-84 https://issues.apache.org/jira/browse/AURORA-84 Repository: aurora Description --- Part 1 of Identity struct deprecation: renaming fields. Diffs - examples/jobs/hello_world.aurora fc7877c3a60e56e301d9ee1fabd73446afca7236 src/main/java/org/apache/aurora/scheduler/TaskIdGenerator.java 5c75cc8cae53edfa069c85c37ebad34774682081 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java e9f251508257cd7287ff00773e0073a3cd130df8 src/main/java/org/apache/aurora/scheduler/base/JobKeys.java a76c3fac71b35115064fba6644cff0066fd9e630 src/main/java/org/apache/aurora/scheduler/base/Query.java eded7a59eb394748b93d7fbc085a1bdf64b043cc src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 865742171c11fbe5cf1469a69dd7258ec1be28c2 src/main/java/org/apache/aurora/scheduler/http/Utilization.java a0cb7bf56aeb7edd92b25d8d69a739d87452777a src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 8c20ab6f2bebf1d1c0f91fed3f1e48361cdf45d6 src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 37176237fac336413267f3c8bb4e1b9a6255150c src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 5dcae4a6132026504cf02093ad4c68ab521e4ab8 src/main/python/apache/aurora/client/api/instance_watcher.py b390aa8993205f1f6938f8c295e3c16a0bf4df6d src/main/python/apache/aurora/client/api/sla.py b9b64680b15f5395ed6aca681b9b1c30ffe2822c src/main/python/apache/aurora/client/cli/task.py c41484bdc27266443bc4e139e1ebb362a59be0f9 src/main/python/apache/aurora/client/commands/admin.py deee0250f3ba9837feeb92acc654f5b3b68b4e0f src/main/python/apache/aurora/client/commands/core.py 58f419e674f1a9a0ae9da6faa2e39c8167bab597 src/main/python/apache/aurora/client/commands/ssh.py d2b8bf675556b924d3d63b545d036dc48a081486 src/main/python/apache/aurora/config/thrift.py 288fb40f65629c8fd4eb7d92c8bf02369237de3b src/main/python/apache/aurora/executor/aurora_executor.py 2c6423d096656f426a4385f4edef6875ebad7049 src/main/python/apache/aurora/executor/common/announcer.py 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 src/main/python/apache/aurora/executor/thermos_task_runner.py 7b346e253677ee9b42c57782f7f67ff63b6a0083 src/main/resources/scheduler/assets/js/controllers.js 7e9037ee921b009dc2b7c5adcf057bedebb01632 src/main/resources/scheduler/assets/js/services.js b744f375411e09b7f776e4a05ee5961227143439 src/main/thrift/org/apache/aurora/gen/api.thrift 8794731f4b3f1033588bdfa33c292e4796319a2a src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 371ae87f5954fa5f092db1f6d21e2291d7576173 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 606c4434b7158220ccf1403b6deac939021fee31 src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java f2d153f446247032ad9d8d173fb70870dbfdcca1 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/async/RescheduleCalculatorImplTest.java 131bd826dfe47f40f3c27f29c095ed42953e316c src/test/java/org/apache/aurora/scheduler/async/TaskGroupsTest.java efdde15939b2a851e38be53cceab395cc2cd82a1 src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java 53d2c6bb78ad08a84639c1ecd48ba64d17c3f9fc src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java
Re: Review Request 26469: Kill code to serve ApiBeta help pages that's no longer used now that the content is served directly.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26469/#review55916 --- Ship it! Ship It! - Zameer Manji On Oct. 8, 2014, 5:15 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26469/ --- (Updated Oct. 8, 2014, 5:15 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Repository: aurora Description --- Kill code to serve ApiBeta help pages that's no longer used now that the content is served directly. Diffs - build.gradle 8f7eed0b58a00f8e5e0521512975087d5788b5b6 src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java eaf63382f689a045f837847736ef24fa75dee874 src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 62154045f49c5b23949dc739d735c3e5d3680b89 Diff: https://reviews.apache.org/r/26469/diff/ Testing --- ./gradlew build -Pq $ curl -I http://192.168.33.7:8081/apibeta/help/method/setQuota.html HTTP/1.1 200 OK Content-Type: text/html Vary: Accept-Encoding Content-Length: 0 Server: Jetty(7.6.15.v20140411) Thanks, Joshua Cohen
Re: Review Request 26383: Health Check Disabler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26383/ --- (Updated Oct. 9, 2014, 12:43 a.m.) Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji. Changes --- Removed configuration for snooze file and snooze duration. Removed time control. Addressed other comments. Repository: aurora Description --- The health check disabler allows health checks for a job to be snoozed temporarily by touching a snooze file in the job's sandbox. The path of the snooze file and the snooze duration can be set in the HealthCheckConfig. The appropriate unit tests were modified/added. The corresponding JIRA ticket is the following: https://issues.apache.org/jira/browse/AURORA-795 Diffs (updated) - src/main/python/apache/aurora/executor/common/health_checker.py 4980411c847d12655cbb363404707ebd9f0bd163 src/test/python/apache/aurora/executor/common/BUILD c7f7a003c865d479ba6e3cd7b5349322f884f653 src/test/python/apache/aurora/executor/common/test_health_checker.py aa36415fa891fc523a3a376ffeca5d3cd5ceabec Diff: https://reviews.apache.org/r/26383/diff/ Testing --- On vagrant in ~/aurora, I ran ./pants src/test/python/apache/aurora/executor:: Thanks, David Pan
Re: Review Request 26383: Health Check Disabler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26383/ --- (Updated Oct. 9, 2014, 12:46 a.m.) Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji. Repository: aurora Description (updated) --- The health check disabler allows health checks for a job to be snoozed temporarily by touching a snooze file in the job's sandbox. The appropriate unit tests were modified/added. The corresponding JIRA ticket is the following: https://issues.apache.org/jira/browse/AURORA-795 Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 4980411c847d12655cbb363404707ebd9f0bd163 src/test/python/apache/aurora/executor/common/BUILD c7f7a003c865d479ba6e3cd7b5349322f884f653 src/test/python/apache/aurora/executor/common/test_health_checker.py aa36415fa891fc523a3a376ffeca5d3cd5ceabec Diff: https://reviews.apache.org/r/26383/diff/ Testing --- On vagrant in ~/aurora, I ran ./pants src/test/python/apache/aurora/executor:: Thanks, David Pan
Re: Review Request 26383: Health Check Disabler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26383/#review55924 --- src/main/python/apache/aurora/executor/common/health_checker.py https://reviews.apache.org/r/26383/#comment96295 Why add a default value here? src/main/python/apache/aurora/executor/common/health_checker.py https://reviews.apache.org/r/26383/#comment96296 Why add a default value here? src/main/python/apache/aurora/executor/common/health_checker.py https://reviews.apache.org/r/26383/#comment96297 Why add a default here? - Zameer Manji On Oct. 8, 2014, 5:46 p.m., David Pan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26383/ --- (Updated Oct. 8, 2014, 5:46 p.m.) Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji. Repository: aurora Description --- The health check disabler allows health checks for a job to be snoozed temporarily by touching a snooze file in the job's sandbox. The appropriate unit tests were modified/added. The corresponding JIRA ticket is the following: https://issues.apache.org/jira/browse/AURORA-795 Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 4980411c847d12655cbb363404707ebd9f0bd163 src/test/python/apache/aurora/executor/common/BUILD c7f7a003c865d479ba6e3cd7b5349322f884f653 src/test/python/apache/aurora/executor/common/test_health_checker.py aa36415fa891fc523a3a376ffeca5d3cd5ceabec Diff: https://reviews.apache.org/r/26383/diff/ Testing --- On vagrant in ~/aurora, I ran ./pants src/test/python/apache/aurora/executor:: Thanks, David Pan
Re: Review Request 26383: Health Check Disabler
On Oct. 8, 2014, 6:09 p.m., Zameer Manji wrote: src/main/python/apache/aurora/executor/common/health_checker.py, line 41 https://reviews.apache.org/r/26383/diff/2/?file=716291#file716291line41 Why add a default value here? David Pan wrote: The reason why ThreadedHealthChecker has defaults is because some tests for ThreadedHealthChecker don't care about all the parameters to ThreadedHealthChecker. For example, if my test doesn't care about the interval_secs, the defaults allow me to ignore that parameter altogether, and focus on the parameters that I actually care about. It isn't acceptable to alter parameters like that. Please modify your tests to pass in a value instead. - Zameer --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26383/#review55924 --- On Oct. 8, 2014, 5:46 p.m., David Pan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26383/ --- (Updated Oct. 8, 2014, 5:46 p.m.) Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji. Repository: aurora Description --- The health check disabler allows health checks for a job to be snoozed temporarily by touching a snooze file in the job's sandbox. The appropriate unit tests were modified/added. The corresponding JIRA ticket is the following: https://issues.apache.org/jira/browse/AURORA-795 Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 4980411c847d12655cbb363404707ebd9f0bd163 src/test/python/apache/aurora/executor/common/BUILD c7f7a003c865d479ba6e3cd7b5349322f884f653 src/test/python/apache/aurora/executor/common/test_health_checker.py aa36415fa891fc523a3a376ffeca5d3cd5ceabec Diff: https://reviews.apache.org/r/26383/diff/ Testing --- On vagrant in ~/aurora, I ran ./pants src/test/python/apache/aurora/executor:: Thanks, David Pan
Re: Review Request 26383: Health Check Disabler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26383/ --- (Updated Oct. 9, 2014, 1:56 a.m.) Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji. Changes --- Removed default parameters from ThreadedHealthChecker Repository: aurora Description --- The health check disabler allows health checks for a job to be snoozed temporarily by touching a snooze file in the job's sandbox. The appropriate unit tests were modified/added. The corresponding JIRA ticket is the following: https://issues.apache.org/jira/browse/AURORA-795 Diffs (updated) - src/main/python/apache/aurora/executor/common/health_checker.py 4980411c847d12655cbb363404707ebd9f0bd163 src/test/python/apache/aurora/executor/common/BUILD c7f7a003c865d479ba6e3cd7b5349322f884f653 src/test/python/apache/aurora/executor/common/test_health_checker.py aa36415fa891fc523a3a376ffeca5d3cd5ceabec Diff: https://reviews.apache.org/r/26383/diff/ Testing --- On vagrant in ~/aurora, I ran ./pants src/test/python/apache/aurora/executor:: Thanks, David Pan
Review Request 26478: Add a flag to deduplicate storage snapshots
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26478/ --- Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji. Bugs: AURORA-722 https://issues.apache.org/jira/browse/AURORA-722 Repository: aurora Description --- Add a new format for deduplicated storage snapshots. Microbenchmarks show a 10x deduplication ratio on Twitter's production snapshots. This format is backwards-incompatible, so this patch introduces a flag to control its use (defaulting off). This only changes the format used to write to the replicated log (where time is of the essence since all writes are done holding the global storage lock) - the format of backups written to disk is unchanged, as backups don't hold the lock. Diffs - build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 65e986eaa2c4193431ca048425a1ed3ab60f5882 src/main/java/org/apache/aurora/scheduler/storage/log/EntrySerializer.java 7239a6a5eb5479e395e16423c83fdf80a77e5a83 src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java 4b50e2069407dc263b4fc93f1827d3a8836253bf src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java f806297d1d0700155c976743f936b2b8a3a390fb src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 769348e6b8a5c701734afff391b1c77de35222c6 src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/log/StreamManager.java 22db80eaf34fe736fa5a3a9289836c9ac9e59906 src/main/java/org/apache/aurora/scheduler/storage/log/StreamManagerFactory.java 52b2e83efc0c289366b5246aff32553a546b1a70 src/main/java/org/apache/aurora/scheduler/storage/log/StreamManagerImpl.java e5cfbf5cf43bf5bbc38c42fe685a7e9f0d03af2a src/main/thrift/org/apache/aurora/gen/storage.thrift 5350ec945fbe028ee4641683815a068ce00b5efc src/test/java/org/apache/aurora/scheduler/storage/log/LogManagerTest.java 39729b374fe4e383f9b5ada7d016923766df9af7 src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 7a8c3b882633376a1bf6a78616d55aaa7401d13f src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicatorImplTest.java PRE-CREATION Diff: https://reviews.apache.org/r/26478/diff/ Testing --- ./gradlew -Pq build Thanks, Kevin Sweeney
Re: Review Request 26478: Add a flag to deduplicate storage snapshots
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26478/ --- (Updated Oct. 8, 2014, 7:39 p.m.) Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji. Changes --- Reduce visibility of some classes Bugs: AURORA-722 https://issues.apache.org/jira/browse/AURORA-722 Repository: aurora Description --- Add a new format for deduplicated storage snapshots. Microbenchmarks show a 10x deduplication ratio on Twitter's production snapshots. This format is backwards-incompatible, so this patch introduces a flag to control its use (defaulting off). This only changes the format used to write to the replicated log (where time is of the essence since all writes are done holding the global storage lock) - the format of backups written to disk is unchanged, as backups don't hold the lock. Diffs (updated) - build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 65e986eaa2c4193431ca048425a1ed3ab60f5882 src/main/java/org/apache/aurora/scheduler/storage/log/EntrySerializer.java 7239a6a5eb5479e395e16423c83fdf80a77e5a83 src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java 4b50e2069407dc263b4fc93f1827d3a8836253bf src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java f806297d1d0700155c976743f936b2b8a3a390fb src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 769348e6b8a5c701734afff391b1c77de35222c6 src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/log/StreamManager.java 22db80eaf34fe736fa5a3a9289836c9ac9e59906 src/main/java/org/apache/aurora/scheduler/storage/log/StreamManagerImpl.java e5cfbf5cf43bf5bbc38c42fe685a7e9f0d03af2a src/main/thrift/org/apache/aurora/gen/storage.thrift 5350ec945fbe028ee4641683815a068ce00b5efc src/test/java/org/apache/aurora/scheduler/storage/log/LogManagerTest.java 39729b374fe4e383f9b5ada7d016923766df9af7 src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 7a8c3b882633376a1bf6a78616d55aaa7401d13f src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicatorImplTest.java PRE-CREATION Diff: https://reviews.apache.org/r/26478/diff/ Testing --- ./gradlew -Pq build Thanks, Kevin Sweeney
Re: Review Request 26478: Add a flag to deduplicate storage snapshots
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26478/#review55943 --- Ship it! Ship It! - Zameer Manji On Oct. 8, 2014, 7:39 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26478/ --- (Updated Oct. 8, 2014, 7:39 p.m.) Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji. Bugs: AURORA-722 https://issues.apache.org/jira/browse/AURORA-722 Repository: aurora Description --- Add a new format for deduplicated storage snapshots. Microbenchmarks show a 10x deduplication ratio on Twitter's production snapshots. This format is backwards-incompatible, so this patch introduces a flag to control its use (defaulting off). This only changes the format used to write to the replicated log (where time is of the essence since all writes are done holding the global storage lock) - the format of backups written to disk is unchanged, as backups don't hold the lock. Diffs - build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 65e986eaa2c4193431ca048425a1ed3ab60f5882 src/main/java/org/apache/aurora/scheduler/storage/log/EntrySerializer.java 7239a6a5eb5479e395e16423c83fdf80a77e5a83 src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java 4b50e2069407dc263b4fc93f1827d3a8836253bf src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java f806297d1d0700155c976743f936b2b8a3a390fb src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 769348e6b8a5c701734afff391b1c77de35222c6 src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/log/StreamManager.java 22db80eaf34fe736fa5a3a9289836c9ac9e59906 src/main/java/org/apache/aurora/scheduler/storage/log/StreamManagerImpl.java e5cfbf5cf43bf5bbc38c42fe685a7e9f0d03af2a src/main/thrift/org/apache/aurora/gen/storage.thrift 5350ec945fbe028ee4641683815a068ce00b5efc src/test/java/org/apache/aurora/scheduler/storage/log/LogManagerTest.java 39729b374fe4e383f9b5ada7d016923766df9af7 src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 7a8c3b882633376a1bf6a78616d55aaa7401d13f src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicatorImplTest.java PRE-CREATION Diff: https://reviews.apache.org/r/26478/diff/ Testing --- ./gradlew -Pq build Thanks, Kevin Sweeney