Re: Review Request 49578: Close `PathChildrenCache` before its framework.

2016-07-04 Thread Stephan Erb

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


Ship it!




Trusting your analysis, the patch looks sane to me.

- Stephan Erb


On July 4, 2016, 1:02 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49578/
> ---
> 
> (Updated July 4, 2016, 1:02 a.m.)
> 
> 
> Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1729
> https://issues.apache.org/jira/browse/AURORA-1729
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously these lifecycles were modeled as independent when, in fact,
> a `CuratorFramework`'s clients must be closed befor it is closed to
> prevent errors in the clients from attempting to use a closed
> `CuratorFramework`.
> 
> The proof that closing was always safe already existed in
> `CuratorServiceGroupMonitorTest::testExceptionalLifecycle`, but this
> safety is now documented and more explicitly tested.
> 
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
>   | 15 ++-
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java
>  | 10 ++
>  
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  | 10 +-
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
>  2656662837ecfdd2addb0d67dd28e54ed6d05330 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java
>  9d8b7bdd49b2e0f907f532825fd79f9e8854a650 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  16692056ffb97e6bfcc8c80c8f4faecc7ae16c62 
> 
> Diff: https://reviews.apache.org/r/49578/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq clean build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>



Re: Review Request 49578: Close `PathChildrenCache` before its framework.

2016-07-03 Thread Aurora ReviewBot

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


Ship it!




Master (1103571) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On July 3, 2016, 11:02 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49578/
> ---
> 
> (Updated July 3, 2016, 11:02 p.m.)
> 
> 
> Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1729
> https://issues.apache.org/jira/browse/AURORA-1729
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously these lifecycles were modeled as independent when, in fact,
> a `CuratorFramework`'s clients must be closed befor it is closed to
> prevent errors in the clients from attempting to use a closed
> `CuratorFramework`.
> 
> The proof that closing was always safe already existed in
> `CuratorServiceGroupMonitorTest::testExceptionalLifecycle`, but this
> safety is now documented and more explicitly tested.
> 
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
>   | 15 ++-
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java
>  | 10 ++
>  
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  | 10 +-
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
>  2656662837ecfdd2addb0d67dd28e54ed6d05330 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java
>  9d8b7bdd49b2e0f907f532825fd79f9e8854a650 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  16692056ffb97e6bfcc8c80c8f4faecc7ae16c62 
> 
> Diff: https://reviews.apache.org/r/49578/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq clean build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>



Re: Review Request 49578: Close `PathChildrenCache` before its framework.

2016-07-03 Thread John Sirois

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



@ReviewBot retry

- John Sirois


On July 3, 2016, 5:02 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49578/
> ---
> 
> (Updated July 3, 2016, 5:02 p.m.)
> 
> 
> Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1729
> https://issues.apache.org/jira/browse/AURORA-1729
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously these lifecycles were modeled as independent when, in fact,
> a `CuratorFramework`'s clients must be closed befor it is closed to
> prevent errors in the clients from attempting to use a closed
> `CuratorFramework`.
> 
> The proof that closing was always safe already existed in
> `CuratorServiceGroupMonitorTest::testExceptionalLifecycle`, but this
> safety is now documented and more explicitly tested.
> 
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
>   | 15 ++-
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java
>  | 10 ++
>  
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  | 10 +-
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
>  2656662837ecfdd2addb0d67dd28e54ed6d05330 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java
>  9d8b7bdd49b2e0f907f532825fd79f9e8854a650 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  16692056ffb97e6bfcc8c80c8f4faecc7ae16c62 
> 
> Diff: https://reviews.apache.org/r/49578/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq clean build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>



Re: Review Request 49578: Close `PathChildrenCache` before its framework.

2016-07-03 Thread Aurora ReviewBot

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



Master (1103571) is red with this patch.
  ./build-support/jenkins/build.sh

