date:20160104

Review Request 41893: Fixup missing commons compile dep.

2016-01-04 Thread John Sirois


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

Review request for Aurora and Zameer Manji.


Repository: aurora


Description
---

I get compile errors when working on
  src/main/java/org/apache/aurora/common/zookeeper/testing.
I'm not sure why CI and others do not.

 build.gradle | 5 +
 1 file changed, 5 insertions(+)


Diffs
-

  build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 

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


Testing
---

Not green locally due to ZK issues from
https://git1-us-west.apache.org/repos/asf/aurora/repo?p=aurora.git;a=commit;h=8706a781968912c68688284d9d3813d34ce45bf7,
but the CI script gets further on my machine with the junit
dep added


Thanks,

John Sirois

Re: Review Request 41804: Proposal - shim interfaces to preface args system overhaul.

2016-01-04 Thread Maxim Khutornenko


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


Has the high level plan been discussed on dev list before or is it the first 
time this proposal is shaped up? It would be great to have an official design 
summary or at least a ticket capturing the goals of this refactoring.

- Maxim Khutornenko


On Dec. 31, 2015, 12:03 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41804/
> ---
> 
> (Updated Dec. 31, 2015, 12:03 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This begins to define a proposed replacement args API, from the perspective 
> of the code consuming args.  Args will be defined in interfaces, which the 
> eventual arg system will be responsible for implementing on the fly 
> (mechanism TBD).  So while what is seen here is a large net increase in code, 
> the eventual conclusion will be roughly equivalent in terms of lines of code 
> in `Module`s.
> 
> There are a few goals with the replacement:
> - sidestep current development hurdles we have encountered with the args 
> system (intellij/gradle not working nicely with apt)
> - leverage a well-maintained third-party argument parsing library
> - encourage better testability of Module classes by always injecting all args
> - enable user-friendly features like logical option groups for better 
> help/usage output
> - stretch: enable alternative configuration inputs like a configuration file 
> or environment variables
> 
> The rough plan of action is as follows (if the proposal looks good):
> 1. repeat this patch for all other `@CmdLine` declaration sites (28 files) 
> 2. introduce a 'boot' `Injector` that is loaded with bindings for these 
> params implementations (i.e. centralize the `new ExecutorModuleParams() { .. 
> }` boilerplate you see here
> 3. replace the args backend
>   (a) remove `@CmdLine Arg<>` declarations, moving help text to annotations 
> on interface methods
>   (b) implement a system to inject proxies that implement params classes 
> based on arg values, and binds them for injection
> 
> Given that this is a large multi-stage effort, i may opt to implement it on a 
> branch and land it all at once in a stream of commits to avoid 
> churn/confusion in the meantime.  Open to thoughts on that.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  949c299bdbc54f976db994266fb97f3099256f13 
> 
> Diff: https://reviews.apache.org/r/41804/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41809: Allow custom announce path

2016-01-04 Thread Aurora ReviewBot


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

Ship it!


Master (8706a78) 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 Jan. 4, 2016, 9:28 p.m., Kunal Thakar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41809/
> ---
> 
> (Updated Jan. 4, 2016, 9:28 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Allow custom announce path
> 
> Related ticket: https://issues.apache.org/jira/browse/AURORA-1569
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md cf63cfa487c531b56f5238a4768e1e9e9ac1c30b 
>   src/main/python/apache/aurora/config/schema/base.py 
> 69182c320579bf5100e5646904c8ce1336afdebb 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 4e9b0278d31654c3c51ff149bc4167aaf94041ec 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> dda76f018f472d7d8228459eb89f4c5daf9df26d 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 46ad784987b626e10e235831707540b807158955 
> 
> Diff: https://reviews.apache.org/r/41809/diff/
> 
> 
> Testing
> ---
> 
> Ran tests ./build-support/jenkins/build.sh
> Manually tested the changes in vagrant.
> 
> 
> Thanks,
> 
> Kunal Thakar
> 
>

Re: Review Request 41809: Allow custom announce path

2016-01-04 Thread Stephan Erb


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



src/main/python/apache/aurora/config/schema/base.py (line 102)


This comment does somewhat imply that I can use a path containing 
variables. Also the rest of the documentation leaves this somewhat unclear.


- Stephan Erb


On Jan. 4, 2016, 10:28 p.m., Kunal Thakar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41809/
> ---
> 
> (Updated Jan. 4, 2016, 10:28 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Allow custom announce path
> 
> Related ticket: https://issues.apache.org/jira/browse/AURORA-1569
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md cf63cfa487c531b56f5238a4768e1e9e9ac1c30b 
>   src/main/python/apache/aurora/config/schema/base.py 
> 69182c320579bf5100e5646904c8ce1336afdebb 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 4e9b0278d31654c3c51ff149bc4167aaf94041ec 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> dda76f018f472d7d8228459eb89f4c5daf9df26d 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 46ad784987b626e10e235831707540b807158955 
> 
> Diff: https://reviews.apache.org/r/41809/diff/
> 
> 
> Testing
> ---
> 
> Ran tests ./build-support/jenkins/build.sh
> Manually tested the changes in vagrant.
> 
> 
> Thanks,
> 
> Kunal Thakar
> 
>

Re: Review Request 41897: Upgrade to the latest zk point release.

2016-01-04 Thread John Sirois


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

(Updated Jan. 4, 2016, 4:44 p.m.)


Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Bill Farner.


Changes
---

Fixup NEWS to reflect upgraded upgrade.

 NEWS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


Repository: aurora


Description
---

This is enabled by https://reviews.apache.org/r/41895/ which
isolated the kerberos configuration.

 build.gradle | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)


Diffs (updated)
-

  NEWS c0c454d080023a2e2796022f957a4d47bdb87b41 
  build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 

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


Testing
---

Both `./build-support/jenkins/build.sh` and
`./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` ran
green locally.


Thanks,

John Sirois

Re: Review Request 41897: Upgrade to the latest zk point release.

2016-01-04 Thread Bill Farner


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

Ship it!


Ship It!

- Bill Farner


On Jan. 4, 2016, 3:44 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41897/
> ---
> 
> (Updated Jan. 4, 2016, 3:44 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is enabled by https://reviews.apache.org/r/41895/ which
> isolated the kerberos configuration.
> 
>  build.gradle | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> 
> Diffs
> -
> 
>   NEWS c0c454d080023a2e2796022f957a4d47bdb87b41 
>   build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 
> 
> Diff: https://reviews.apache.org/r/41897/diff/
> 
> 
> Testing
> ---
> 
> Both `./build-support/jenkins/build.sh` and
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` ran
> green locally.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Review Request 41895: Fix Kerberos5ShiroRealmModule: use dedicated jaas config.

2016-01-04 Thread John Sirois


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

Review request for Aurora, Bill Farner and Zameer Manji.


Repository: aurora


Description
---

This fix to Kerberos initialization that moves away from setting a
system property to instead use a ConfigFile object directly.
This prevents using the default LoginContext internals and entering
into races with other components (notably zookeeper).

 
src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java
 | 19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)


Diffs
-

  
src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java
 09d8db4cdaa6e9320bc2e8bb455adf16c4ec64f9 

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


Testing
---

I now have `./build-support/jenkins/build.sh` and
`./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` green
locally.

Prior to this last fix (see depends-on chain for 2 others), got:
```
org.apache.aurora.scheduler.app.SchedulerIT > testLaunch FAILED

java.io.IOException: Could not find a 'Server' entry in this configuration: 
Server cannot start.
at 
org.apache.zookeeper.server.auth.SaslServerCallbackHandler.(SaslServerCallbackHandler.java:53)
at 
org.apache.zookeeper.server.NIOServerCnxnFactory.configure(NIOServerCnxnFactory.java:96)
at 
org.apache.aurora.common.zookeeper.testing.ZooKeeperTestServer.startNetwork(ZooKeeperTestServer.java:81)
at 
org.apache.aurora.common.zookeeper.testing.BaseZooKeeperTest.setUp(BaseZooKeeperTest.java:64)
...
```


Thanks,

John Sirois

Re: Review Request 41893: Fixup missing commons compile dep.

2016-01-04 Thread Bill Farner


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

Ship it!


Ship It!

- Bill Farner


On Jan. 4, 2016, 4:17 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41893/
> ---
> 
> (Updated Jan. 4, 2016, 4:17 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> I get compile errors when working on
>   src/main/java/org/apache/aurora/common/zookeeper/testing.
> I'm not sure why CI and others do not.
> 
>  build.gradle | 5 +
>  1 file changed, 5 insertions(+)
> 
> 
> Diffs
> -
> 
>   build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 
> 
> Diff: https://reviews.apache.org/r/41893/diff/
> 
> 
> Testing
> ---
> 
> Not green locally due to ZK issues from
> https://git1-us-west.apache.org/repos/asf/aurora/repo?p=aurora.git;a=commit;h=8706a781968912c68688284d9d3813d34ce45bf7,
> but the CI script gets further on my machine with the junit
> dep added
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41897: Upgrade to the latest zk point release.

2016-01-04 Thread John Sirois



> On Jan. 4, 2016, 4:17 p.m., Stephan Erb wrote:
> > News file needs updating too :-)

Good call - change coming as well as an updated reviews list.


- John


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


On Jan. 4, 2016, 4:02 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41897/
> ---
> 
> (Updated Jan. 4, 2016, 4:02 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is enabled by https://reviews.apache.org/r/41895/ which
> isolated the kerberos configuration.
> 
>  build.gradle | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> 
> Diffs
> -
> 
>   build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 
> 
> Diff: https://reviews.apache.org/r/41897/diff/
> 
> 
> Testing
> ---
> 
> Both `./build-support/jenkins/build.sh` and
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` ran
> green locally.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41899: Upgrade to pants 0.0.66.

2016-01-04 Thread Maxim Khutornenko


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

Ship it!


Ship It!

- Maxim Khutornenko


On Jan. 4, 2016, 11:34 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41899/
> ---
> 
> (Updated Jan. 4, 2016, 11:34 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The changelog can be read here:
>   http://pantsbuild.github.io/changelog.html
> 
> Of note for aurora is the ability to kill BUILD.tools.
> 
> Additionally, add the pants generated .pids/ dir to .gitignore and
> normalize all directory ignore to omit the redundant *.
> 
>  .gitignore  | 29 +++--
>  BUILD.tools | 18 --
>  pants.ini   |  2 +-
>  3 files changed, 16 insertions(+), 33 deletions(-)
> 
> 
> Diffs
> -
> 
>   .gitignore 12eff83938329f12ecd0a63043d8f34a608b7e94 
>   BUILD.tools 2e237c43c47fda0771106cbf5a8cccb0b8147710 
>   pants.ini 579d86cd2e02ea3e1a7add9cdd8291a6dc9669ec 
> 
> Diff: https://reviews.apache.org/r/41899/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./build-support/jenkins/build.sh && 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41809: Allow custom announce path

2016-01-04 Thread Kunal Thakar


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

(Updated Jan. 4, 2016, 9:26 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

Added a parameter to the executor to allow custom announce paths.

@ReviewBot retry


Repository: aurora


Description
---

Allow custom announce path

Related ticket: https://issues.apache.org/jira/browse/AURORA-1569


Diffs (updated)
-

  docs/configuration-reference.md cf63cfa487c531b56f5238a4768e1e9e9ac1c30b 
  src/main/python/apache/aurora/config/schema/base.py 
69182c320579bf5100e5646904c8ce1336afdebb 
  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
4e9b0278d31654c3c51ff149bc4167aaf94041ec 
  src/main/python/apache/aurora/executor/common/announcer.py 
dda76f018f472d7d8228459eb89f4c5daf9df26d 
  src/test/python/apache/aurora/executor/common/test_announcer.py 
46ad784987b626e10e235831707540b807158955 

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


Testing
---

Ran unit tests ./pants test src/test/python/apache/aurora:all
Manually tested the changes in vagrant.


Thanks,

Kunal Thakar

Review Request 41899: Upgrade to pants 0.0.66.

2016-01-04 Thread John Sirois


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

Review request for Aurora, Joshua Cohen and Maxim Khutornenko.


Repository: aurora


Description
---

The changelog can be read here:
  http://pantsbuild.github.io/changelog.html

Of note for aurora is the ability to kill BUILD.tools.

Additionally, add the pants generated .pids/ dir to .gitignore and
normalize all directory ignore to omit the redundant *.

 .gitignore  | 29 +++--
 BUILD.tools | 18 --
 pants.ini   |  2 +-
 3 files changed, 16 insertions(+), 33 deletions(-)


Diffs
-

  .gitignore 12eff83938329f12ecd0a63043d8f34a608b7e94 
  BUILD.tools 2e237c43c47fda0771106cbf5a8cccb0b8147710 
  pants.ini 579d86cd2e02ea3e1a7add9cdd8291a6dc9669ec 

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


Testing
---

Locally green:
```
./build-support/jenkins/build.sh && 
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
```


Thanks,

John Sirois

Re: Review Request 41895: Fix Kerberos5ShiroRealmModule: use dedicated jaas config.

2016-01-04 Thread Bill Farner


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

Ship it!


Ship It!

- Bill Farner


On Jan. 4, 2016, 2:08 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41895/
> ---
> 
> (Updated Jan. 4, 2016, 2:08 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This fix to Kerberos initialization that moves away from setting a
> system property to instead use a ConfigFile object directly.
> This prevents using the default LoginContext internals and entering
> into races with other components (notably zookeeper).
> 
>  
> src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java
>  | 19 +++
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java
>  09d8db4cdaa6e9320bc2e8bb455adf16c4ec64f9 
> 
> Diff: https://reviews.apache.org/r/41895/diff/
> 
> 
> Testing
> ---
> 
> I now have `./build-support/jenkins/build.sh` and
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` green
> locally.
> 
> Prior to this last fix (see depends-on chain for 2 others), got:
> ```
> org.apache.aurora.scheduler.app.SchedulerIT > testLaunch FAILED
> 
> java.io.IOException: Could not find a 'Server' entry in this configuration: 
> Server cannot start.
>   at 
> org.apache.zookeeper.server.auth.SaslServerCallbackHandler.(SaslServerCallbackHandler.java:53)
>   at 
> org.apache.zookeeper.server.NIOServerCnxnFactory.configure(NIOServerCnxnFactory.java:96)
>   at 
> org.apache.aurora.common.zookeeper.testing.ZooKeeperTestServer.startNetwork(ZooKeeperTestServer.java:81)
>   at 
> org.apache.aurora.common.zookeeper.testing.BaseZooKeeperTest.setUp(BaseZooKeeperTest.java:64)
> ...
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41895: Fix Kerberos5ShiroRealmModule: use dedicated jaas config.

2016-01-04 Thread Bill Farner



> On Jan. 4, 2016, 4:14 p.m., Bill Farner wrote:
> > Ship It!

Preempting Zameer's review since we also had Maxim chime in.


- Bill


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


On Jan. 4, 2016, 2:08 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41895/
> ---
> 
> (Updated Jan. 4, 2016, 2:08 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This fix to Kerberos initialization that moves away from setting a
> system property to instead use a ConfigFile object directly.
> This prevents using the default LoginContext internals and entering
> into races with other components (notably zookeeper).
> 
>  
> src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java
>  | 19 +++
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java
>  09d8db4cdaa6e9320bc2e8bb455adf16c4ec64f9 
> 
> Diff: https://reviews.apache.org/r/41895/diff/
> 
> 
> Testing
> ---
> 
> I now have `./build-support/jenkins/build.sh` and
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` green
> locally.
> 
> Prior to this last fix (see depends-on chain for 2 others), got:
> ```
> org.apache.aurora.scheduler.app.SchedulerIT > testLaunch FAILED
> 
> java.io.IOException: Could not find a 'Server' entry in this configuration: 
> Server cannot start.
>   at 
> org.apache.zookeeper.server.auth.SaslServerCallbackHandler.(SaslServerCallbackHandler.java:53)
>   at 
> org.apache.zookeeper.server.NIOServerCnxnFactory.configure(NIOServerCnxnFactory.java:96)
>   at 
> org.apache.aurora.common.zookeeper.testing.ZooKeeperTestServer.startNetwork(ZooKeeperTestServer.java:81)
>   at 
> org.apache.aurora.common.zookeeper.testing.BaseZooKeeperTest.setUp(BaseZooKeeperTest.java:64)
> ...
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41894: Fixup ZooKeeperTestServer restartNetowrk.

2016-01-04 Thread Aurora ReviewBot


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

Ship it!


Master (8706a78) 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 Jan. 4, 2016, 9:30 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41894/
> ---
> 
> (Updated Jan. 4, 2016, 9:30 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> With the upgrade to 3.4.2 and the switch from `NIOServerCnxn.Factory` to
> `NIOServerCnxnFactory` came a change in the server internals.  Calling
> `connectionFactory.startup(zooKeeperServer)` now eventually calls
> `ZooKeeperServer.startup` which calls `SessionTrackerImpl.start`.  This
> last is a problem since `SessionTrackerImpl` is a `Thread` and so throws
> `IllegalThreadStateException`.
> 
>  
> commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
>  | 28 
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
>  54b0a09355c1467362c2ad788e217dd5f52de522 
> 
> Diff: https://reviews.apache.org/r/41894/diff/
> 
> 
> Testing
> ---
> 
> Not green locally due to ZK/krb interaction issues from
> https://git1-us-west.apache.org/repos/asf/aurora/repo?p=aurora.git;a=commit;h=8706a781968912c68688284d9d3813d34ce45bf7,
> but the CI script gets further on my machine with this fix.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41897: Upgrade to the latest zk point release.

2016-01-04 Thread Maxim Khutornenko


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

Ship it!


Ship It!

- Maxim Khutornenko


On Jan. 4, 2016, 11:02 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41897/
> ---
> 
> (Updated Jan. 4, 2016, 11:02 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is enabled by https://reviews.apache.org/r/41895/ which
> isolated the kerberos configuration.
> 
>  build.gradle | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> 
> Diffs
> -
> 
>   build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 
> 
> Diff: https://reviews.apache.org/r/41897/diff/
> 
> 
> Testing
> ---
> 
> Both `./build-support/jenkins/build.sh` and
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` ran
> green locally.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41897: Upgrade to the latest zk point release.

2016-01-04 Thread Aurora ReviewBot


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


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

 
src/test/python/apache/thermos/observer/test_detector.py::test_observer_task_detector_standard_transitions
 [32mPASSED[0m
 
src/test/python/apache/thermos/observer/test_detector.py::test_observer_task_detector_nonstandard_transitions
 [32mPASSED[0m
 
src/test/python/apache/thermos/observer/test_task_observer.py::TaskObserverTest::test_run_loop
 [31mFAILED[0m
 
  FAILURES 
 _ TaskObserverTest.test_run_loop _
 
 self = 
 
 [1mdef test_run_loop(self):[0m
 [1m  """Test observer run loop."""[0m
 [1m  mock_task_detector = 
create_autospec(spec=ObserverTaskDetector)[0m
 [1m  with patch([0m
 [1m  
"apache.thermos.observer.task_observer.ObserverTaskDetector",[0m
 [1m  return_value=mock_task_detector) as 
mock_detector:[0m
 [1mwith patch('threading._Event.wait') as 
mock_wait:[0m
 [1m[0m
 [1m  run_count = 3[0m
 [1m  interval = 15[0m
 [1m  observer = TaskObserver(mock_detector, 
interval=Amount(interval, Time.SECONDS))[0m
 [1m  observer.start()[0m
 [1m  while len(mock_wait.mock_calls) < 
run_count:[0m
 [1mpass[0m
 [1m[0m
 [1m  observer.stop()[0m
 [1m[0m
 [1m> assert len(mock_task_detector.mock_calls) >= 
run_count[0m
 [1m[31mE AssertionError: assert 1 >= 3[0m
 [1m[31mE  +  where 1 = len([call.refresh()])[0m
 [1m[31mE  +where [call.refresh()] = 
.mock_calls[0m
 
 
src/test/python/apache/thermos/observer/test_task_observer.py:42: AssertionError
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/src.test.python.apache.thermos.observer.observer.xml
 
 [1m[31m=== 1 failed, 3 passed in 0.25 seconds 
===[0m
 
FAILURE


23:20:37 04:05   [complete][31m
   FAILURE[0m


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

- Aurora ReviewBot


On Jan. 4, 2016, 11:02 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41897/
> ---
> 
> (Updated Jan. 4, 2016, 11:02 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is enabled by https://reviews.apache.org/r/41895/ which
> isolated the kerberos configuration.
> 
>  build.gradle | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> 
> Diffs
> -
> 
>   build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 
> 
> Diff: https://reviews.apache.org/r/41897/diff/
> 
> 
> Testing
> ---
> 
> Both `./build-support/jenkins/build.sh` and
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` ran
> green locally.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41894: Fixup ZooKeeperTestServer restartNetwork.

2016-01-04 Thread Bill Farner


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

Ship it!


Ship It!

- Bill Farner


On Jan. 4, 2016, 3:04 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41894/
> ---
> 
> (Updated Jan. 4, 2016, 3:04 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> With the upgrade to 3.4.2 and the switch from `NIOServerCnxn.Factory` to
> `NIOServerCnxnFactory` came a change in the server internals.  Calling
> `connectionFactory.startup(zooKeeperServer)` now eventually calls
> `ZooKeeperServer.startup` which calls `SessionTrackerImpl.start`.  This
> last is a problem since `SessionTrackerImpl` is a `Thread` and so throws
> `IllegalThreadStateException`.
> 
>  
> commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
>  | 28 
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
>  54b0a09355c1467362c2ad788e217dd5f52de522 
> 
> Diff: https://reviews.apache.org/r/41894/diff/
> 
> 
> Testing
> ---
> 
> Not green locally due to ZK/krb interaction issues from
> https://git1-us-west.apache.org/repos/asf/aurora/repo?p=aurora.git;a=commit;h=8706a781968912c68688284d9d3813d34ce45bf7,
> but the CI script gets further on my machine with this fix.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41893: Fixup missing commons compile dep.

2016-01-04 Thread John Sirois


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


Showing my work:
```
$ git grep org.junit | grep src/main/java
commons/src/main/java/org/apache/aurora/common/testing/TearDownTestCase.java:import
 org.junit.After;
commons/src/main/java/org/apache/aurora/common/testing/easymock/EasyMockTest.java:import
 org.junit.Before;
commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperTest.java:import
 org.junit.Before;
commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperTest.java:import
 org.junit.Rule;
commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperTest.java:import
 org.junit.rules.TemporaryFolder;
```

- John Sirois


On Jan. 4, 2016, 2:16 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41893/
> ---
> 
> (Updated Jan. 4, 2016, 2:16 p.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> I get compile errors when working on
>   src/main/java/org/apache/aurora/common/zookeeper/testing.
> I'm not sure why CI and others do not.
> 
>  build.gradle | 5 +
>  1 file changed, 5 insertions(+)
> 
> 
> Diffs
> -
> 
>   build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 
> 
> Diff: https://reviews.apache.org/r/41893/diff/
> 
> 
> Testing
> ---
> 
> Not green locally due to ZK issues from
> https://git1-us-west.apache.org/repos/asf/aurora/repo?p=aurora.git;a=commit;h=8706a781968912c68688284d9d3813d34ce45bf7,
> but the CI script gets further on my machine with the junit
> dep added
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41895: Fix Kerberos5ShiroRealmModule: use dedicated jaas config.

2016-01-04 Thread Aurora ReviewBot


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


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

However, it appears that it might lack test coverage.

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

- Aurora ReviewBot


On Jan. 4, 2016, 10:08 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41895/
> ---
> 
> (Updated Jan. 4, 2016, 10:08 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This fix to Kerberos initialization that moves away from setting a
> system property to instead use a ConfigFile object directly.
> This prevents using the default LoginContext internals and entering
> into races with other components (notably zookeeper).
> 
>  
> src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java
>  | 19 +++
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java
>  09d8db4cdaa6e9320bc2e8bb455adf16c4ec64f9 
> 
> Diff: https://reviews.apache.org/r/41895/diff/
> 
> 
> Testing
> ---
> 
> I now have `./build-support/jenkins/build.sh` and
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` green
> locally.
> 
> Prior to this last fix (see depends-on chain for 2 others), got:
> ```
> org.apache.aurora.scheduler.app.SchedulerIT > testLaunch FAILED
> 
> java.io.IOException: Could not find a 'Server' entry in this configuration: 
> Server cannot start.
>   at 
> org.apache.zookeeper.server.auth.SaslServerCallbackHandler.(SaslServerCallbackHandler.java:53)
>   at 
> org.apache.zookeeper.server.NIOServerCnxnFactory.configure(NIOServerCnxnFactory.java:96)
>   at 
> org.apache.aurora.common.zookeeper.testing.ZooKeeperTestServer.startNetwork(ZooKeeperTestServer.java:81)
>   at 
> org.apache.aurora.common.zookeeper.testing.BaseZooKeeperTest.setUp(BaseZooKeeperTest.java:64)
> ...
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41897: Upgrade to the latest zk point release.

2016-01-04 Thread John Sirois



> On Jan. 4, 2016, 4:17 p.m., Stephan Erb wrote:
> > News file needs updating too :-)
> 
> John Sirois wrote:
> Good call - change coming as well as an updated reviews list.

Fixed.


- John


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


On Jan. 4, 2016, 4:02 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41897/
> ---
> 
> (Updated Jan. 4, 2016, 4:02 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is enabled by https://reviews.apache.org/r/41895/ which
> isolated the kerberos configuration.
> 
>  build.gradle | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> 
> Diffs
> -
> 
>   build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 
> 
> Diff: https://reviews.apache.org/r/41897/diff/
> 
> 
> Testing
> ---
> 
> Both `./build-support/jenkins/build.sh` and
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` ran
> green locally.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41899: Upgrade to pants 0.0.66.

2016-01-04 Thread Aurora ReviewBot


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

Ship it!


Master (8706a78) 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 Jan. 4, 2016, 11:34 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41899/
> ---
> 
> (Updated Jan. 4, 2016, 11:34 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The changelog can be read here:
>   http://pantsbuild.github.io/changelog.html
> 
> Of note for aurora is the ability to kill BUILD.tools.
> 
> Additionally, add the pants generated .pids/ dir to .gitignore and
> normalize all directory ignore to omit the redundant *.
> 
>  .gitignore  | 29 +++--
>  BUILD.tools | 18 --
>  pants.ini   |  2 +-
>  3 files changed, 16 insertions(+), 33 deletions(-)
> 
> 
> Diffs
> -
> 
>   .gitignore 12eff83938329f12ecd0a63043d8f34a608b7e94 
>   BUILD.tools 2e237c43c47fda0771106cbf5a8cccb0b8147710 
>   pants.ini 579d86cd2e02ea3e1a7add9cdd8291a6dc9669ec 
> 
> Diff: https://reviews.apache.org/r/41899/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./build-support/jenkins/build.sh && 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41809: Allow custom announce path

2016-01-04 Thread Kunal Thakar


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

(Updated Jan. 4, 2016, 9:28 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Repository: aurora


Description
---

Allow custom announce path

Related ticket: https://issues.apache.org/jira/browse/AURORA-1569


Diffs
-

  docs/configuration-reference.md cf63cfa487c531b56f5238a4768e1e9e9ac1c30b 
  src/main/python/apache/aurora/config/schema/base.py 
69182c320579bf5100e5646904c8ce1336afdebb 
  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
4e9b0278d31654c3c51ff149bc4167aaf94041ec 
  src/main/python/apache/aurora/executor/common/announcer.py 
dda76f018f472d7d8228459eb89f4c5daf9df26d 
  src/test/python/apache/aurora/executor/common/test_announcer.py 
46ad784987b626e10e235831707540b807158955 

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


Testing (updated)
---

Ran tests ./build-support/jenkins/build.sh
Manually tested the changes in vagrant.


Thanks,

Kunal Thakar

Re: Review Request 41894: Fixup ZooKeeperTestServer restartNetwork.

2016-01-04 Thread John Sirois


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

(Updated Jan. 4, 2016, 4:04 p.m.)


Review request for Aurora and Bill Farner.


Summary (updated)
-

Fixup ZooKeeperTestServer restartNetwork.


Repository: aurora


Description
---

With the upgrade to 3.4.2 and the switch from `NIOServerCnxn.Factory` to
`NIOServerCnxnFactory` came a change in the server internals.  Calling
`connectionFactory.startup(zooKeeperServer)` now eventually calls
`ZooKeeperServer.startup` which calls `SessionTrackerImpl.start`.  This
last is a problem since `SessionTrackerImpl` is a `Thread` and so throws
`IllegalThreadStateException`.

 
commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
 | 28 
 1 file changed, 16 insertions(+), 12 deletions(-)


Diffs
-

  
commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
 54b0a09355c1467362c2ad788e217dd5f52de522 

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


Testing
---

Not green locally due to ZK/krb interaction issues from
https://git1-us-west.apache.org/repos/asf/aurora/repo?p=aurora.git;a=commit;h=8706a781968912c68688284d9d3813d34ce45bf7,
but the CI script gets further on my machine with this fix.


Thanks,

John Sirois

Re: Review Request 41897: Upgrade to the latest zk point release.

2016-01-04 Thread Stephan Erb


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


News file needs updating too :-)

- Stephan Erb


On Jan. 5, 2016, 12:02 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41897/
> ---
> 
> (Updated Jan. 5, 2016, 12:02 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is enabled by https://reviews.apache.org/r/41895/ which
> isolated the kerberos configuration.
> 
>  build.gradle | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> 
> Diffs
> -
> 
>   build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 
> 
> Diff: https://reviews.apache.org/r/41897/diff/
> 
> 
> Testing
> ---
> 
> Both `./build-support/jenkins/build.sh` and
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` ran
> green locally.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41897: Upgrade to the latest zk point release.

2016-01-04 Thread John Sirois



> On Jan. 4, 2016, 4:20 p.m., Aurora ReviewBot wrote:
> > Master (8706a78) is red with this patch.
> >   ./build-support/jenkins/build.sh
> > 
> >  
> > src/test/python/apache/thermos/observer/test_detector.py::test_observer_task_detector_standard_transitions
> >  [32mPASSED[0m
> >  
> > src/test/python/apache/thermos/observer/test_detector.py::test_observer_task_detector_nonstandard_transitions
> >  [32mPASSED[0m
> >  
> > src/test/python/apache/thermos/observer/test_task_observer.py::TaskObserverTest::test_run_loop
> >  [31mFAILED[0m
> >  
> >   FAILURES 
> >  _ TaskObserverTest.test_run_loop _
> >  
> >  self =  > testMethod=test_run_loop>
> >  
> >  [1mdef test_run_loop(self):[0m
> >  [1m  """Test observer run loop."""[0m
> >  [1m  mock_task_detector = 
> > create_autospec(spec=ObserverTaskDetector)[0m
> >  [1m  with patch([0m
> >  [1m  
> > "apache.thermos.observer.task_observer.ObserverTaskDetector",[0m
> >  [1m  return_value=mock_task_detector) as 
> > mock_detector:[0m
> >  [1mwith patch('threading._Event.wait') as 
> > mock_wait:[0m
> >  [1m[0m
> >  [1m  run_count = 3[0m
> >  [1m  interval = 15[0m
> >  [1m  observer = TaskObserver(mock_detector, 
> > interval=Amount(interval, Time.SECONDS))[0m
> >  [1m  observer.start()[0m
> >  [1m  while len(mock_wait.mock_calls) < 
> > run_count:[0m
> >  [1mpass[0m
> >  [1m[0m
> >  [1m  observer.stop()[0m
> >  [1m[0m
> >  [1m> assert 
> > len(mock_task_detector.mock_calls) >= run_count[0m
> >  [1m[31mE AssertionError: assert 1 >= 3[0m
> >  [1m[31mE  +  where 1 = 
> > len([call.refresh()])[0m
> >  [1m[31mE  +where [call.refresh()] = 
> > .mock_calls[0m
> >  
> >  
> > src/test/python/apache/thermos/observer/test_task_observer.py:42: 
> > AssertionError
> >   generated xml file: 
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/src.test.python.apache.thermos.observer.observer.xml
> >  
> >  [1m[31m=== 1 failed, 3 passed in 0.25 seconds 
> > ===[0m
> >  
> > FAILURE
> > 
> > 
> > 23:20:37 04:05   [complete][31m
> >FAILURE[0m
> > 
> > 
> > I will refresh this build result if you post a review containing 
> > "@ReviewBot retry"
> 
> John Sirois wrote:
> I found no jira issue but will file shortly: Anyone recall if this is 
> known flaky?

Filed https://issues.apache.org/jira/browse/AURORA-1570


- John


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


On Jan. 4, 2016, 4:44 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41897/
> ---
> 
> (Updated Jan. 4, 2016, 4:44 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is enabled by https://reviews.apache.org/r/41895/ which
> isolated the kerberos configuration.
> 
>  build.gradle | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> 
> Diffs
> -
> 
>   NEWS c0c454d080023a2e2796022f957a4d47bdb87b41 
>   build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 
> 
> Diff: https://reviews.apache.org/r/41897/diff/
> 
> 
> Testing
> ---
> 
> Both `./build-support/jenkins/build.sh` and
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` ran
> green locally.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41894: Fixup ZooKeeperTestServer restartNetwork.

2016-01-04 Thread John Sirois


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


"Fixup missing commons compile dep." < not the right RB.

- John Sirois


On Jan. 4, 2016, 4:04 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41894/
> ---
> 
> (Updated Jan. 4, 2016, 4:04 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> With the upgrade to 3.4.2 and the switch from `NIOServerCnxn.Factory` to
> `NIOServerCnxnFactory` came a change in the server internals.  Calling
> `connectionFactory.startup(zooKeeperServer)` now eventually calls
> `ZooKeeperServer.startup` which calls `SessionTrackerImpl.start`.  This
> last is a problem since `SessionTrackerImpl` is a `Thread` and so throws
> `IllegalThreadStateException`.
> 
>  
> commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
>  | 28 
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
>  54b0a09355c1467362c2ad788e217dd5f52de522 
> 
> Diff: https://reviews.apache.org/r/41894/diff/
> 
> 
> Testing
> ---
> 
> Not green locally due to ZK/krb interaction issues from
> https://git1-us-west.apache.org/repos/asf/aurora/repo?p=aurora.git;a=commit;h=8706a781968912c68688284d9d3813d34ce45bf7,
> but the CI script gets further on my machine with this fix.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41915: Fixup TaskObserverTest to respect thread memory models.

2016-01-04 Thread Maxim Khutornenko


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

Ship it!



src/main/python/apache/thermos/observer/task_observer.py (line 72)


We usually put this default initialization directly into the arg list, 
e.g:'stop_event=threading.Event()'.


- Maxim Khutornenko


On Jan. 5, 2016, 3:02 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41915/
> ---
> 
> (Updated Jan. 5, 2016, 3:02 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1570
> https://issues.apache.org/jira/browse/AURORA-1570
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously, a mock threading.Event was waited on in one thread
> and the count of waits was read in another thread.  Most thread
> memory models do not guaranty reads are fresh in this scenario
> unless there is a memory barrier of some sort forcing per-cpu
> caches to be flushed.
> 
> This change uses the underlying threading.Event as the memory
> barrier instead of mocking it and just wraps the event to record
> calls manually.
> 
>  src/main/python/apache/thermos/observer/task_observer.py  |  5 +++--
>  src/test/python/apache/thermos/observer/test_task_observer.py | 36 
> 
>  2 files changed, 27 insertions(+), 14 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/observer/task_observer.py 
> 1485de8faef52716f11b82a3556064de26c67427 
>   src/test/python/apache/thermos/observer/test_task_observer.py 
> ace15c5305e75fac3a82971f4d71b92bcb37bafc 
> 
> Diff: https://reviews.apache.org/r/41915/diff/
> 
> 
> Testing
> ---
> 
> Before this change I got a failure between 1/5 and 1/10th of the
> time via:
> ```
> while true
> do
>   ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest
> done
> ```
> 
> After the change I cannot trigger the failure.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41915: Fixup TaskObserverTest to respect thread memory models.

2016-01-04 Thread John Sirois



> On Jan. 4, 2016, 8:25 p.m., Maxim Khutornenko wrote:
> > src/main/python/apache/thermos/observer/task_observer.py, line 72
> > 
> >
> > We usually put this default initialization directly into the arg list, 
> > e.g:'stop_event=threading.Event()'.
> 
> John Sirois wrote:
> I'm leery of that for mutable objects like an Event.  Surprising things 
> happen if/when the containing object gets constructed a 2nd time and the 
> single default Event has been mutated!

Maxim - your test originally so your call.  For a refresh - see here: 
https://reviews.apache.org/r/35527/


- John


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


On Jan. 4, 2016, 8:02 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41915/
> ---
> 
> (Updated Jan. 4, 2016, 8:02 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1570
> https://issues.apache.org/jira/browse/AURORA-1570
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously, a mock threading.Event was waited on in one thread
> and the count of waits was read in another thread.  Most thread
> memory models do not guaranty reads are fresh in this scenario
> unless there is a memory barrier of some sort forcing per-cpu
> caches to be flushed.
> 
> This change uses the underlying threading.Event as the memory
> barrier instead of mocking it and just wraps the event to record
> calls manually.
> 
>  src/main/python/apache/thermos/observer/task_observer.py  |  5 +++--
>  src/test/python/apache/thermos/observer/test_task_observer.py | 36 
> 
>  2 files changed, 27 insertions(+), 14 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/observer/task_observer.py 
> 1485de8faef52716f11b82a3556064de26c67427 
>   src/test/python/apache/thermos/observer/test_task_observer.py 
> ace15c5305e75fac3a82971f4d71b92bcb37bafc 
> 
> Diff: https://reviews.apache.org/r/41915/diff/
> 
> 
> Testing
> ---
> 
> Before this change I got a failure between 1/5 and 1/10th of the
> time via:
> ```
> while true
> do
>   ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest
> done
> ```
> 
> After the change I cannot trigger the failure.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41915: Fixup TaskObserverTest to respect thread memory models.

2016-01-04 Thread Maxim Khutornenko



> On Jan. 5, 2016, 3:05 a.m., John Sirois wrote:
> > Another answer could be to delete this test altogether.  It looks like it 
> > only really tests the proper converson from Time Amounts to fractional 
> > second waits.
> 
> Bill Farner wrote:
> I'm tempted to take this route as well.  It's not clear to me what this 
> test accomplishes, and it has the downside of high coupling to the 
> implementation being tested.
> 
> John Sirois wrote:
> Maxim - your test originally so your call.  For a refresh - see here: 
> https://reviews.apache.org/r/35527/

I am fine dropping this test.


- Maxim


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


On Jan. 5, 2016, 3:02 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41915/
> ---
> 
> (Updated Jan. 5, 2016, 3:02 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1570
> https://issues.apache.org/jira/browse/AURORA-1570
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously, a mock threading.Event was waited on in one thread
> and the count of waits was read in another thread.  Most thread
> memory models do not guaranty reads are fresh in this scenario
> unless there is a memory barrier of some sort forcing per-cpu
> caches to be flushed.
> 
> This change uses the underlying threading.Event as the memory
> barrier instead of mocking it and just wraps the event to record
> calls manually.
> 
>  src/main/python/apache/thermos/observer/task_observer.py  |  5 +++--
>  src/test/python/apache/thermos/observer/test_task_observer.py | 36 
> 
>  2 files changed, 27 insertions(+), 14 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/observer/task_observer.py 
> 1485de8faef52716f11b82a3556064de26c67427 
>   src/test/python/apache/thermos/observer/test_task_observer.py 
> ace15c5305e75fac3a82971f4d71b92bcb37bafc 
> 
> Diff: https://reviews.apache.org/r/41915/diff/
> 
> 
> Testing
> ---
> 
> Before this change I got a failure between 1/5 and 1/10th of the
> time via:
> ```
> while true
> do
>   ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest
> done
> ```
> 
> After the change I cannot trigger the failure.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Review Request 41915: Fixup TaskObserverTest to respect thread memory models.

2016-01-04 Thread John Sirois


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

Review request for Aurora, Maxim Khutornenko and Bill Farner.


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


Repository: aurora


Description
---

Previously, a mock threading.Event was waited on in one thread
and the count of waits was read in another thread.  Most thread
memory models do not guaranty reads a fresh in this scenario
unless there is a memory barrier of some sort forcing per-cpu
caches to be flushed.

This change uses the underlying threading.Event as the memory
barrier instead of mocking it and just wraps the event to record
calls manually.

 src/main/python/apache/thermos/observer/task_observer.py  |  5 +++--
 src/test/python/apache/thermos/observer/test_task_observer.py | 36 

 2 files changed, 27 insertions(+), 14 deletions(-)


Diffs
-

  src/main/python/apache/thermos/observer/task_observer.py 
1485de8faef52716f11b82a3556064de26c67427 
  src/test/python/apache/thermos/observer/test_task_observer.py 
ace15c5305e75fac3a82971f4d71b92bcb37bafc 

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


Testing
---

Before this change I got a failure between 1/5 and 1/10th of the
time via:
```
while true
do
  ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest
done
```

After the change I cannot trigger the failure.


Thanks,

John Sirois

Re: Review Request 41894: Fixup ZooKeeperTestServer restartNetwork.

2016-01-04 Thread Bill Farner



> On Jan. 4, 2016, 4:27 p.m., John Sirois wrote:
> > "Fixup missing commons compile dep." < not the right RB.

Sorry about that, fixed!


- Bill


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


On Jan. 4, 2016, 3:04 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41894/
> ---
> 
> (Updated Jan. 4, 2016, 3:04 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> With the upgrade to 3.4.2 and the switch from `NIOServerCnxn.Factory` to
> `NIOServerCnxnFactory` came a change in the server internals.  Calling
> `connectionFactory.startup(zooKeeperServer)` now eventually calls
> `ZooKeeperServer.startup` which calls `SessionTrackerImpl.start`.  This
> last is a problem since `SessionTrackerImpl` is a `Thread` and so throws
> `IllegalThreadStateException`.
> 
>  
> commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
>  | 28 
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
>  54b0a09355c1467362c2ad788e217dd5f52de522 
> 
> Diff: https://reviews.apache.org/r/41894/diff/
> 
> 
> Testing
> ---
> 
> Not green locally due to ZK/krb interaction issues from
> https://git1-us-west.apache.org/repos/asf/aurora/repo?p=aurora.git;a=commit;h=8706a781968912c68688284d9d3813d34ce45bf7,
> but the CI script gets further on my machine with this fix.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41915: Fixup TaskObserverTest to respect thread memory models.

2016-01-04 Thread John Sirois



> On Jan. 4, 2016, 8:25 p.m., Maxim Khutornenko wrote:
> > src/main/python/apache/thermos/observer/task_observer.py, line 72
> > 
> >
> > We usually put this default initialization directly into the arg list, 
> > e.g:'stop_event=threading.Event()'.
> 
> John Sirois wrote:
> I'm leery of that for mutable objects like an Event.  Surprising things 
> happen if/when the containing object gets constructed a 2nd time and the 
> single default Event has been mutated!
> 
> John Sirois wrote:
> Maxim - your test originally so your call.  For a refresh - see here: 
> https://reviews.apache.org/r/35527/

Disregard comment immediately above - now moved up under Bill's comment further 
above where it belongs.


- John


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


On Jan. 4, 2016, 8:02 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41915/
> ---
> 
> (Updated Jan. 4, 2016, 8:02 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1570
> https://issues.apache.org/jira/browse/AURORA-1570
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously, a mock threading.Event was waited on in one thread
> and the count of waits was read in another thread.  Most thread
> memory models do not guaranty reads are fresh in this scenario
> unless there is a memory barrier of some sort forcing per-cpu
> caches to be flushed.
> 
> This change uses the underlying threading.Event as the memory
> barrier instead of mocking it and just wraps the event to record
> calls manually.
> 
>  src/main/python/apache/thermos/observer/task_observer.py  |  5 +++--
>  src/test/python/apache/thermos/observer/test_task_observer.py | 36 
> 
>  2 files changed, 27 insertions(+), 14 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/observer/task_observer.py 
> 1485de8faef52716f11b82a3556064de26c67427 
>   src/test/python/apache/thermos/observer/test_task_observer.py 
> ace15c5305e75fac3a82971f4d71b92bcb37bafc 
> 
> Diff: https://reviews.apache.org/r/41915/diff/
> 
> 
> Testing
> ---
> 
> Before this change I got a failure between 1/5 and 1/10th of the
> time via:
> ```
> while true
> do
>   ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest
> done
> ```
> 
> After the change I cannot trigger the failure.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41915: Fixup TaskObserverTest to respect thread memory models.

2016-01-04 Thread John Sirois



> On Jan. 4, 2016, 8:05 p.m., John Sirois wrote:
> > Another answer could be to delete this test altogether.  It looks like it 
> > only really tests the proper converson from Time Amounts to fractional 
> > second waits.
> 
> Bill Farner wrote:
> I'm tempted to take this route as well.  It's not clear to me what this 
> test accomplishes, and it has the downside of high coupling to the 
> implementation being tested.

Maxim - your test originally so your call.  For a refresh - see here: 
https://reviews.apache.org/r/35527/


- John


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


On Jan. 4, 2016, 8:02 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41915/
> ---
> 
> (Updated Jan. 4, 2016, 8:02 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1570
> https://issues.apache.org/jira/browse/AURORA-1570
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously, a mock threading.Event was waited on in one thread
> and the count of waits was read in another thread.  Most thread
> memory models do not guaranty reads are fresh in this scenario
> unless there is a memory barrier of some sort forcing per-cpu
> caches to be flushed.
> 
> This change uses the underlying threading.Event as the memory
> barrier instead of mocking it and just wraps the event to record
> calls manually.
> 
>  src/main/python/apache/thermos/observer/task_observer.py  |  5 +++--
>  src/test/python/apache/thermos/observer/test_task_observer.py | 36 
> 
>  2 files changed, 27 insertions(+), 14 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/observer/task_observer.py 
> 1485de8faef52716f11b82a3556064de26c67427 
>   src/test/python/apache/thermos/observer/test_task_observer.py 
> ace15c5305e75fac3a82971f4d71b92bcb37bafc 
> 
> Diff: https://reviews.apache.org/r/41915/diff/
> 
> 
> Testing
> ---
> 
> Before this change I got a failure between 1/5 and 1/10th of the
> time via:
> ```
> while true
> do
>   ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest
> done
> ```
> 
> After the change I cannot trigger the failure.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41785: Remove scheduler log scaffolding

2016-01-04 Thread Jake Farrell


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

Ship it!


Ship It!

- Jake Farrell


On Jan. 4, 2016, 5:04 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41785/
> ---
> 
> (Updated Jan. 4, 2016, 5:04 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Using log4j as a straw man.  If we decide to choose logback instead, i'm 
> happy to update this patch.
> 
> Here's an example of the log format:
> ```
> I1229 22:16:54.568 [pool-10-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> ```
> As opposed to what's on master:
> ```
> I1230 06:26:14.987 THREAD143 
> org.apache.aurora.scheduler.app.local.FakeMaster.lambda$start$0: All offers 
> consumed, suppressing offer cycle.
> ```
> 
> I could more closely match the existing format, but i think the change is an 
> improvement.
> 
> TODO before submitting: Update `LogConfig` to align with the selected 
> backend.  I'll wait until we decide on a backend before i update this.
> 
> 
> Diffs
> -
> 
>   NEWS c0c454d080023a2e2796022f957a4d47bdb87b41 
>   build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 
>   commons/src/main/java/org/apache/aurora/common/logging/Glog.java 
> 5bae399cd9360a0093c67c608cf68b013a709194 
>   commons/src/main/java/org/apache/aurora/common/logging/LogFormatter.java 
> 0cb621da2b2f286d9eda9eb18ecac087208d7b6b 
>   commons/src/main/java/org/apache/aurora/common/logging/RootLogConfig.java 
> 26dd0aa5682faf31ba4cc086d32bf61557ef1fde 
>   
> commons/src/main/java/org/apache/aurora/common/logging/log4j/GlogLayout.java 
> 1a90ded72c81a7e3fdc9dab515a2622c55563e6a 
>   
> commons/src/test/java/org/apache/aurora/common/logging/LogFormatterTest.java 
> 9f041915c7fd4513a6255b05b3d0096779b5f1b3 
>   
> commons/src/test/java/org/apache/aurora/common/logging/RootLogConfigTest.java 
> 9d55a1aaf555d6e25ad97622612fad61271d0e25 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 4f43892723db4744db205ea7dd107e9e9ce9d5db 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 4033184451f36cb5f0233ea96e3dceaae6741275 
>   src/main/java/org/apache/aurora/scheduler/app/Log4jConfigurator.java 
> 348ff1345a3d5ff4212a2cb211e973ad9e2ca2e8 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> de018dd3cd82b7a0a1cb285f8f3172dae529817f 
>   src/main/resources/log4j2.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41785/diff/
> 
> 
> Testing
> ---
> 
> end-to-end tests are green
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41915: Fixup TaskObserverTest to respect thread memory models.

2016-01-04 Thread John Sirois


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

(Updated Jan. 4, 2016, 8:02 p.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


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


Repository: aurora


Description (updated)
---

Previously, a mock threading.Event was waited on in one thread
and the count of waits was read in another thread.  Most thread
memory models do not guaranty reads are fresh in this scenario
unless there is a memory barrier of some sort forcing per-cpu
caches to be flushed.

This change uses the underlying threading.Event as the memory
barrier instead of mocking it and just wraps the event to record
calls manually.

 src/main/python/apache/thermos/observer/task_observer.py  |  5 +++--
 src/test/python/apache/thermos/observer/test_task_observer.py | 36 

 2 files changed, 27 insertions(+), 14 deletions(-)


Diffs
-

  src/main/python/apache/thermos/observer/task_observer.py 
1485de8faef52716f11b82a3556064de26c67427 
  src/test/python/apache/thermos/observer/test_task_observer.py 
ace15c5305e75fac3a82971f4d71b92bcb37bafc 

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


Testing
---

Before this change I got a failure between 1/5 and 1/10th of the
time via:
```
while true
do
  ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest
done
```

After the change I cannot trigger the failure.


Thanks,

John Sirois

Re: Review Request 41915: Fixup TaskObserverTest to respect thread memory models.

2016-01-04 Thread John Sirois



> On Jan. 4, 2016, 8:25 p.m., Maxim Khutornenko wrote:
> > src/main/python/apache/thermos/observer/task_observer.py, line 72
> > 
> >
> > We usually put this default initialization directly into the arg list, 
> > e.g:'stop_event=threading.Event()'.

I'm leery of that for mutable objects like an Event.  Surprising things happen 
if/when the containing object gets constructed a 2nd time and the single 
default Event has been mutated!


- John


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


On Jan. 4, 2016, 8:02 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41915/
> ---
> 
> (Updated Jan. 4, 2016, 8:02 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1570
> https://issues.apache.org/jira/browse/AURORA-1570
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously, a mock threading.Event was waited on in one thread
> and the count of waits was read in another thread.  Most thread
> memory models do not guaranty reads are fresh in this scenario
> unless there is a memory barrier of some sort forcing per-cpu
> caches to be flushed.
> 
> This change uses the underlying threading.Event as the memory
> barrier instead of mocking it and just wraps the event to record
> calls manually.
> 
>  src/main/python/apache/thermos/observer/task_observer.py  |  5 +++--
>  src/test/python/apache/thermos/observer/test_task_observer.py | 36 
> 
>  2 files changed, 27 insertions(+), 14 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/observer/task_observer.py 
> 1485de8faef52716f11b82a3556064de26c67427 
>   src/test/python/apache/thermos/observer/test_task_observer.py 
> ace15c5305e75fac3a82971f4d71b92bcb37bafc 
> 
> Diff: https://reviews.apache.org/r/41915/diff/
> 
> 
> Testing
> ---
> 
> Before this change I got a failure between 1/5 and 1/10th of the
> time via:
> ```
> while true
> do
>   ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest
> done
> ```
> 
> After the change I cannot trigger the failure.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41915: Fixup TaskObserverTest to respect thread memory models.

2016-01-04 Thread Bill Farner



> On Jan. 4, 2016, 7:05 p.m., John Sirois wrote:
> > Another answer could be to delete this test altogether.  It looks like it 
> > only really tests the proper converson from Time Amounts to fractional 
> > second waits.

I'm tempted to take this route as well.  It's not clear to me what this test 
accomplishes, and it has the downside of high coupling to the implementation 
being tested.


- Bill


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


On Jan. 4, 2016, 7:02 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41915/
> ---
> 
> (Updated Jan. 4, 2016, 7:02 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1570
> https://issues.apache.org/jira/browse/AURORA-1570
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously, a mock threading.Event was waited on in one thread
> and the count of waits was read in another thread.  Most thread
> memory models do not guaranty reads are fresh in this scenario
> unless there is a memory barrier of some sort forcing per-cpu
> caches to be flushed.
> 
> This change uses the underlying threading.Event as the memory
> barrier instead of mocking it and just wraps the event to record
> calls manually.
> 
>  src/main/python/apache/thermos/observer/task_observer.py  |  5 +++--
>  src/test/python/apache/thermos/observer/test_task_observer.py | 36 
> 
>  2 files changed, 27 insertions(+), 14 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/observer/task_observer.py 
> 1485de8faef52716f11b82a3556064de26c67427 
>   src/test/python/apache/thermos/observer/test_task_observer.py 
> ace15c5305e75fac3a82971f4d71b92bcb37bafc 
> 
> Diff: https://reviews.apache.org/r/41915/diff/
> 
> 
> Testing
> ---
> 
> Before this change I got a failure between 1/5 and 1/10th of the
> time via:
> ```
> while true
> do
>   ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest
> done
> ```
> 
> After the change I cannot trigger the failure.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 40922: Thermos: Add ability to specify process outputs destination

2016-01-04 Thread Martin Hrabovcin


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

(Updated Jan. 4, 2016, 12:25 p.m.)


Review request for Aurora and John Sirois.


Changes
---

Updated patch addresses @StephanErb comments. @ReviewBot retry


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


Repository: aurora


Description
---

This patch will provide way to **optionally** specify running process outputs 
destination. Implementation was built on top of 
https://reviews.apache.org/r/30695/

**What was changed:**

New `destination` parameter is available on global cluster level and also on 
each `Process` level. Possible options are `file` (default), `stream` to parent 
process stdout/stderr, `mixed` will split output to files and stream and 
finally `none` to discard any logs produced by running process.


Diffs (updated)
-

  NEWS c0c454d 
  docs/configuration-reference.md cf63cfa 
  docs/deploying-aurora-scheduler.md c0988e8 
  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 4e9b027 
  src/main/python/apache/aurora/executor/thermos_task_runner.py c019fc9 
  src/main/python/apache/thermos/config/schema_base.py 5552108 
  src/main/python/apache/thermos/core/process.py 8a181b0 
  src/main/python/apache/thermos/core/runner.py 5623dce 
  src/main/python/apache/thermos/runner/thermos_runner.py 3dacd45 
  src/test/python/apache/thermos/core/test_process.py da4c494 

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


Testing
---

Unit test coverage is provided for new functionality.

I did also manual testing with mesos/docker and I made sure that logs are being 
written to expected files and also same output gets to docker daemon.


Thanks,

Martin Hrabovcin

Re: Review Request 40922: Thermos: Add ability to specify process outputs destination

2016-01-04 Thread Aurora ReviewBot


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

Ship it!


Master (8706a78) 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 Jan. 4, 2016, 12:25 p.m., Martin Hrabovcin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40922/
> ---
> 
> (Updated Jan. 4, 2016, 12:25 p.m.)
> 
> 
> Review request for Aurora and John Sirois.
> 
> 
> Bugs: AURORA-1548
> https://issues.apache.org/jira/browse/AURORA-1548
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch will provide way to **optionally** specify running process outputs 
> destination. Implementation was built on top of 
> https://reviews.apache.org/r/30695/
> 
> **What was changed:**
> 
> New `destination` parameter is available on global cluster level and also on 
> each `Process` level. Possible options are `file` (default), `stream` to 
> parent process stdout/stderr, `mixed` will split output to files and stream 
> and finally `none` to discard any logs produced by running process.
> 
> 
> Diffs
> -
> 
>   NEWS c0c454d 
>   docs/configuration-reference.md cf63cfa 
>   docs/deploying-aurora-scheduler.md c0988e8 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 4e9b027 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py c019fc9 
>   src/main/python/apache/thermos/config/schema_base.py 5552108 
>   src/main/python/apache/thermos/core/process.py 8a181b0 
>   src/main/python/apache/thermos/core/runner.py 5623dce 
>   src/main/python/apache/thermos/runner/thermos_runner.py 3dacd45 
>   src/test/python/apache/thermos/core/test_process.py da4c494 
> 
> Diff: https://reviews.apache.org/r/40922/diff/
> 
> 
> Testing
> ---
> 
> Unit test coverage is provided for new functionality.
> 
> I did also manual testing with mesos/docker and I made sure that logs are 
> being written to expected files and also same output gets to docker daemon.
> 
> 
> Thanks,
> 
> Martin Hrabovcin
> 
>

Re: Review Request 41915: Kill flaky TaskObserverTest.

2016-01-04 Thread John Sirois



> On Jan. 4, 2016, 8:25 p.m., Maxim Khutornenko wrote:
> > src/main/python/apache/thermos/observer/task_observer.py, line 72
> > 
> >
> > We usually put this default initialization directly into the arg list, 
> > e.g:'stop_event=threading.Event()'.
> 
> John Sirois wrote:
> I'm leery of that for mutable objects like an Event.  Surprising things 
> happen if/when the containing object gets constructed a 2nd time and the 
> single default Event has been mutated!
> 
> John Sirois wrote:
> Maxim - your test originally so your call.  For a refresh - see here: 
> https://reviews.apache.org/r/35527/
> 
> John Sirois wrote:
> Disregard comment immediately above - now moved up under Bill's comment 
> further above where it belongs.
> 
> Maxim Khutornenko wrote:
> Not sure how the current approach helps with anything. If a constructor 
> is called once with `stop_event` and another time without it, wouldn't we end 
> up in the same state with mutated field due to `stop_event` arg being None 
> during the second call?
> 
> John Sirois wrote:
> The issue is 2 or more calls without.  Say the interleaving goes like 
> this:
> ```
> to1 = TaskObserver(createPathDetector())
> to1.stop()
> to2 = TaskObserver(createPathDetector())
> ```
> 
> Since `to2` shares `to1`'s Event, its already `set` from the `stop` of 
> `to1` and so never runs.
> 
> Instead, by using `None` as the sentinel for the default, both `to1` and 
> `to2` each get their own fresh Event.

This might explain things better than words:
```
>>> def surprise(value, mutable=[]):
...   mutable.append(value)
...   return mutable
... 
>>> surprise(42)
[42]
>>> surprise(1137)
[42, 1137]
>>> def surprise(value, mutable=None):
...   mutable = mutable or []
...   mutable.append(value)
...   return mutable
... 
>>> surprise(42)
[42]
>>> surprise(1137)
[1137]
>>>
```


- John


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


On Jan. 4, 2016, 10:15 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41915/
> ---
> 
> (Updated Jan. 4, 2016, 10:15 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1570
> https://issues.apache.org/jira/browse/AURORA-1570
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously, a mock threading.Event was waited on in one thread
> and the count of waits was read in another thread.  Most thread
> memory models do not guaranty reads are fresh in this scenario
> unless there is a memory barrier of some sort forcing per-cpu
> caches to be flushed.
> 
> Since the test really only verified correct conversion of a poll
> interval to fractional seconds - kill the test as not pulling its
> weight.
> 
>  src/test/python/apache/thermos/observer/BUILD |  1 +
>  src/test/python/apache/thermos/observer/test_task_observer.py | 45 
> -
>  2 files changed, 1 insertion(+), 45 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/test/python/apache/thermos/observer/BUILD 
> f3f697c35ee5473072171926923459a4dde15545 
>   src/test/python/apache/thermos/observer/test_task_observer.py 
> ace15c5305e75fac3a82971f4d71b92bcb37bafc 
> 
> Diff: https://reviews.apache.org/r/41915/diff/
> 
> 
> Testing
> ---
> 
> Before this change I got a failure between 1/5 and 1/10th of the
> time via:
> ```
> while true
> do
>   ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest
> done
> ```
> 
> After the change I cannot trigger the failure.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Review Request 41917: Avoid zk 3.4.7 to fix test hangs.

2016-01-04 Thread John Sirois


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

Review request for Aurora, Maxim Khutornenko and Bill Farner.


Repository: aurora


Description
---

The commons tests hang under CI after bumping from zk 3.4.2 to 3.4.7.
Although not root-caused, this zk bug introduced in 3.4.7 seems like a
match for this sort of hang:
  https://issues.apache.org/jira/browse/ZOOKEEPER-2347

Downgrade to 3.4.6 with a note about why 3.4.7 should be skipped.

 NEWS | 2 +-
 build.gradle | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)


Diffs
-

  NEWS a4775adb18938ad80ee752e37d3f7c10549d3b70 
  build.gradle 14ebf6b1d295e0780b7e390d00efadb25d30fc79 

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


Testing
---

Locally green: `./build-support/jenkins/build.sh`.

That said - I could not reproduce the CI 120 minute timeouts as seen 
here:
  https://builds.apache.org/job/Aurora/1312/
  https://builds.apache.org/job/Aurora/1313/


Thanks,

John Sirois

Re: Review Request 41915: Fixup TaskObserverTest to respect thread memory models.

2016-01-04 Thread Aurora ReviewBot


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

Ship it!


Master (227a3a6) 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 Jan. 5, 2016, 3:02 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41915/
> ---
> 
> (Updated Jan. 5, 2016, 3:02 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1570
> https://issues.apache.org/jira/browse/AURORA-1570
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously, a mock threading.Event was waited on in one thread
> and the count of waits was read in another thread.  Most thread
> memory models do not guaranty reads are fresh in this scenario
> unless there is a memory barrier of some sort forcing per-cpu
> caches to be flushed.
> 
> This change uses the underlying threading.Event as the memory
> barrier instead of mocking it and just wraps the event to record
> calls manually.
> 
>  src/main/python/apache/thermos/observer/task_observer.py  |  5 +++--
>  src/test/python/apache/thermos/observer/test_task_observer.py | 36 
> 
>  2 files changed, 27 insertions(+), 14 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/observer/task_observer.py 
> 1485de8faef52716f11b82a3556064de26c67427 
>   src/test/python/apache/thermos/observer/test_task_observer.py 
> ace15c5305e75fac3a82971f4d71b92bcb37bafc 
> 
> Diff: https://reviews.apache.org/r/41915/diff/
> 
> 
> Testing
> ---
> 
> Before this change I got a failure between 1/5 and 1/10th of the
> time via:
> ```
> while true
> do
>   ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest
> done
> ```
> 
> After the change I cannot trigger the failure.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41915: Kill flaky TaskObserverTest.

2016-01-04 Thread John Sirois


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

(Updated Jan. 4, 2016, 10:15 p.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Changes
---

Kill test_task_observer.py.

Fixup the associated BUILD as well.

 src/main/python/apache/thermos/observer/task_observer.py  |  5 ++---
 src/test/python/apache/thermos/observer/BUILD |  1 +
 src/test/python/apache/thermos/observer/test_task_observer.py | 57 
--
 3 files changed, 3 insertions(+), 60 deletions(-)


Summary (updated)
-

Kill flaky TaskObserverTest.


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


Repository: aurora


Description (updated)
---

Previously, a mock threading.Event was waited on in one thread
and the count of waits was read in another thread.  Most thread
memory models do not guaranty reads are fresh in this scenario
unless there is a memory barrier of some sort forcing per-cpu
caches to be flushed.

Since the test really only verified correct conversion of a poll
interval to fractional seconds - kill the test as not pulling its
weight.

 src/test/python/apache/thermos/observer/BUILD |  1 +
 src/test/python/apache/thermos/observer/test_task_observer.py | 45 
-
 2 files changed, 1 insertion(+), 45 deletions(-)


Diffs (updated)
-

  src/test/python/apache/thermos/observer/BUILD 
f3f697c35ee5473072171926923459a4dde15545 
  src/test/python/apache/thermos/observer/test_task_observer.py 
ace15c5305e75fac3a82971f4d71b92bcb37bafc 

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


Testing
---

Before this change I got a failure between 1/5 and 1/10th of the
time via:
```
while true
do
  ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest
done
```

After the change I cannot trigger the failure.


Thanks,

John Sirois

Re: Review Request 41915: Fixup TaskObserverTest to respect thread memory models.

2016-01-04 Thread John Sirois



> On Jan. 4, 2016, 8:05 p.m., John Sirois wrote:
> > Another answer could be to delete this test altogether.  It looks like it 
> > only really tests the proper converson from Time Amounts to fractional 
> > second waits.
> 
> Bill Farner wrote:
> I'm tempted to take this route as well.  It's not clear to me what this 
> test accomplishes, and it has the downside of high coupling to the 
> implementation being tested.
> 
> John Sirois wrote:
> Maxim - your test originally so your call.  For a refresh - see here: 
> https://reviews.apache.org/r/35527/
> 
> Maxim Khutornenko wrote:
> I am fine dropping this test.

OK - done.


- John


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


On Jan. 4, 2016, 8:02 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41915/
> ---
> 
> (Updated Jan. 4, 2016, 8:02 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1570
> https://issues.apache.org/jira/browse/AURORA-1570
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously, a mock threading.Event was waited on in one thread
> and the count of waits was read in another thread.  Most thread
> memory models do not guaranty reads are fresh in this scenario
> unless there is a memory barrier of some sort forcing per-cpu
> caches to be flushed.
> 
> This change uses the underlying threading.Event as the memory
> barrier instead of mocking it and just wraps the event to record
> calls manually.
> 
>  src/main/python/apache/thermos/observer/task_observer.py  |  5 +++--
>  src/test/python/apache/thermos/observer/test_task_observer.py | 36 
> 
>  2 files changed, 27 insertions(+), 14 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/observer/task_observer.py 
> 1485de8faef52716f11b82a3556064de26c67427 
>   src/test/python/apache/thermos/observer/test_task_observer.py 
> ace15c5305e75fac3a82971f4d71b92bcb37bafc 
> 
> Diff: https://reviews.apache.org/r/41915/diff/
> 
> 
> Testing
> ---
> 
> Before this change I got a failure between 1/5 and 1/10th of the
> time via:
> ```
> while true
> do
>   ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest
> done
> ```
> 
> After the change I cannot trigger the failure.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41915: Kill flaky TaskObserverTest.

2016-01-04 Thread Maxim Khutornenko



> On Jan. 5, 2016, 3:25 a.m., Maxim Khutornenko wrote:
> > src/main/python/apache/thermos/observer/task_observer.py, line 72
> > 
> >
> > We usually put this default initialization directly into the arg list, 
> > e.g:'stop_event=threading.Event()'.
> 
> John Sirois wrote:
> I'm leery of that for mutable objects like an Event.  Surprising things 
> happen if/when the containing object gets constructed a 2nd time and the 
> single default Event has been mutated!
> 
> John Sirois wrote:
> Maxim - your test originally so your call.  For a refresh - see here: 
> https://reviews.apache.org/r/35527/
> 
> John Sirois wrote:
> Disregard comment immediately above - now moved up under Bill's comment 
> further above where it belongs.
> 
> Maxim Khutornenko wrote:
> Not sure how the current approach helps with anything. If a constructor 
> is called once with `stop_event` and another time without it, wouldn't we end 
> up in the same state with mutated field due to `stop_event` arg being None 
> during the second call?
> 
> John Sirois wrote:
> The issue is 2 or more calls without.  Say the interleaving goes like 
> this:
> ```
> to1 = TaskObserver(createPathDetector())
> to1.stop()
> to2 = TaskObserver(createPathDetector())
> ```
> 
> Since `to2` shares `to1`'s Event, its already `set` from the `stop` of 
> `to1` and so never runs.
> 
> Instead, by using `None` as the sentinel for the default, both `to1` and 
> `to2` each get their own fresh Event.
> 
> John Sirois wrote:
> This might explain things better than words:
> ```
> >>> def surprise(value, mutable=[]):
> ...   mutable.append(value)
> ...   return mutable
> ... 
> >>> surprise(42)
> [42]
> >>> surprise(1137)
> [42, 1137]
> >>> def surprise(value, mutable=None):
> ...   mutable = mutable or []
> ...   mutable.append(value)
> ...   return mutable
> ... 
> >>> surprise(42)
> [42]
> >>> surprise(1137)
> [1137]
> >>>
> ```

Ah, you're right. This python mutable default argument "feature" keeps catching 
me every time.


- Maxim


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


On Jan. 5, 2016, 5:15 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41915/
> ---
> 
> (Updated Jan. 5, 2016, 5:15 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1570
> https://issues.apache.org/jira/browse/AURORA-1570
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously, a mock threading.Event was waited on in one thread
> and the count of waits was read in another thread.  Most thread
> memory models do not guaranty reads are fresh in this scenario
> unless there is a memory barrier of some sort forcing per-cpu
> caches to be flushed.
> 
> Since the test really only verified correct conversion of a poll
> interval to fractional seconds - kill the test as not pulling its
> weight.
> 
>  src/test/python/apache/thermos/observer/BUILD |  1 +
>  src/test/python/apache/thermos/observer/test_task_observer.py | 45 
> -
>  2 files changed, 1 insertion(+), 45 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/test/python/apache/thermos/observer/BUILD 
> f3f697c35ee5473072171926923459a4dde15545 
>   src/test/python/apache/thermos/observer/test_task_observer.py 
> ace15c5305e75fac3a82971f4d71b92bcb37bafc 
> 
> Diff: https://reviews.apache.org/r/41915/diff/
> 
> 
> Testing
> ---
> 
> Before this change I got a failure between 1/5 and 1/10th of the
> time via:
> ```
> while true
> do
>   ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest
> done
> ```
> 
> After the change I cannot trigger the failure.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41917: Avoid zk 3.4.7 to fix test hangs.

2016-01-04 Thread Aurora ReviewBot


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

Ship it!


Master (227a3a6) 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 Jan. 5, 2016, 6:06 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41917/
> ---
> 
> (Updated Jan. 5, 2016, 6:06 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The commons tests hang under CI after bumping from zk 3.4.2 to 3.4.7.
> Although not root-caused, this zk bug introduced in 3.4.7 seems like a
> match for this sort of hang:
>   https://issues.apache.org/jira/browse/ZOOKEEPER-2347
> 
> Downgrade to 3.4.6 with a note about why 3.4.7 should be skipped.
> 
>  NEWS | 2 +-
>  build.gradle | 4 +++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> 
> Diffs
> -
> 
>   NEWS a4775adb18938ad80ee752e37d3f7c10549d3b70 
>   build.gradle 14ebf6b1d295e0780b7e390d00efadb25d30fc79 
> 
> Diff: https://reviews.apache.org/r/41917/diff/
> 
> 
> Testing
> ---
> 
> Locally green: `./build-support/jenkins/build.sh`.
> 
> That said - I could not reproduce the CI 120 minute timeouts as seen 
> here:
>   https://builds.apache.org/job/Aurora/1312/
>   https://builds.apache.org/job/Aurora/1313/
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41917: Avoid zk 3.4.7 to fix test hangs.

2016-01-04 Thread John Sirois


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

(Updated Jan. 5, 2016, 12:37 a.m.)


Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Bill Farner.


Bugs: ZOOKEEPER-2347
https://issues.apache.org/jira/browse/ZOOKEEPER-2347


Repository: aurora


Description
---

The commons tests hang under CI after bumping from zk 3.4.2 to 3.4.7.
Although not root-caused, this zk bug introduced in 3.4.7 seems like a
match for this sort of hang:
  https://issues.apache.org/jira/browse/ZOOKEEPER-2347

Downgrade to 3.4.6 with a note about why 3.4.7 should be skipped.

 NEWS | 2 +-
 build.gradle | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)


Diffs
-

  NEWS a4775adb18938ad80ee752e37d3f7c10549d3b70 
  build.gradle 14ebf6b1d295e0780b7e390d00efadb25d30fc79 

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


Testing
---

Locally green: `./build-support/jenkins/build.sh`.

That said - I could not reproduce the CI 120 minute timeouts as seen 
here:
  https://builds.apache.org/job/Aurora/1312/
  https://builds.apache.org/job/Aurora/1313/


Thanks,

John Sirois

Re: Review Request 41915: Kill flaky TaskObserverTest.

2016-01-04 Thread Aurora ReviewBot


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

Ship it!


Master (227a3a6) 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 Jan. 5, 2016, 5:15 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41915/
> ---
> 
> (Updated Jan. 5, 2016, 5:15 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1570
> https://issues.apache.org/jira/browse/AURORA-1570
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously, a mock threading.Event was waited on in one thread
> and the count of waits was read in another thread.  Most thread
> memory models do not guaranty reads are fresh in this scenario
> unless there is a memory barrier of some sort forcing per-cpu
> caches to be flushed.
> 
> Since the test really only verified correct conversion of a poll
> interval to fractional seconds - kill the test as not pulling its
> weight.
> 
>  src/test/python/apache/thermos/observer/BUILD |  1 +
>  src/test/python/apache/thermos/observer/test_task_observer.py | 45 
> -
>  2 files changed, 1 insertion(+), 45 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/test/python/apache/thermos/observer/BUILD 
> f3f697c35ee5473072171926923459a4dde15545 
>   src/test/python/apache/thermos/observer/test_task_observer.py 
> ace15c5305e75fac3a82971f4d71b92bcb37bafc 
> 
> Diff: https://reviews.apache.org/r/41915/diff/
> 
> 
> Testing
> ---
> 
> Before this change I got a failure between 1/5 and 1/10th of the
> time via:
> ```
> while true
> do
>   ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest
> done
> ```
> 
> After the change I cannot trigger the failure.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Review Request 41897: Upgrade to the latest zk point release.

2016-01-04 Thread John Sirois


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

Review request for Aurora, Maxim Khutornenko and Bill Farner.


Repository: aurora


Description
---

This is enabled by https://reviews.apache.org/r/41895/ which
isolated the kerberos configuration.

 build.gradle | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)


Diffs
-

  build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 

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


Testing
---

Both `./build-support/jenkins/build.sh` and
`./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` ran
green locally.


Thanks,

John Sirois

50 matches

Mail list logo