Re: Review Request 26430: Remove deprecated configuration options.

2014-10-13 Thread Zameer Manji

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


Pinging Mark and Wickman to this review.

- Zameer Manji


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 26383: Health Check Disabler

2014-10-13 Thread David Pan

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

(Updated Oct. 13, 2014, 6:22 p.m.)


Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji.


Changes
---

Added bug.

(-wfarner)


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


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-13 Thread David Pan

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

(Updated Oct. 13, 2014, 12:27 p.m.)


Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji.


Changes
---

Removed time.


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


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



Re: Review Request 26458: Adding wait loop into host_drain status monitoring.

2014-10-13 Thread Maxim Khutornenko

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


Ping, Joe, Brian.

- Maxim Khutornenko


On Oct. 10, 2014, 10 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26458/
 ---
 
 (Updated Oct. 10, 2014, 10 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 
   src/test/python/apache/aurora/client/commands/test_maintenance.py 
 d86aaf677804301fa5ddf1f76dba552f4fafb8c3 
 
 Diff: https://reviews.apache.org/r/26458/diff/
 
 
 Testing
 ---
 
 ./pants src/test/python:all
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 26383: Health Check Disabler

2014-10-13 Thread David Pan

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

(Updated Oct. 13, 2014, 1:53 p.m.)


Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji.


Changes
---

Change to log.info for Health check snooze file found ...


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


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



Re: Review Request 26662: Move coverage report check to buildSrc/

2014-10-13 Thread Bill Farner

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

(Updated Oct. 13, 2014, 11:03 p.m.)


Review request for Aurora, Joshua Cohen and Kevin Sweeney.


Changes
---

Added license header.


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


Repository: aurora


Description
---

Making good on a TODO from https://reviews.apache.org/r/26574/


Diffs (updated)
-

  .gitignore ad4206d179b46f1269fd58c62de9e6e62f4b8738 
  build.gradle c200746ff9ce2d7ff973c22764d9413bf64636e7 
  buildSrc/src/main/groovy/org/apache/aurora/CoverageReportCheck.groovy 
PRE-CREATION 
  config/legacy_untested_classes.txt PRE-CREATION 

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


Testing
---

./gradlew build -Pq


Thanks,

Bill Farner



Re: Review Request 26662: Move coverage report check to buildSrc/

2014-10-13 Thread Kevin Sweeney

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



build.gradle
https://reviews.apache.org/r/26662/#comment96805

Does this slow down the build at all (you're now doing I/O in configure?)



buildSrc/src/main/groovy/org/apache/aurora/CoverageReportCheck.groovy
https://reviews.apache.org/r/26662/#comment96807

typo in analyzes



buildSrc/src/main/groovy/org/apache/aurora/CoverageReportCheck.groovy
https://reviews.apache.org/r/26662/#comment96801

I think you want to throw a GradleException here and below



buildSrc/src/main/groovy/org/apache/aurora/CoverageReportCheck.groovy
https://reviews.apache.org/r/26662/#comment96804

typo in anonymous


- Kevin Sweeney


On Oct. 13, 2014, 4:03 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26662/
 ---
 
 (Updated Oct. 13, 2014, 4:03 p.m.)
 
 
 Review request for Aurora, Joshua Cohen and Kevin Sweeney.
 
 
 Bugs: AURORA-833
 https://issues.apache.org/jira/browse/AURORA-833
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Making good on a TODO from https://reviews.apache.org/r/26574/
 
 
 Diffs
 -
 
   .gitignore ad4206d179b46f1269fd58c62de9e6e62f4b8738 
   build.gradle c200746ff9ce2d7ff973c22764d9413bf64636e7 
   buildSrc/src/main/groovy/org/apache/aurora/CoverageReportCheck.groovy 
 PRE-CREATION 
   config/legacy_untested_classes.txt PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/26662/diff/
 
 
 Testing
 ---
 
 ./gradlew build -Pq
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 26662: Move coverage report check to buildSrc/

2014-10-13 Thread Bill Farner


 On Oct. 13, 2014, 11:23 p.m., Joshua Cohen wrote:
  buildSrc/src/main/groovy/org/apache/aurora/CoverageReportCheck.groovy, line 
  53
  https://reviews.apache.org/r/26662/diff/2/?file=719791#file719791line53
 
  I'm not super familiar w/ groovy, any reason why we can't just inline 
  the computeCoverage(...) call here (and below)?

This was a nuance in the way the assertion error would show - with inlining, 
you wouldn't see the actual value for some reason.  So you would see 0.9 
instead of 0.89  0.9.


- Bill


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


On Oct. 13, 2014, 11:03 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26662/
 ---
 
 (Updated Oct. 13, 2014, 11:03 p.m.)
 
 
 Review request for Aurora, Joshua Cohen and Kevin Sweeney.
 
 
 Bugs: AURORA-833
 https://issues.apache.org/jira/browse/AURORA-833
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Making good on a TODO from https://reviews.apache.org/r/26574/
 
 
 Diffs
 -
 
   .gitignore ad4206d179b46f1269fd58c62de9e6e62f4b8738 
   build.gradle c200746ff9ce2d7ff973c22764d9413bf64636e7 
   buildSrc/src/main/groovy/org/apache/aurora/CoverageReportCheck.groovy 
 PRE-CREATION 
   config/legacy_untested_classes.txt PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/26662/diff/
 
 
 Testing
 ---
 
 ./gradlew build -Pq
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 26662: Move coverage report check to buildSrc/

2014-10-13 Thread Bill Farner


 On Oct. 13, 2014, 11:23 p.m., Joshua Cohen wrote:
  buildSrc/src/main/groovy/org/apache/aurora/CoverageReportCheck.groovy, line 
  53
  https://reviews.apache.org/r/26662/diff/2/?file=719791#file719791line53
 
  I'm not super familiar w/ groovy, any reason why we can't just inline 
  the computeCoverage(...) call here (and below)?
 
 Bill Farner wrote:
 This was a nuance in the way the assertion error would show - with 
 inlining, you wouldn't see the actual value for some reason.  So you would 
 see 0.9 instead of 0.89  0.9.

I've gone ahead and inlined now, though, since in retrospect i don't think we 
really care about the absolute value; and it's easy enough to get from the 
coverage report HTML.


- Bill


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


On Oct. 13, 2014, 11:03 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26662/
 ---
 
 (Updated Oct. 13, 2014, 11:03 p.m.)
 
 
 Review request for Aurora, Joshua Cohen and Kevin Sweeney.
 
 
 Bugs: AURORA-833
 https://issues.apache.org/jira/browse/AURORA-833
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Making good on a TODO from https://reviews.apache.org/r/26574/
 
 
 Diffs
 -
 
   .gitignore ad4206d179b46f1269fd58c62de9e6e62f4b8738 
   build.gradle c200746ff9ce2d7ff973c22764d9413bf64636e7 
   buildSrc/src/main/groovy/org/apache/aurora/CoverageReportCheck.groovy 
 PRE-CREATION 
   config/legacy_untested_classes.txt PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/26662/diff/
 
 
 Testing
 ---
 
 ./gradlew build -Pq
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 26662: Move coverage report check to buildSrc/

2014-10-13 Thread Kevin Sweeney


 On Oct. 13, 2014, 4:23 p.m., Joshua Cohen wrote:
  buildSrc/src/main/groovy/org/apache/aurora/CoverageReportCheck.groovy, line 
  53
  https://reviews.apache.org/r/26662/diff/2/?file=719791#file719791line53
 
  I'm not super familiar w/ groovy, any reason why we can't just inline 
  the computeCoverage(...) call here (and below)?
 
 Bill Farner wrote:
 This was a nuance in the way the assertion error would show - with 
 inlining, you wouldn't see the actual value for some reason.  So you would 
 see 0.9 instead of 0.89  0.9.
 
 Bill Farner wrote:
 I've gone ahead and inlined now, though, since in retrospect i don't 
 think we really care about the absolute value; and it's easy enough to get 
 from the coverage report HTML.

Technically you could write it in Java :)


- Kevin


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


On Oct. 13, 2014, 4:36 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26662/
 ---
 
 (Updated Oct. 13, 2014, 4:36 p.m.)
 
 
 Review request for Aurora, Joshua Cohen and Kevin Sweeney.
 
 
 Bugs: AURORA-833
 https://issues.apache.org/jira/browse/AURORA-833
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Making good on a TODO from https://reviews.apache.org/r/26574/
 
 
 Diffs
 -
 
   .gitignore ad4206d179b46f1269fd58c62de9e6e62f4b8738 
   build.gradle c200746ff9ce2d7ff973c22764d9413bf64636e7 
   buildSrc/src/main/groovy/org/apache/aurora/CoverageReportCheck.groovy 
 PRE-CREATION 
   config/legacy_untested_classes.txt PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/26662/diff/
 
 
 Testing
 ---
 
 ./gradlew build -Pq
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 26662: Move coverage report check to buildSrc/

2014-10-13 Thread Bill Farner

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

(Updated Oct. 13, 2014, 11:36 p.m.)


Review request for Aurora, Joshua Cohen and Kevin Sweeney.


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


Repository: aurora


Description
---

Making good on a TODO from https://reviews.apache.org/r/26574/


Diffs (updated)
-

  .gitignore ad4206d179b46f1269fd58c62de9e6e62f4b8738 
  build.gradle c200746ff9ce2d7ff973c22764d9413bf64636e7 
  buildSrc/src/main/groovy/org/apache/aurora/CoverageReportCheck.groovy 
PRE-CREATION 
  config/legacy_untested_classes.txt PRE-CREATION 

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


Testing
---

./gradlew build -Pq


Thanks,

Bill Farner