1015 tests completed, 31 failed, 2 skipped
:test FAILED
:jacocoTestReport
Coverage report generated: 
file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/jacoco/test/html/index.html
:analyzeReport
Instruction coverage is 0.8707329713165544, but must be greater than 0.89
Branch coverage is 0.242044358728, but must be greater than 0.835
:analyzeReport FAILED

FAILURE: Build completed with 2 failures.

1: Task failed with an exception.
---
* What went wrong:
Execution failed for task ':test'.
> There were failing tests. See the report at: 
> file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/tests/index.html

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.
==

2: Task failed with an exception.
---
* What went wrong:
Execution failed for task ':analyzeReport'.
> Test coverage missing for 
> org/apache/aurora/scheduler/http/LeaderRedirectFilter
  Test coverage missing for org/apache/aurora/scheduler/http/HttpStatsFilter
  Test coverage missing for org/apache/aurora/scheduler/http/HttpStatsFilter$1
  Test coverage missing for 
org/apache/aurora/scheduler/http/HttpStatsFilter$ResponseWithStatus
  Test coverage missing for 
org/apache/aurora/scheduler/http/api/security/ShiroKerberosAuthenticationFilter
  Test coverage missing for 
org/apache/aurora/scheduler/http/api/security/ShiroKerberosPermissiveAuthenticationFilter
  Test coverage missing for org/apache/aurora/scheduler/http/api/ApiBeta

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.
==

BUILD FAILED

Total time: 5 mins 1.164 secs


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On July 3, 2016, 11:02 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49578/
> ---
> 
> (Updated July 3, 2016, 11:02 p.m.)
> 
> 
> Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1729
> https://issues.apache.org/jira/browse/AURORA-1729
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously these lifecycles were modeled as independent when, in fact,
> a `CuratorFramework`'s clients must be closed befor it is closed to
> prevent errors in the clients from attempting to use a closed
> `CuratorFramework`.
> 
> The proof that closing was always safe already existed in
> `CuratorServiceGroupMonitorTest::testExceptionalLifecycle`, but this
> safety is now documented and more explicitly tested.
> 
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
>   | 15 ++-
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java
>  | 10 ++
>  
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  | 10 +-
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
>  2656662837ecfdd2addb0d67dd28e54ed6d05330 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java
>  9d8b7bdd49b2e0f907f532825fd79f9e8854a650 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  16692056ffb97e6bfcc8c80c8f4faecc7ae16c62 
> 
> Diff: https://reviews.apache.org/r/49578/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq clean build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>



Re: Review Request 49578: Close `PathChildrenCache` before its framework.

2016-07-03 Thread John Sirois


> On July 3, 2016, 5:16 p.m., Aurora ReviewBot wrote:
> > Master (1103571) is red with this patch.
> >   ./build-support/jenkins/build.sh
> > 
> > 1015 tests completed, 31 failed, 2 skipped
> > :test FAILED
> > :jacocoTestReport
> > Coverage report generated: 
> > file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/jacoco/test/html/index.html
> > :analyzeReport
> > Instruction coverage is 0.8706879069872243, but must be greater than 0.89
> > Branch coverage is 0.7767598842815815, but must be greater than 0.835
> > :analyzeReport FAILED
> > 
> > FAILURE: Build completed with 2 failures.
> > 
> > 1: Task failed with an exception.
> > ---
> > * What went wrong:
> > Execution failed for task ':test'.
> > > There were failing tests. See the report at: 
> > > file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/tests/index.html
> > 
> > * Try:
> > Run with --stacktrace option to get the stack trace. Run with --info or 
> > --debug option to get more log output.
> > ==
> > 
> > 2: Task failed with an exception.
> > ---
> > * What went wrong:
> > Execution failed for task ':analyzeReport'.
> > > Test coverage missing for 
> > > org/apache/aurora/scheduler/http/LeaderRedirectFilter
> >   Test coverage missing for org/apache/aurora/scheduler/http/HttpStatsFilter
> >   Test coverage missing for 
> > org/apache/aurora/scheduler/http/HttpStatsFilter$1
> >   Test coverage missing for 
> > org/apache/aurora/scheduler/http/HttpStatsFilter$ResponseWithStatus
> >   Test coverage missing for 
> > org/apache/aurora/scheduler/http/api/security/ShiroKerberosAuthenticationFilter
> >   Test coverage missing for 
> > org/apache/aurora/scheduler/http/api/security/ShiroKerberosPermissiveAuthenticationFilter
> >   Test coverage missing for org/apache/aurora/scheduler/http/api/ApiBeta
> > 
> > * Try:
> > Run with --stacktrace option to get the stack trace. Run with --info or 
> > --debug option to get more log output.
> > ==
> > 
> > BUILD FAILED
> > 
> > Total time: 5 mins 14.307 secs
> > 
> > 
> > I will refresh this build result if you post a review containing 
> > "@ReviewBot retry"

