Re: Review Request 26328: Improve handling of unknown errors in the aurora client.

2014-10-08 Thread Mark Chu-Carroll

---
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.

2014-10-08 Thread David McLaughlin


 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.

2014-10-08 Thread Bill Farner


 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.

2014-10-08 Thread Bill Farner


 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.

2014-10-08 Thread Mark Chu-Carroll

---
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.

2014-10-08 Thread Bill Farner

---
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

2014-10-08 Thread Bill Farner

---
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.

2014-10-08 Thread Bill Farner


 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

2014-10-08 Thread Bill Farner


 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.

2014-10-08 Thread Maxim Khutornenko


 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

2014-10-08 Thread Bill Farner


 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.

2014-10-08 Thread Bill Farner

---
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

2014-10-08 Thread Joshua Cohen
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

2014-10-08 Thread David McLaughlin

---
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.

2014-10-08 Thread Joshua Cohen


 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

2014-10-08 Thread Kevin Sweeney

---
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

2014-10-08 Thread Kevin Sweeney

---
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

2014-10-08 Thread David McLaughlin

---
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

2014-10-08 Thread David McLaughlin

---
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

2014-10-08 Thread Kevin Sweeney


 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

2014-10-08 Thread Brian Wickman

---
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.

2014-10-08 Thread Mark Chu-Carroll

---
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.

2014-10-08 Thread Zameer Manji

---
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.

2014-10-08 Thread Zameer Manji

---
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

2014-10-08 Thread Joe Smith

---
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.

2014-10-08 Thread Bill Farner


 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

2014-10-08 Thread Bill Farner


 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.

2014-10-08 Thread Kevin Sweeney

---
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.

2014-10-08 Thread Kevin Sweeney

---
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.

2014-10-08 Thread David McLaughlin

---
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.

2014-10-08 Thread Joshua Cohen


 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

2014-10-08 Thread Kevin Sweeney


 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

2014-10-08 Thread Bill Farner

---
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

2014-10-08 Thread Kevin Sweeney

---
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.

2014-10-08 Thread Brian Wickman

---
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.

2014-10-08 Thread Joe Smith

---
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

2014-10-08 Thread Joe Smith

---
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.

2014-10-08 Thread Joe Smith

---
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.

2014-10-08 Thread Maxim Khutornenko

---
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.

2014-10-08 Thread Kevin Sweeney

---
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.

2014-10-08 Thread Kevin Sweeney

---
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.

2014-10-08 Thread Kevin Sweeney

---
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.

2014-10-08 Thread Brian Wickman

---
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.

2014-10-08 Thread Bill Farner

---
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.

2014-10-08 Thread Maxim Khutornenko

---
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.

2014-10-08 Thread Joshua Cohen


 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.

2014-10-08 Thread Zameer Manji

---
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.

2014-10-08 Thread Zameer Manji

---
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.

2014-10-08 Thread Kevin Sweeney

---
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.

2014-10-08 Thread Maxim Khutornenko


 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.

2014-10-08 Thread Maxim Khutornenko

---
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

2014-10-08 Thread David Pan


 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.

2014-10-08 Thread Maxim Khutornenko


 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.

2014-10-08 Thread Zameer Manji

---
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).

2014-10-08 Thread Maxim Khutornenko

---
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.

2014-10-08 Thread Bill Farner


 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

2014-10-08 Thread Maxim Khutornenko

---
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

2014-10-08 Thread Maxim Khutornenko


 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.

2014-10-08 Thread Joshua Cohen

---
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.

2014-10-08 Thread Mark Chu-Carroll

---
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.

2014-10-08 Thread Bill Farner


 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).

2014-10-08 Thread Bill Farner

---
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.

2014-10-08 Thread Joe Smith

---
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.

2014-10-08 Thread Tobias Weingartner

---
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.

2014-10-08 Thread Joshua Cohen


 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.

2014-10-08 Thread Joshua Cohen

---
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.

2014-10-08 Thread Joshua Cohen


 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).

2014-10-08 Thread Maxim Khutornenko


 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.

2014-10-08 Thread Zameer Manji

---
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

2014-10-08 Thread David Pan

---
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

2014-10-08 Thread David Pan

---
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

2014-10-08 Thread Zameer Manji

---
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

2014-10-08 Thread Zameer Manji


 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

2014-10-08 Thread David Pan

---
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

2014-10-08 Thread Kevin Sweeney

---
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

2014-10-08 Thread Kevin Sweeney

---
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

2014-10-08 Thread Zameer Manji

---
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