Another instance of multiple:
```
org.apache.aurora.scheduler.http.api.security.ShiroKerberosPermissiveAuthenticationFilterTest
 > testInterceptsUnauthenticatedException FAILED
com.sun.jersey.api.client.ClientHandlerException at 
ShiroKerberosPermissiveAuthenticationFilterTest.java:90
Caused by: java.net.ConnectException at 
ShiroKerberosPermissiveAuthenticationFilterTest.java:90
java.lang.AssertionError
```


- John


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


On July 3, 2016, 5:02 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49578/
> ---
> 
> (Updated July 3, 2016, 5:02 p.m.)
> 
> 
> Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1729
> https://issues.apache.org/jira/browse/AURORA-1729
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously these lifecycles were modeled as independent when, in fact,
> a `CuratorFramework`'s clients must be closed befor it is closed to
> prevent errors in the clients from attempting to use a closed
> `CuratorFramework`.
> 
> The proof that closing was always safe already existed in
> `CuratorServiceGroupMonitorTest::testExceptionalLifecycle`, but this
> safety is now documented and more explicitly tested.
> 
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
>   | 15 ++-
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java
>  | 10 ++
>  
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  | 10 +-
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
>  2656662837ecfdd2addb0d67dd28e54ed6d05330 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java
>  9d8b7bdd49b2e0f907f532825fd79f9e8854a650 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  16692056ffb97e6bfcc8c80c8f4faecc7ae16c62 
> 
> Diff: https://reviews.apache.org/r/49578/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq clean build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>



Re: Review Request 49578: Close `PathChildrenCache` before its framework.

2016-07-03 Thread John Sirois

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



@ReviewBot retry

- John Sirois


On July 3, 2016, 5:02 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49578/
> ---
> 
> (Updated July 3, 2016, 5:02 p.m.)
> 
> 
> Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1729
> https://issues.apache.org/jira/browse/AURORA-1729
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously these lifecycles were modeled as independent when, in fact,
> a `CuratorFramework`'s clients must be closed befor it is closed to
> prevent errors in the clients from attempting to use a closed
> `CuratorFramework`.
> 
> The proof that closing was always safe already existed in
> `CuratorServiceGroupMonitorTest::testExceptionalLifecycle`, but this
> safety is now documented and more explicitly tested.
> 
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
>   | 15 ++-
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java
>  | 10 ++
>  
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  | 10 +-
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
>  2656662837ecfdd2addb0d67dd28e54ed6d05330 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java
>  9d8b7bdd49b2e0f907f532825fd79f9e8854a650 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  16692056ffb97e6bfcc8c80c8f4faecc7ae16c62 
> 
> Diff: https://reviews.apache.org/r/49578/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq clean build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>



Re: Review Request 49578: Close `PathChildrenCache` before its framework.

2016-07-03 Thread Aurora ReviewBot

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



Master (1103571) is red with this patch.
  ./build-support/jenkins/build.sh

1015 tests completed, 31 failed, 2 skipped
:test FAILED
:jacocoTestReport
Coverage report generated: 
file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/jacoco/test/html/index.html
:analyzeReport
Instruction coverage is 0.8706879069872243, but must be greater than 0.89
Branch coverage is 0.7767598842815815, but must be greater than 0.835
:analyzeReport FAILED

FAILURE: Build completed with 2 failures.

1: Task failed with an exception.
---
* What went wrong:
Execution failed for task ':test'.
> There were failing tests. See the report at: 
> file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/tests/index.html

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.
==

2: Task failed with an exception.
---
* What went wrong:
Execution failed for task ':analyzeReport'.
> Test coverage missing for 
> org/apache/aurora/scheduler/http/LeaderRedirectFilter
  Test coverage missing for org/apache/aurora/scheduler/http/HttpStatsFilter
  Test coverage missing for org/apache/aurora/scheduler/http/HttpStatsFilter$1
  Test coverage missing for 
org/apache/aurora/scheduler/http/HttpStatsFilter$ResponseWithStatus
  Test coverage missing for 
org/apache/aurora/scheduler/http/api/security/ShiroKerberosAuthenticationFilter
  Test coverage missing for 
org/apache/aurora/scheduler/http/api/security/ShiroKerberosPermissiveAuthenticationFilter
  Test coverage missing for org/apache/aurora/scheduler/http/api/ApiBeta

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.
==

BUILD FAILED

Total time: 5 mins 14.307 secs


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On July 3, 2016, 11:02 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49578/
> ---
> 
> (Updated July 3, 2016, 11:02 p.m.)
> 
> 
> Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1729
> https://issues.apache.org/jira/browse/AURORA-1729
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously these lifecycles were modeled as independent when, in fact,
> a `CuratorFramework`'s clients must be closed befor it is closed to
> prevent errors in the clients from attempting to use a closed
> `CuratorFramework`.
> 
> The proof that closing was always safe already existed in
> `CuratorServiceGroupMonitorTest::testExceptionalLifecycle`, but this
> safety is now documented and more explicitly tested.
> 
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
>   | 15 ++-
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java
>  | 10 ++
>  
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  | 10 +-
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
>  2656662837ecfdd2addb0d67dd28e54ed6d05330 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java
>  9d8b7bdd49b2e0f907f532825fd79f9e8854a650 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  16692056ffb97e6bfcc8c80c8f4faecc7ae16c62 
> 
> Diff: https://reviews.apache.org/r/49578/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq clean build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>



Re: Review Request 49578: Close `PathChildrenCache` before its framework.

2016-07-03 Thread John Sirois

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



NB: I included both Bill and Zameer on this review since they were involved in 
its predecessor reviews despite the fact one or both may not be interested in 
reviewing at present.  As such, I won't block on their feedback, but will 
happily take it if offered.

- John Sirois


On July 3, 2016, 5:02 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49578/
> ---
> 
> (Updated July 3, 2016, 5:02 p.m.)
> 
> 
> Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1729
> https://issues.apache.org/jira/browse/AURORA-1729
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously these lifecycles were modeled as independent when, in fact,
> a `CuratorFramework`'s clients must be closed befor it is closed to
> prevent errors in the clients from attempting to use a closed
> `CuratorFramework`.
> 
> The proof that closing was always safe already existed in
> `CuratorServiceGroupMonitorTest::testExceptionalLifecycle`, but this
> safety is now documented and more explicitly tested.
> 
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
>   | 15 ++-
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java
>  | 10 ++
>  
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  | 10 +-
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
>  2656662837ecfdd2addb0d67dd28e54ed6d05330 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java
>  9d8b7bdd49b2e0f907f532825fd79f9e8854a650 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  16692056ffb97e6bfcc8c80c8f4faecc7ae16c62 
> 
> Diff: https://reviews.apache.org/r/49578/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq clean build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>