Re: Review Request 32329: Extract job key from RPC parameters

[Kevin Sweeney]

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

(Updated March 23, 2015, 12:14 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


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


Repository: aurora


Description (updated)
---

Apologies for the large diff, this wound up needing to input validation at the 
AOP layer.

Probably the best place to start reading this diff is ApiSecurityIT to see the 
feature this patch enables.


Diffs
-

  config/pmd/custom.xml 521fd500146eb2e45f8e77c5c3c0cce330fedabb 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
827e85b6cac8bd52359610bbc2002973a769705c 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
2408cd1f9af5f109a339f5c78134465cb117f7fc 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
 cc9cfd38239f909b8a77bd1a773e31ec30130d41 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
 808987939b2c4a850e488dc033b50b0178e95ba0 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java
 4e341e05c34b1be38f0040c26b671a0cc797a771 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetter.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetter.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetters.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
5588d1793d6713ee4581ac9f938d9a8689acb315 
  src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java 
bdd2185f3a7a94b39bcec3c73455e970d87f0c6a 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
cafd10f6b705568588c1b92644b482003242fe2e 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java 
ed284f46ac8f01bd6d9e317f995f16d6e666a68d 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
76cb691e6d7d4fada3a18fde73aceed7039bcaa4 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptorTest.java
 PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
 PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java
 d2ba2730c4509dc9a636fd32e9244b0d7fa2884f 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetterTest.java
 PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
1f24e7d47e1f777ffef19a73d01171fcacd31cdb 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
d20c9da3c4944ec8c50fe8d48b7e459ff1c7082b 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney

Re: Review Request 32377: Add a mechanism to lazily instantiate module classes.

[Kevin Sweeney]

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

Ship it!



src/main/java/org/apache/aurora/scheduler/app/Modules.java
<https://reviews.apache.org/r/32377/#comment125503>

consider adding a Class... overload.


- Kevin Sweeney


On March 23, 2015, 11:51 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32377/
> ---
> 
> (Updated March 23, 2015, 11:51 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1217
> https://issues.apache.org/jira/browse/AURORA-1217
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add an indirection for modules that must not be statically instantiated.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/Modules.java 
> e95cb2a255e6986e0678a4085036deb24f9cb359 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
>  cc9cfd38239f909b8a77bd1a773e31ec30130d41 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java 
> fe6409ab14ebfb89b161f919f764879d42e53877 
>   src/test/java/org/apache/aurora/scheduler/app/ModulesTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/32377/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 32377: Add a mechanism to lazily instantiate module classes.

[Kevin Sweeney]

> On March 23, 2015, 10:43 a.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java,
> >  line 61
> > <https://reviews.apache.org/r/32377/diff/1/?file=902302#file902302line61>
> >
> > Can you push the lazy instantiation logic to ModuleParser and change 
> > this back to Arg>?

To be clear I suggest defining

```java
private static final Arg> SHIRO_REALM_MODULES = 
Arg.create(ImmutableSet.of(Modules.lazilyInstantiated(IniShiroRealmModule.class)));
```


- Kevin


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


On March 22, 2015, 11:37 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32377/
> ---
> 
> (Updated March 22, 2015, 11:37 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1217
> https://issues.apache.org/jira/browse/AURORA-1217
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add an indirection for modules that must not be statically instantiated.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/Modules.java 
> e95cb2a255e6986e0678a4085036deb24f9cb359 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
>  cc9cfd38239f909b8a77bd1a773e31ec30130d41 
>   src/test/java/org/apache/aurora/scheduler/app/ModulesTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/32377/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 32377: Add a mechanism to lazily instantiate module classes.

[Kevin Sweeney]

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



src/main/java/org/apache/aurora/scheduler/app/Modules.java
<https://reviews.apache.org/r/32377/#comment125482>

Can you push this to ModuleParser?



src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
<https://reviews.apache.org/r/32377/#comment125483>

Can you push the lazy instantiation logic to ModuleParser and change this 
back to Arg>?


- Kevin Sweeney


On March 22, 2015, 11:37 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32377/
> ---
> 
> (Updated March 22, 2015, 11:37 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1217
> https://issues.apache.org/jira/browse/AURORA-1217
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add an indirection for modules that must not be statically instantiated.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/Modules.java 
> e95cb2a255e6986e0678a4085036deb24f9cb359 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
>  cc9cfd38239f909b8a77bd1a773e31ec30130d41 
>   src/test/java/org/apache/aurora/scheduler/app/ModulesTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/32377/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 32329: Extract job key from RPC parameters

[Kevin Sweeney]

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

(Updated March 20, 2015, 5:16 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


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


Repository: aurora


Description
---

Apologies for the large diff, this wound up needing to input validation at the 
AOP layer.

Probably the best place to start reading this diff is ApiSecurityIT to see the 
feature this patch enables.

Also the best place to start reviewing FieldGetterDag is also its tests and 
usage.


Diffs (updated)
-

  config/pmd/custom.xml 521fd500146eb2e45f8e77c5c3c0cce330fedabb 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
827e85b6cac8bd52359610bbc2002973a769705c 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
2408cd1f9af5f109a339f5c78134465cb117f7fc 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
 cc9cfd38239f909b8a77bd1a773e31ec30130d41 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
 808987939b2c4a850e488dc033b50b0178e95ba0 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java
 4e341e05c34b1be38f0040c26b671a0cc797a771 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetter.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetter.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetters.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
5588d1793d6713ee4581ac9f938d9a8689acb315 
  src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java 
bdd2185f3a7a94b39bcec3c73455e970d87f0c6a 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
cafd10f6b705568588c1b92644b482003242fe2e 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java 
ed284f46ac8f01bd6d9e317f995f16d6e666a68d 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
76cb691e6d7d4fada3a18fde73aceed7039bcaa4 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptorTest.java
 PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
 PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java
 d2ba2730c4509dc9a636fd32e9244b0d7fa2884f 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetterTest.java
 PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
1f24e7d47e1f777ffef19a73d01171fcacd31cdb 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
d20c9da3c4944ec8c50fe8d48b7e459ff1c7082b 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney

Re: Review Request 32329: Extract job key from RPC parameters

[Kevin Sweeney]

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

(Updated March 20, 2015, 3:01 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

stop referencing nonexistent StoredJob.


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


Repository: aurora


Description
---

Apologies for the large diff, this wound up needing to input validation at the 
AOP layer.

Probably the best place to start reading this diff is ApiSecurityIT to see the 
feature this patch enables.

Also the best place to start reviewing FieldGetterDag is also its tests and 
usage.


Diffs (updated)
-

  config/pmd/custom.xml 521fd500146eb2e45f8e77c5c3c0cce330fedabb 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
827e85b6cac8bd52359610bbc2002973a769705c 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
2408cd1f9af5f109a339f5c78134465cb117f7fc 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
 cc9cfd38239f909b8a77bd1a773e31ec30130d41 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
 808987939b2c4a850e488dc033b50b0178e95ba0 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/FieldGetterDag.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java
 4e341e05c34b1be38f0040c26b671a0cc797a771 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetter.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetter.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetters.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
5588d1793d6713ee4581ac9f938d9a8689acb315 
  src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java 
bdd2185f3a7a94b39bcec3c73455e970d87f0c6a 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
cafd10f6b705568588c1b92644b482003242fe2e 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java 
ed284f46ac8f01bd6d9e317f995f16d6e666a68d 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
76cb691e6d7d4fada3a18fde73aceed7039bcaa4 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/FieldGetterDagTest.java
 PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptorTest.java
 PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
 PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java
 d2ba2730c4509dc9a636fd32e9244b0d7fa2884f 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetterTest.java
 PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
1f24e7d47e1f777ffef19a73d01171fcacd31cdb 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
d20c9da3c4944ec8c50fe8d48b7e459ff1c7082b 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney

Review Request 32329: Extract job key from RPC parameters

[Kevin Sweeney]

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

Review request for Aurora, Joshua Cohen and Bill Farner.


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


Repository: aurora


Description
---

Apologies for the large diff, this wound up needing to input validation at the 
AOP layer.

Probably the best place to start reading this diff is ApiSecurityIT to see the 
feature this patch enables.

Also the best place to start reviewing FieldGetterDag is also its tests and 
usage.


Diffs
-

  config/pmd/custom.xml 521fd500146eb2e45f8e77c5c3c0cce330fedabb 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
827e85b6cac8bd52359610bbc2002973a769705c 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
2408cd1f9af5f109a339f5c78134465cb117f7fc 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
 cc9cfd38239f909b8a77bd1a773e31ec30130d41 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
 808987939b2c4a850e488dc033b50b0178e95ba0 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/FieldGetterDag.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java
 4e341e05c34b1be38f0040c26b671a0cc797a771 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetter.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetter.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetters.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
5588d1793d6713ee4581ac9f938d9a8689acb315 
  src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java 
bdd2185f3a7a94b39bcec3c73455e970d87f0c6a 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
cafd10f6b705568588c1b92644b482003242fe2e 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java 
ed284f46ac8f01bd6d9e317f995f16d6e666a68d 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
76cb691e6d7d4fada3a18fde73aceed7039bcaa4 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/FieldGetterDagTest.java
 PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptorTest.java
 PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
 PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java
 d2ba2730c4509dc9a636fd32e9244b0d7fa2884f 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetterTest.java
 PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
1f24e7d47e1f777ffef19a73d01171fcacd31cdb 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
d20c9da3c4944ec8c50fe8d48b7e459ff1c7082b 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney

Review Request 32323: DRY up PMD configuration.

[Kevin Sweeney]

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

Review request for Aurora, Joshua Cohen and Bill Farner.


Repository: aurora


Description
---

DRY up PMD configuration.


Diffs
-

  build.gradle 4a13e5c4281de53d0ea991235502943a439d1310 
  config/pmd/custom.xml 7026d04d8e5c2f185a19c7c44d44698b2fb846c9 
  config/pmd/design.xml 2741e407e35826e73bd4dc8335c0ef6294086398 
  config/pmd/imports.xml 40dc2260a3c27fd66be352a5974427dcf165ab34 
  config/pmd/logging-java.xml 66fecbf14aa61f340678dc23d57a1c074cad824a 
  config/pmd/naming.xml 233352d57857a0fe09ddbfe9228b48d4c09bb624 

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


Testing
---

./gradlew -Pq pmdMain -d

diffed the "Using rule" log lines from before and after:

```
% diff -u <(sort -u old-rules.txt) <(sort -u new-rules.txt)
%
```


Thanks,

Kevin Sweeney

Re: Review Request 32276: Fix error listing active updates.

[Kevin Sweeney]

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

Ship it!



src/main/python/apache/aurora/client/cli/update.py
<https://reviews.apache.org/r/32276/#comment125032>

You can use frozenset here as well. It's a shame thrift sets are mutable by 
default (which causes them to be unhashable).


- Kevin Sweeney


On March 19, 2015, 6:53 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32276/
> ---
> 
> (Updated March 19, 2015, 6:53 p.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes this error:
> ```
> $ aurora update list devcluster/vagrant/test/http_example --status active
> Traceback (most recent call last):
>   File "/usr/local/bin/aurora/.bootstrap/_pex/pex.py", line 272, in execute
> self.execute_entry(entry_point, args)
>   File "/usr/local/bin/aurora/.bootstrap/_pex/pex.py", line 320, in 
> execute_entry
> runner(entry_point)
>   File "/usr/local/bin/aurora/.bootstrap/_pex/pex.py", line 343, in 
> execute_pkg_resources
> runner()
>   File "/usr/local/bin/aurora/apache/aurora/client/cli/client.py", line 95, 
> in proxy_main
>   File "/usr/local/bin/aurora/apache/aurora/client/cli/__init__.py", line 
> 329, in execute
>   File "/usr/local/bin/aurora/apache/aurora/client/cli/__init__.py", line 
> 306, in _execute
>   File "/usr/local/bin/aurora/apache/aurora/client/cli/__init__.py", line 
> 382, in execute
>   File "apache/aurora/client/cli/update.py", line 315, in execute
> TypeError: unhashable type: 'set'
> ```
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/update.py 
> 2168e99a315dd2916086100589c8345cd3a2c4ff 
> 
> Diff: https://reviews.apache.org/r/32276/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 32171: Change "update list" subcommand to accept a hierarchy.

[Kevin Sweeney]

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



src/test/python/apache/aurora/client/cli/test_supdate.py
<https://reviews.apache.org/r/32171/#comment124532>

How would you feel about including all of the currently available data with 
a header row?

Also instead of raw epoch time how about ISO 8601 - using the spaceless 'T' 
form (`2015-03-15T13:27:36+00:00`)


- Kevin Sweeney


On March 17, 2015, 4:54 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32171/
> ---
> 
> (Updated March 17, 2015, 4:54 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: AURORA-1168
> https://issues.apache.org/jira/browse/AURORA-1168
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> NOTE: I also am proposing with this change that the output format of `aurora 
> update list` be line-oriented and more concise (see test changes for 
> examples).  The idea is to follow up ~immediately with AURORA-1206.  The 
> proposal is for this command to let you retrieve update identifiers, and 
> `update status` (whose name may change) will allow you to drill into updates.
> 
> I'm open to input on this.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/update.py 
> f025d46d50592156e2455313890e981722ab63a5 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> cb66439a778349fc5add4985a7395655c9e1328a 
> 
> Diff: https://reviews.apache.org/r/32171/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 32175: Add a test to ensure annotations exist for AuroraSchedulerManager

[Kevin Sweeney]

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

(Updated March 17, 2015, 2:59 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

Appease the bot.


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


Repository: aurora


Description
---

This addresses review feedback from https://reviews.apache.org/r/32141/ and 
prevents divergence when api.thrift changes.


Diffs (updated)
-

  
src/test/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdminTest.java
 PRE-CREATION 

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


Testing
---

./gradlew build


Thanks,

Kevin Sweeney

Review Request 32175: Add a test to ensure annotations exist for AuroraSchedulerManager

[Kevin Sweeney]

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

Review request for Aurora, Joshua Cohen and Bill Farner.


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


Repository: aurora


Description
---

This addresses review feedback from https://reviews.apache.org/r/32141/ and 
prevents divergence when api.thrift changes.


Diffs
-

  
src/test/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdminTest.java
 PRE-CREATION 

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


Testing
---

./gradlew build


Thanks,

Kevin Sweeney

Re: Review Request 32141: Introduce AnnotatedAuroraAdmin superclass for annotations.

[Kevin Sweeney]

> On March 17, 2015, 12:56 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java,
> >  line 39
> > <https://reviews.apache.org/r/32141/diff/5/?file=898006#file898006line39>
> >
> > Can you add a corresponding comment to api.thrift that any new methods 
> > (or parameters?) added should be added (or updated?) here.

I'll followup with a test case that ensures all AuroraSchedulerManager.Iface 
methods are represented here.


- Kevin


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


On March 17, 2015, 1:03 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32141/
> ---
> 
> (Updated March 17, 2015, 1:03 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-1187
> https://issues.apache.org/jira/browse/AURORA-1187
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add an AnnotatedAuroraAdmin subclass for AuroraAdmin.Iface and change code to 
> inherit from it. This gives us a place to put annotations like the 
> AuthorizingParam one introduced in this review without needing to copy-paste 
> them when we override a new method. A future diff will use these annotations 
> to determine which permission a method call needs by inspecting the annotated 
> parameter. I created a new interface to enable DRY - otherwise I'd need to 
> annotate both ForwardingThrift and SchedulerThriftInterface and keep them in 
> sync.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  6b15bfdf727e2db57327936a0341d5dce98bd47c 
>   src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
> d9184eb540b82c988e7ac590d5cff441f37e62fb 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
>  f0e40b646092e96955fddc46c3a1e62a8237b00f 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
> df6b53a524b005cd5fabb099fd0c08d83e3b287d 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptorTest.java
>  ee98f66de7f671018fa0a0b4894f114c7a283eda 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> 3900c2228038668576cdbb37e87127246a33317c 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> 7b1bf2ef8b413d2c1a08b41722a04af091305304 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/ServerInfoInterceptorTest.java
>  7e20aaa6836bd205261afe5b1244fb6af8a56356 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java
>  aae5cd7709abe3896c2ae06c218a0c90ca11c576 
> 
> Diff: https://reviews.apache.org/r/32141/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 32141: Introduce AnnotatedAuroraAdmin superclass for annotations.

[Kevin Sweeney]

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

(Updated March 17, 2015, 1:03 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

Fix bad rebase.


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


Repository: aurora


Description
---

Add an AnnotatedAuroraAdmin subclass for AuroraAdmin.Iface and change code to 
inherit from it. This gives us a place to put annotations like the 
AuthorizingParam one introduced in this review without needing to copy-paste 
them when we override a new method. A future diff will use these annotations to 
determine which permission a method call needs by inspecting the annotated 
parameter. I created a new interface to enable DRY - otherwise I'd need to 
annotate both ForwardingThrift and SchedulerThriftInterface and keep them in 
sync.


Diffs (updated)
-

  
src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
6b15bfdf727e2db57327936a0341d5dce98bd47c 
  src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
d9184eb540b82c988e7ac590d5cff441f37e62fb 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
f0e40b646092e96955fddc46c3a1e62a8237b00f 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
df6b53a524b005cd5fabb099fd0c08d83e3b287d 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptorTest.java
 ee98f66de7f671018fa0a0b4894f114c7a283eda 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
3900c2228038668576cdbb37e87127246a33317c 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
7b1bf2ef8b413d2c1a08b41722a04af091305304 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/ServerInfoInterceptorTest.java
 7e20aaa6836bd205261afe5b1244fb6af8a56356 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java
 aae5cd7709abe3896c2ae06c218a0c90ca11c576 

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


Testing
---

./gradlew build


Thanks,

Kevin Sweeney

Re: Review Request 32141: Introduce AnnotatedAuroraAdmin superclass for annotations.

[Kevin Sweeney]

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

(Updated March 17, 2015, 12:41 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

rebase


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


Repository: aurora


Description
---

Add an AnnotatedAuroraAdmin subclass for AuroraAdmin.Iface and change code to 
inherit from it. This gives us a place to put annotations like the 
AuthorizingParam one introduced in this review without needing to copy-paste 
them when we override a new method. A future diff will use these annotations to 
determine which permission a method call needs by inspecting the annotated 
parameter. I created a new interface to enable DRY - otherwise I'd need to 
annotate both ForwardingThrift and SchedulerThriftInterface and keep them in 
sync.


Diffs (updated)
-

  
src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
6b15bfdf727e2db57327936a0341d5dce98bd47c 
  src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
d9184eb540b82c988e7ac590d5cff441f37e62fb 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
f0e40b646092e96955fddc46c3a1e62a8237b00f 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
df6b53a524b005cd5fabb099fd0c08d83e3b287d 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptorTest.java
 ee98f66de7f671018fa0a0b4894f114c7a283eda 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
3900c2228038668576cdbb37e87127246a33317c 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
7b1bf2ef8b413d2c1a08b41722a04af091305304 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/ServerInfoInterceptorTest.java
 7e20aaa6836bd205261afe5b1244fb6af8a56356 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java
 aae5cd7709abe3896c2ae06c218a0c90ca11c576 

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


Testing
---

./gradlew build


Thanks,

Kevin Sweeney

Re: Review Request 32141: Introduce AnnotatedAuroraAdmin superclass for annotations.

[Kevin Sweeney]

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

(Updated March 17, 2015, 12:18 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

Remove @Inherited, which is only defined to have any meaning for class 
annotations.


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


Repository: aurora


Description
---

Add an AnnotatedAuroraAdmin subclass for AuroraAdmin.Iface and change code to 
inherit from it. This gives us a place to put annotations like the 
AuthorizingParam one introduced in this review without needing to copy-paste 
them when we override a new method. A future diff will use these annotations to 
determine which permission a method call needs by inspecting the annotated 
parameter. I created a new interface to enable DRY - otherwise I'd need to 
annotate both ForwardingThrift and SchedulerThriftInterface and keep them in 
sync.


Diffs (updated)
-

  
src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
c60d1e78e0f5c474ff0dcf6393e0c305d865bded 
  src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
b1d633602f3fb021a026a087b91d41580798eeaf 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
f0e40b646092e96955fddc46c3a1e62a8237b00f 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
df6b53a524b005cd5fabb099fd0c08d83e3b287d 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptorTest.java
 ee98f66de7f671018fa0a0b4894f114c7a283eda 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
3900c2228038668576cdbb37e87127246a33317c 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
7b1bf2ef8b413d2c1a08b41722a04af091305304 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/ServerInfoInterceptorTest.java
 7e20aaa6836bd205261afe5b1244fb6af8a56356 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java
 aae5cd7709abe3896c2ae06c218a0c90ca11c576 

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


Testing
---

./gradlew build


Thanks,

Kevin Sweeney

Re: Review Request 32141: Introduce AnnotatedAuroraAdmin superclass for annotations.

[Kevin Sweeney]

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

(Updated March 17, 2015, 12:16 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

More nullables.


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


Repository: aurora


Description
---

Add an AnnotatedAuroraAdmin subclass for AuroraAdmin.Iface and change code to 
inherit from it. This gives us a place to put annotations like the 
AuthorizingParam one introduced in this review without needing to copy-paste 
them when we override a new method. A future diff will use these annotations to 
determine which permission a method call needs by inspecting the annotated 
parameter. I created a new interface to enable DRY - otherwise I'd need to 
annotate both ForwardingThrift and SchedulerThriftInterface and keep them in 
sync.


Diffs (updated)
-

  
src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
c60d1e78e0f5c474ff0dcf6393e0c305d865bded 
  src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
b1d633602f3fb021a026a087b91d41580798eeaf 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
f0e40b646092e96955fddc46c3a1e62a8237b00f 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
df6b53a524b005cd5fabb099fd0c08d83e3b287d 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptorTest.java
 ee98f66de7f671018fa0a0b4894f114c7a283eda 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
3900c2228038668576cdbb37e87127246a33317c 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
7b1bf2ef8b413d2c1a08b41722a04af091305304 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/ServerInfoInterceptorTest.java
 7e20aaa6836bd205261afe5b1244fb6af8a56356 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java
 aae5cd7709abe3896c2ae06c218a0c90ca11c576 

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


Testing
---

./gradlew build


Thanks,

Kevin Sweeney

Re: Review Request 32141: Introduce AnnotatedAuroraAdmin superclass for annotations.

[Kevin Sweeney]

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

(Updated March 17, 2015, 11:55 a.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

Bill and Maxim's feedback.


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


Repository: aurora


Description
---

Add an AnnotatedAuroraAdmin subclass for AuroraAdmin.Iface and change code to 
inherit from it. This gives us a place to put annotations like the 
AuthorizingParam one introduced in this review without needing to copy-paste 
them when we override a new method. A future diff will use these annotations to 
determine which permission a method call needs by inspecting the annotated 
parameter. I created a new interface to enable DRY - otherwise I'd need to 
annotate both ForwardingThrift and SchedulerThriftInterface and keep them in 
sync.


Diffs (updated)
-

  
src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
c60d1e78e0f5c474ff0dcf6393e0c305d865bded 
  src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
b1d633602f3fb021a026a087b91d41580798eeaf 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
f0e40b646092e96955fddc46c3a1e62a8237b00f 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
df6b53a524b005cd5fabb099fd0c08d83e3b287d 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptorTest.java
 ee98f66de7f671018fa0a0b4894f114c7a283eda 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
3900c2228038668576cdbb37e87127246a33317c 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
7b1bf2ef8b413d2c1a08b41722a04af091305304 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/ServerInfoInterceptorTest.java
 7e20aaa6836bd205261afe5b1244fb6af8a56356 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java
 aae5cd7709abe3896c2ae06c218a0c90ca11c576 

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


Testing
---

./gradlew build


Thanks,

Kevin Sweeney

Re: Review Request 32141: Introduce AnnotatedAuroraAdmin superclass for annotations.

[Kevin Sweeney]

> On March 17, 2015, 7:59 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java,
> >  line 27
> > <https://reviews.apache.org/r/32141/diff/1/?file=897129#file897129line27>
> >
> >  for breaks.  check out the rendered javadoc in intellij to see what 
> > i mean.
> > 
> > ditto in other files

fixed


- Kevin


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


On March 16, 2015, 6:29 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32141/
> ---
> 
> (Updated March 16, 2015, 6:29 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-1187
> https://issues.apache.org/jira/browse/AURORA-1187
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add an AnnotatedAuroraAdmin subclass for AuroraAdmin.Iface and change code to 
> inherit from it. This gives us a place to put annotations like the 
> AuthorizingParam one introduced in this review without needing to copy-paste 
> them when we override a new method. A future diff will use these annotations 
> to determine which permission a method call needs by inspecting the annotated 
> parameter. I created a new interface to enable DRY - otherwise I'd need to 
> annotate both ForwardingThrift and SchedulerThriftInterface and keep them in 
> sync.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  c60d1e78e0f5c474ff0dcf6393e0c305d865bded 
>   src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
> b1d633602f3fb021a026a087b91d41580798eeaf 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
>  f0e40b646092e96955fddc46c3a1e62a8237b00f 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
> df6b53a524b005cd5fabb099fd0c08d83e3b287d 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptorTest.java
>  ee98f66de7f671018fa0a0b4894f114c7a283eda 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> 3900c2228038668576cdbb37e87127246a33317c 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> 7b1bf2ef8b413d2c1a08b41722a04af091305304 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/ServerInfoInterceptorTest.java
>  7e20aaa6836bd205261afe5b1244fb6af8a56356 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java
>  aae5cd7709abe3896c2ae06c218a0c90ca11c576 
> 
> Diff: https://reviews.apache.org/r/32141/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 32141: Introduce AnnotatedAuroraAdmin superclass for annotations.

[Kevin Sweeney]

> On March 17, 2015, 9:31 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java,
> >  line 60
> > <https://reviews.apache.org/r/32141/diff/1/?file=897132#file897132line60>
> >
> > What's the guidance behind usage of @Nullable here? Technically, the 
> > `shardsId` could come as NULL from thrift as well as `TaskQuery` in 
> > `killTasks`. Both are required for RPC operation but the latter one is 
> > annotated with @Nullable.

This is a good point - I was mistaken about the codegen's handling of null 
collection types. Added Nullable here as well.


- Kevin


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


On March 16, 2015, 6:29 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32141/
> ---
> 
> (Updated March 16, 2015, 6:29 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-1187
> https://issues.apache.org/jira/browse/AURORA-1187
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add an AnnotatedAuroraAdmin subclass for AuroraAdmin.Iface and change code to 
> inherit from it. This gives us a place to put annotations like the 
> AuthorizingParam one introduced in this review without needing to copy-paste 
> them when we override a new method. A future diff will use these annotations 
> to determine which permission a method call needs by inspecting the annotated 
> parameter. I created a new interface to enable DRY - otherwise I'd need to 
> annotate both ForwardingThrift and SchedulerThriftInterface and keep them in 
> sync.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  c60d1e78e0f5c474ff0dcf6393e0c305d865bded 
>   src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
> b1d633602f3fb021a026a087b91d41580798eeaf 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
>  f0e40b646092e96955fddc46c3a1e62a8237b00f 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
> df6b53a524b005cd5fabb099fd0c08d83e3b287d 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptorTest.java
>  ee98f66de7f671018fa0a0b4894f114c7a283eda 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> 3900c2228038668576cdbb37e87127246a33317c 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> 7b1bf2ef8b413d2c1a08b41722a04af091305304 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/ServerInfoInterceptorTest.java
>  7e20aaa6836bd205261afe5b1244fb6af8a56356 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java
>  aae5cd7709abe3896c2ae06c218a0c90ca11c576 
> 
> Diff: https://reviews.apache.org/r/32141/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Review Request 32141: Introduce AnnotatedAuroraAdmin superclass for annotations.

[Kevin Sweeney]

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

Review request for Aurora, Joshua Cohen and Bill Farner.


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


Repository: aurora


Description
---

Add an AnnotatedAuroraAdmin subclass for AuroraAdmin.Iface and change code to 
inherit from it. This gives us a place to put annotations like the 
AuthorizingParam one introduced in this review without needing to copy-paste 
them when we override a new method. A future diff will use these annotations to 
determine which permission a method call needs by inspecting the annotated 
parameter. I created a new interface to enable DRY - otherwise I'd need to 
annotate both ForwardingThrift and SchedulerThriftInterface and keep them in 
sync.


Diffs
-

  
src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
c60d1e78e0f5c474ff0dcf6393e0c305d865bded 
  src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
b1d633602f3fb021a026a087b91d41580798eeaf 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
f0e40b646092e96955fddc46c3a1e62a8237b00f 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
df6b53a524b005cd5fabb099fd0c08d83e3b287d 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptorTest.java
 ee98f66de7f671018fa0a0b4894f114c7a283eda 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
3900c2228038668576cdbb37e87127246a33317c 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
7b1bf2ef8b413d2c1a08b41722a04af091305304 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/ServerInfoInterceptorTest.java
 7e20aaa6836bd205261afe5b1244fb6af8a56356 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java
 aae5cd7709abe3896c2ae06c218a0c90ca11c576 

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


Testing
---

./gradlew build


Thanks,

Kevin Sweeney

Review Request 32118: Move creation of auth_module into ThriftAuthModule.

[Kevin Sweeney]

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

Review request for Aurora, Joshua Cohen and Bill Farner.


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


Repository: aurora


Description
---

Move creation of auth_module into ThriftAuthModule.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/app/Modules.java 
e86e35f5435e7e83956fd80c8d3ae39eb50c9170 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
c6f8787c894533327114425ba17bca966f07b554 
  src/main/java/org/apache/aurora/scheduler/thrift/auth/ThriftAuthModule.java 
e390af12669ac9464e2e7f6e9e1288136ed5ed9b 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
bb19fc72cafc1640f6331c5eaa3761da1a4af7c0 

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


Testing
---

./gradlew build


Thanks,

Kevin Sweeney

Re: Review Request 32055: Add a flag to configure Shiro at runtime.

[Kevin Sweeney]

> On March 13, 2015, 3:19 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java,
> >  line 28
> > <https://reviews.apache.org/r/32055/diff/4/?file=894636#file894636line28>
> >
> > Can you update SchedulerMain to take advantage of this?  Feel free to 
> > TODO if the answer is "yes but not now".

sure, I'll do that in a follow-up.


- Kevin


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


On March 13, 2015, 1:31 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32055/
> ---
> 
> (Updated March 13, 2015, 1:31 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-809
> https://issues.apache.org/jira/browse/AURORA-809
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add a flag to configure Shiro at runtime.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
>  1c3ea24d8d4751f62e1bea260f8823f039e56e10 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/IniShiroRealmModule.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
>  2876b1f0595fc05fe76718a69ae280b4ce7d7178 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ModuleParserTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/32055/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 32055: Add a flag to configure Shiro at runtime.

[Kevin Sweeney]

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

(Updated March 13, 2015, 1:31 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

Add comment.


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


Repository: aurora


Description
---

Add a flag to configure Shiro at runtime.


Diffs (updated)
-

  
src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
 1c3ea24d8d4751f62e1bea260f8823f039e56e10 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/IniShiroRealmModule.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
2876b1f0595fc05fe76718a69ae280b4ce7d7178 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ModuleParserTest.java
 PRE-CREATION 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney

Re: Review Request 32055: Add a flag to configure Shiro at runtime.

[Kevin Sweeney]

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

(Updated March 13, 2015, 1:26 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

Fix PMD warning.


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


Repository: aurora


Description
---

Add a flag to configure Shiro at runtime.


Diffs (updated)
-

  
src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
 1c3ea24d8d4751f62e1bea260f8823f039e56e10 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/IniShiroRealmModule.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
2876b1f0595fc05fe76718a69ae280b4ce7d7178 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ModuleParserTest.java
 PRE-CREATION 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney

Re: Review Request 32055: Add a flag to configure Shiro at runtime.

[Kevin Sweeney]

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

(Updated March 13, 2015, 1:18 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

License headers.


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


Repository: aurora


Description
---

Add a flag to configure Shiro at runtime.


Diffs (updated)
-

  
src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
 1c3ea24d8d4751f62e1bea260f8823f039e56e10 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/IniShiroRealmModule.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
2876b1f0595fc05fe76718a69ae280b4ce7d7178 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ModuleParserTest.java
 PRE-CREATION 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney

Re: Review Request 32055: Add a flag to configure Shiro at runtime.

[Kevin Sweeney]

> On March 13, 2015, 12:59 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java,
> >  line 95
> > <https://reviews.apache.org/r/32055/diff/1/?file=894545#file894545line95>
> >
> > I thought it's generally preferable when installing user-supplied 
> > modules to do so within a private module to sandbox the module?

it is, but then multibinders don't work: 
https://groups.google.com/forum/#!topic/google-guice/h70a9pwD6_g


> On March 13, 2015, 12:59 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java,
> >  line 15
> > <https://reviews.apache.org/r/32055/diff/1/?file=894547#file894547line15>
> >
> > Do we need this? Can we just use an arg like... Arg > extends Module>> and rely on the existing ClassParser to give us the type, 
> > then just call newInstance() on that?

basically this boilerplate will appear everywhere we do this - seems reasonable 
to standardize on a parser to make the error messages nice. makes unit tests 
easier as well as we can pass instances of module (usually AICs) instead of a 
class object.


- Kevin


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


On March 13, 2015, 12:38 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32055/
> ---
> 
> (Updated March 13, 2015, 12:38 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-809
> https://issues.apache.org/jira/browse/AURORA-809
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add a flag to configure Shiro at runtime.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
>  1c3ea24d8d4751f62e1bea260f8823f039e56e10 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/IniShiroRealmModule.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
>  2876b1f0595fc05fe76718a69ae280b4ce7d7178 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ModuleParserTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/32055/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Review Request 32055: Add a flag to configure Shiro at runtime.

[Kevin Sweeney]

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

Review request for Aurora, Joshua Cohen and Bill Farner.


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


Repository: aurora


Description
---

Add a flag to configure Shiro at runtime.


Diffs
-

  
src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
 1c3ea24d8d4751f62e1bea260f8823f039e56e10 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/IniShiroRealmModule.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
2876b1f0595fc05fe76718a69ae280b4ce7d7178 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ModuleParserTest.java
 PRE-CREATION 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney

Re: Review Request 32051: Detect an invalid .auroraversion expansion and provide a helpful error message.

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On March 13, 2015, 9:41 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32051/
> ---
> 
> (Updated March 13, 2015, 9:41 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1169
> https://issues.apache.org/jira/browse/AURORA-1169
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Help users notice and make progress more quickly when fatefully attempting to 
> build with a Windows host OS.
> 
> An alternative approach would be to not rely on the symlink here, but i'm not 
> confident that this would kick the can down the road to one of the numerous 
> other places we rely on symlinks.
> 
> This does not fix AURORA-1169, but i propose that we do this to make it clear 
> earlier that Windows as a build platform is not currently supported.  Given 
> the number of native dependnecies we have, i think this is the most sane 
> course for the project today.
> 
> 
> Diffs
> -
> 
>   build.gradle dc366d4339b83af9e1fea663fe8e5860702644e9 
> 
> Diff: https://reviews.apache.org/r/32051/diff/
> 
> 
> Testing
> ---
> 
> Ran the build with a dummy version value, got this:
> 
> ```
> $ ./gradlew build
> :buildSrc:compileJava UP-TO-DATE
> :buildSrc:compileGroovy UP-TO-DATE
> :buildSrc:processResources UP-TO-DATE
> :buildSrc:classes UP-TO-DATE
> :buildSrc:jar UP-TO-DATE
> :buildSrc:assemble UP-TO-DATE
> :buildSrc:compileTestJava UP-TO-DATE
> :buildSrc:compileTestGroovy UP-TO-DATE
> :buildSrc:processTestResources UP-TO-DATE
> :buildSrc:testClasses UP-TO-DATE
> :buildSrc:test UP-TO-DATE
> :buildSrc:check UP-TO-DATE
> :buildSrc:build UP-TO-DATE
> 
> FAILURE: Build failed with an exception.
> 
> * Where:
> Build file '/home/wfarner/code/aurora/build.gradle' line: 53
> 
> * What went wrong:
> A problem occurred evaluating root project 'aurora'.
> > 
>   
> ***
>   The application version string read by gradle is invalid.  This is known to
>   happen when building on Windows, or within vagrant with a Windows host.
>   You can work around this issue by cloning git _within_ vagrant and building
>   from there.
>   
>   For more details, please see 
> https://issues.apache.org/jira/browse/AURORA-1169
>   
> ***
> 
> 
> * Try:
> Run with --stacktrace option to get the stack trace. Run with --info or 
> --debug option to get more log output.
> 
> BUILD FAILED
> 
> Total time: 6.14 secs
> ```
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31869: Catch only known Exception types in the client.

[Kevin Sweeney]

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

Ship it!


LGTM once `self` or `cls` are used in place of class names.

- Kevin Sweeney


On March 10, 2015, 5:17 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31869/
> ---
> 
> (Updated March 10, 2015, 5:17 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Zameer Manji.
> 
> 
> Bugs: AURORA-1176
> https://issues.apache.org/jira/browse/AURORA-1176
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> As indicated by some changes in tests - there were legitimate issues hiding 
> behind this catch.  After this change, the client will allow unhandled 
> exceptions to surface in their full glory.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 194629f61192c1d7d5e7064e9226adf26d03e890 
>   src/main/python/apache/aurora/client/cli/__init__.py 
> 4d9ef09749e3075b9d9e2ae1db311e60f7bdb4ee 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 286b2182d5fe25703882f0b367739ad03d6c8fe8 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> b855c3c2d74125738d2106e18a9e9b0ebed6ac4b 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 459d157155f74b6a3d140b85d3b7f0364367 
>   src/test/python/apache/aurora/client/cli/test_kill.py 
> 7aad34a2fe5591937c5bca890751073439e3a1a6 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> 1806769426a196793481f948892f5474df8dd665 
>   src/test/python/apache/aurora/client/cli/util.py 
> b65970a2717a1f36767c61e5e09c980b04895f01 
> 
> Diff: https://reviews.apache.org/r/31869/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31869: Catch only known Exception types in the client.

[Kevin Sweeney]

> On March 10, 2015, 5:08 p.m., Kevin Sweeney wrote:
> > src/main/python/apache/aurora/client/cli/__init__.py, line 282
> > <https://reviews.apache.org/r/31869/diff/3/?file=890382#file890382line282>
> >
> > My understanding was that zmanji was attempting to remove 
> > AuroraCommandContext and patch print instead. Perhaps you and him should 
> > come to a consensus.
> 
> Bill Farner wrote:
> Did he not express agreement by giving a ship-it on this diff?
> 
> Zameer Manji wrote:
> We can address that problem another time. Consistent use of context and 
> print_error will help solve that problem later.

Aha didn't see that. Carry on then.


- Kevin


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


On March 10, 2015, 5:17 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31869/
> -------
> 
> (Updated March 10, 2015, 5:17 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Zameer Manji.
> 
> 
> Bugs: AURORA-1176
> https://issues.apache.org/jira/browse/AURORA-1176
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> As indicated by some changes in tests - there were legitimate issues hiding 
> behind this catch.  After this change, the client will allow unhandled 
> exceptions to surface in their full glory.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 194629f61192c1d7d5e7064e9226adf26d03e890 
>   src/main/python/apache/aurora/client/cli/__init__.py 
> 4d9ef09749e3075b9d9e2ae1db311e60f7bdb4ee 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 286b2182d5fe25703882f0b367739ad03d6c8fe8 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> b855c3c2d74125738d2106e18a9e9b0ebed6ac4b 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 459d157155f74b6a3d140b85d3b7f0364367 
>   src/test/python/apache/aurora/client/cli/test_kill.py 
> 7aad34a2fe5591937c5bca890751073439e3a1a6 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> 1806769426a196793481f948892f5474df8dd665 
>   src/test/python/apache/aurora/client/cli/util.py 
> b65970a2717a1f36767c61e5e09c980b04895f01 
> 
> Diff: https://reviews.apache.org/r/31869/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31869: Catch only known Exception types in the client.

[Kevin Sweeney]

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



src/main/python/apache/aurora/client/api/__init__.py
<https://reviews.apache.org/r/31869/#comment123343>

Revert this - self will correctly find the class field.



src/main/python/apache/aurora/client/cli/__init__.py
<https://reviews.apache.org/r/31869/#comment123344>

My understanding was that zmanji was attempting to remove 
AuroraCommandContext and patch print instead. Perhaps you and him should come 
to a consensus.



src/main/python/apache/aurora/client/cli/jobs.py
<https://reviews.apache.org/r/31869/#comment123346>

revert to use self so that subclass delegation will work.


- Kevin Sweeney


On March 10, 2015, 10:44 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31869/
> ---
> 
> (Updated March 10, 2015, 10:44 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Zameer Manji.
> 
> 
> Bugs: AURORA-1176
> https://issues.apache.org/jira/browse/AURORA-1176
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> As indicated by some changes in tests - there were legitimate issues hiding 
> behind this catch.  After this change, the client will allow unhandled 
> exceptions to surface in their full glory.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 194629f61192c1d7d5e7064e9226adf26d03e890 
>   src/main/python/apache/aurora/client/cli/__init__.py 
> 4d9ef09749e3075b9d9e2ae1db311e60f7bdb4ee 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 286b2182d5fe25703882f0b367739ad03d6c8fe8 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> b855c3c2d74125738d2106e18a9e9b0ebed6ac4b 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 459d157155f74b6a3d140b85d3b7f0364367 
>   src/test/python/apache/aurora/client/cli/test_kill.py 
> 7aad34a2fe5591937c5bca890751073439e3a1a6 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> 1806769426a196793481f948892f5474df8dd665 
>   src/test/python/apache/aurora/client/cli/util.py 
> b65970a2717a1f36767c61e5e09c980b04895f01 
> 
> Diff: https://reviews.apache.org/r/31869/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31820: Support HTTP Basic auth and shiro.ini configuration

[Kevin Sweeney]

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

(Updated March 10, 2015, 12:45 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

Josh's feedback.


Bugs: AURORA-809 and AURORA-811
https://issues.apache.org/jira/browse/AURORA-809
https://issues.apache.org/jira/browse/AURORA-811


Repository: aurora


Description
---

* Add dependency on Apache Shiro.
* HTTP Basic Authentication.
* Authorization based on shiro.ini.
* Sample shiro.ini for local mode.


Diffs (updated)
-

  build.gradle b723118e84038a237b368ef4e3fe9549cd4b2854 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
24b61c1e4f615295acf28d904588e1512972d3f4 
  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
8a59d89c07b406ce98076ca7ee51b958599a39ec 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroIniParser.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
3e7483b1e4e674397fd093f1e301d9cb2d3ca166 
  src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
1e4ba014804b56a2ea02770d09beb63faaabf684 
  src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 
640acdf4e73f99418473ca97bcdc4f5f4c190f10 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroIniParserTest.java
 PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java
 PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
52fe0ea063dbc7a71a20926630bf449dbd936306 
  
src/test/resources/org/apache/aurora/scheduler/http/api/security/shiro-example.ini
 PRE-CREATION 
  
src/test/resources/org/apache/aurora/scheduler/http/api/security/shiro-malformed-extra-sections.ini
 PRE-CREATION 
  
src/test/resources/org/apache/aurora/scheduler/http/api/security/shiro-malformed-missing-sections.ini
 PRE-CREATION 

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


Testing
---

./gradlew -Pq build

Local testing in the UI and with cURL.

Updates to e2e test and Vagrant environment to follow.


Thanks,

Kevin Sweeney

Re: Review Request 31820: Support HTTP Basic auth and shiro.ini configuration

[Kevin Sweeney]

> On March 10, 2015, 11:48 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java,
> >  lines 59-62
> > <https://reviews.apache.org/r/31820/diff/3/?file=889978#file889978line59>
> >
> > Maybe move this to the ticket and kill the commented out code here?

Done.


> On March 10, 2015, 11:48 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroIniParser.java,
> >  lines 38-39
> > <https://reviews.apache.org/r/31820/diff/3/?file=889979#file889979line38>
> >
> > nit: probably preferable to move ImmutableSortedSet.of down to the next 
> > line.

Fixed.


> On March 10, 2015, 11:48 a.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/common/clusters.py, line 92
> > <https://reviews.apache.org/r/31820/diff/3/?file=889983#file889983line92>
> >
> > Is this change related?

Not sure what happened, rebased.


- Kevin


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


On March 10, 2015, 12:45 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31820/
> ---
> 
> (Updated March 10, 2015, 12:45 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-809 and AURORA-811
> https://issues.apache.org/jira/browse/AURORA-809
> https://issues.apache.org/jira/browse/AURORA-811
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> * Add dependency on Apache Shiro.
> * HTTP Basic Authentication.
> * Authorization based on shiro.ini.
> * Sample shiro.ini for local mode.
> 
> 
> Diffs
> -
> 
>   build.gradle b723118e84038a237b368ef4e3fe9549cd4b2854 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 24b61c1e4f615295acf28d904588e1512972d3f4 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 8a59d89c07b406ce98076ca7ee51b958599a39ec 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroIniParser.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
> 3e7483b1e4e674397fd093f1e301d9cb2d3ca166 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
> 1e4ba014804b56a2ea02770d09beb63faaabf684 
>   src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 
> 640acdf4e73f99418473ca97bcdc4f5f4c190f10 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroIniParserTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> 52fe0ea063dbc7a71a20926630bf449dbd936306 
>   
> src/test/resources/org/apache/aurora/scheduler/http/api/security/shiro-example.ini
>  PRE-CREATION 
>   
> src/test/resources/org/apache/aurora/scheduler/http/api/security/shiro-malformed-extra-sections.ini
>  PRE-CREATION 
>   
> src/test/resources/org/apache/aurora/scheduler/http/api/security/shiro-malformed-missing-sections.ini
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31820/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> Local testing in the UI and with cURL.
> 
> Updates to e2e test and Vagrant environment to follow.
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 31820: Support HTTP Basic auth and shiro.ini configuration

[Kevin Sweeney]

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

(Updated March 9, 2015, 6:18 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

Fix checkstyle.


Bugs: AURORA-809 and AURORA-811
https://issues.apache.org/jira/browse/AURORA-809
https://issues.apache.org/jira/browse/AURORA-811


Repository: aurora


Description
---

* Add dependency on Apache Shiro.
* HTTP Basic Authentication.
* Authorization based on shiro.ini.
* Sample shiro.ini for local mode.


Diffs (updated)
-

  build.gradle b723118e84038a237b368ef4e3fe9549cd4b2854 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
24b61c1e4f615295acf28d904588e1512972d3f4 
  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
8a59d89c07b406ce98076ca7ee51b958599a39ec 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroIniParser.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
3e7483b1e4e674397fd093f1e301d9cb2d3ca166 
  src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
1e4ba014804b56a2ea02770d09beb63faaabf684 
  src/main/python/apache/aurora/common/clusters.py 
c4b7fefca30313b281808478bf23158a9b7fbf85 
  src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 
640acdf4e73f99418473ca97bcdc4f5f4c190f10 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroIniParserTest.java
 PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java
 PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
52fe0ea063dbc7a71a20926630bf449dbd936306 
  src/test/python/apache/aurora/common/test_clusters.py 
1bd696e9cd28d87d0cac68b33ab043407d796b61 
  
src/test/resources/org/apache/aurora/scheduler/http/api/security/shiro-example.ini
 PRE-CREATION 
  
src/test/resources/org/apache/aurora/scheduler/http/api/security/shiro-malformed-extra-sections.ini
 PRE-CREATION 
  
src/test/resources/org/apache/aurora/scheduler/http/api/security/shiro-malformed-missing-sections.ini
 PRE-CREATION 

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


Testing
---

./gradlew -Pq build

Local testing in the UI and with cURL.

Updates to e2e test and Vagrant environment to follow.


Thanks,

Kevin Sweeney

Re: Review Request 31820: Support HTTP Basic auth and shiro.ini configuration

[Kevin Sweeney]

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

(Updated March 9, 2015, 5:37 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

Address review feedback.


Bugs: AURORA-809 and AURORA-811
https://issues.apache.org/jira/browse/AURORA-809
https://issues.apache.org/jira/browse/AURORA-811


Repository: aurora


Description
---

* Add dependency on Apache Shiro.
* HTTP Basic Authentication.
* Authorization based on shiro.ini.
* Sample shiro.ini for local mode.


Diffs (updated)
-

  build.gradle b723118e84038a237b368ef4e3fe9549cd4b2854 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
24b61c1e4f615295acf28d904588e1512972d3f4 
  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
8a59d89c07b406ce98076ca7ee51b958599a39ec 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroIniParser.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroIniParserTest.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
3e7483b1e4e674397fd093f1e301d9cb2d3ca166 
  src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
1e4ba014804b56a2ea02770d09beb63faaabf684 
  src/main/python/apache/aurora/common/clusters.py 
c4b7fefca30313b281808478bf23158a9b7fbf85 
  src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 
640acdf4e73f99418473ca97bcdc4f5f4c190f10 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java
 PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
52fe0ea063dbc7a71a20926630bf449dbd936306 
  src/test/python/apache/aurora/common/test_clusters.py 
1bd696e9cd28d87d0cac68b33ab043407d796b61 
  
src/test/resources/org/apache/aurora/scheduler/http/api/security/shiro-example.ini
 PRE-CREATION 
  
src/test/resources/org/apache/aurora/scheduler/http/api/security/shiro-malformed-extra-sections.ini
 PRE-CREATION 
  
src/test/resources/org/apache/aurora/scheduler/http/api/security/shiro-malformed-missing-sections.ini
 PRE-CREATION 

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


Testing
---

./gradlew -Pq build

Local testing in the UI and with cURL.

Updates to e2e test and Vagrant environment to follow.


Thanks,

Kevin Sweeney

Re: Review Request 31820: Support HTTP Basic auth and shiro.ini configuration

[Kevin Sweeney]

> On March 9, 2015, 11:36 a.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java,
> >  line 50
> > <https://reviews.apache.org/r/31820/diff/1/?file=888081#file888081line50>
> >
> > Would it be possible to create a Ini parser for Args instead of just 
> > holding the path?
> 
> Bill Farner wrote:
> +1, this would make for more natural error output in some trivial 
> misconfiguration cases.

Done.


> On March 9, 2015, 11:36 a.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java,
> >  line 63
> > <https://reviews.apache.org/r/31820/diff/1/?file=888082#file888082line63>
> >
> > Can you increment a stat here? It seems these failures would be of 
> > interest to an operator.

Added.


- Kevin


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


On March 6, 2015, 4:54 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31820/
> ---
> 
> (Updated March 6, 2015, 4:54 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-809 and AURORA-811
> https://issues.apache.org/jira/browse/AURORA-809
> https://issues.apache.org/jira/browse/AURORA-811
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> * Add dependency on Apache Shiro.
> * HTTP Basic Authentication.
> * Authorization based on shiro.ini.
> * Sample shiro.ini for local mode.
> 
> 
> Diffs
> -
> 
>   build.gradle b723118e84038a237b368ef4e3fe9549cd4b2854 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 24b61c1e4f615295acf28d904588e1512972d3f4 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 8a59d89c07b406ce98076ca7ee51b958599a39ec 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
> 3e7483b1e4e674397fd093f1e301d9cb2d3ca166 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
> 1e4ba014804b56a2ea02770d09beb63faaabf684 
>   src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 
> 640acdf4e73f99418473ca97bcdc4f5f4c190f10 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> 52fe0ea063dbc7a71a20926630bf449dbd936306 
>   src/test/resources/org/apache/aurora/scheduler/app/local/shiro.ini 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31820/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> Local testing in the UI and with cURL.
> 
> Updates to e2e test and Vagrant environment to follow.
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 31820: Support HTTP Basic auth and shiro.ini configuration

[Kevin Sweeney]

> On March 9, 2015, 1:47 p.m., Bill Farner wrote:
> > build.gradle, line 260
> > <https://reviews.apache.org/r/31820/diff/1/?file=888077#file888077line260>
> >
> > Holy cow, they're on guava 9!?  Hopefully this doesn't burn us, but a 
> > quick grep through their code suggests they only have minimal use of guava. 
> >  Perhaps include a TODO to commit a patch bumping their version?

Added common.


> On March 9, 2015, 1:47 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java,
> >  line 49
> > <https://reviews.apache.org/r/31820/diff/1/?file=888081#file888081line49>
> >
> > s/info/configuration/

Fixed.


> On March 9, 2015, 1:47 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java,
> >  line 107
> > <https://reviews.apache.org/r/31820/diff/1/?file=888084#file888084line107>
> >
> > Please rephrase and possibly add context.  As written, this sounds like 
> > it could be a TODO.

Added.


- Kevin


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


On March 6, 2015, 4:54 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31820/
> ---
> 
> (Updated March 6, 2015, 4:54 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-809 and AURORA-811
> https://issues.apache.org/jira/browse/AURORA-809
> https://issues.apache.org/jira/browse/AURORA-811
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> * Add dependency on Apache Shiro.
> * HTTP Basic Authentication.
> * Authorization based on shiro.ini.
> * Sample shiro.ini for local mode.
> 
> 
> Diffs
> -
> 
>   build.gradle b723118e84038a237b368ef4e3fe9549cd4b2854 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 24b61c1e4f615295acf28d904588e1512972d3f4 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 8a59d89c07b406ce98076ca7ee51b958599a39ec 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
> 3e7483b1e4e674397fd093f1e301d9cb2d3ca166 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
> 1e4ba014804b56a2ea02770d09beb63faaabf684 
>   src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 
> 640acdf4e73f99418473ca97bcdc4f5f4c190f10 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> 52fe0ea063dbc7a71a20926630bf449dbd936306 
>   src/test/resources/org/apache/aurora/scheduler/app/local/shiro.ini 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31820/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> Local testing in the UI and with cURL.
> 
> Updates to e2e test and Vagrant environment to follow.
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 31820: Support HTTP Basic auth and shiro.ini configuration

[Kevin Sweeney]

> On March 9, 2015, 11:27 a.m., Joshua Cohen wrote:
> > Should we add tests for the full scheduler API as well as the admin 
> > interface?
> > 
> > Also, worth adding a unit test for the interceptor?

Added ShiroThriftInterceptorTest.

Not sure what you mean by test the full scheduler API - I check methods on 
ReadOnlyScheduler, AuroraSchedulerManager, and AuroraAdmin in the integration 
test.


> On March 9, 2015, 11:27 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java,
> >  line 41
> > <https://reviews.apache.org/r/31820/diff/1/?file=888081#file888081line41>
> >
> > Seems like most of our guice modules have javadoc. Mind adding one 
> > here? Or has our style changed in this regard w/ the switch to Google's 
> > checkstyle rules?

Added.


> On March 9, 2015, 11:27 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java,
> >  line 50
> > <https://reviews.apache.org/r/31820/diff/1/?file=888081#file888081line50>
> >
> > @CanRead
> > @Exists

Implemented in an ArgParser.


- Kevin


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


On March 6, 2015, 4:54 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31820/
> ---
> 
> (Updated March 6, 2015, 4:54 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-809 and AURORA-811
> https://issues.apache.org/jira/browse/AURORA-809
> https://issues.apache.org/jira/browse/AURORA-811
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> * Add dependency on Apache Shiro.
> * HTTP Basic Authentication.
> * Authorization based on shiro.ini.
> * Sample shiro.ini for local mode.
> 
> 
> Diffs
> -
> 
>   build.gradle b723118e84038a237b368ef4e3fe9549cd4b2854 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 24b61c1e4f615295acf28d904588e1512972d3f4 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 8a59d89c07b406ce98076ca7ee51b958599a39ec 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
> 3e7483b1e4e674397fd093f1e301d9cb2d3ca166 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
> 1e4ba014804b56a2ea02770d09beb63faaabf684 
>   src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 
> 640acdf4e73f99418473ca97bcdc4f5f4c190f10 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> 52fe0ea063dbc7a71a20926630bf449dbd936306 
>   src/test/resources/org/apache/aurora/scheduler/app/local/shiro.ini 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31820/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> Local testing in the UI and with cURL.
> 
> Updates to e2e test and Vagrant environment to follow.
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 31869: Catch only known Exception types in the client.

[Kevin Sweeney]

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



src/main/python/apache/aurora/client/base.py
<https://reviews.apache.org/r/31869/#comment123121>

As this is only raised from a single place prefer declaring it there and 
matching the convention in [the style 
guide](https://github.com/twitter/commons/blob/master/src/python/twitter/common/styleguide.md#best-practices)
 (number 6). I don't have a strong opinion on the merits of this practice, but 
it is a convention across our code whose change I think is orthogonal to the 
purpose of this diff.



src/main/python/apache/aurora/client/base.py
<https://reviews.apache.org/r/31869/#comment123120>

To be clear my suggestion was to remove the constructor and leave the 
docstring. Overriding a constructor without changing its behavior just adds 
noise and is inconsistent with the convention used through most of the codebase.



src/test/python/apache/aurora/client/cli/test_create.py
<https://reviews.apache.org/r/31869/#comment123119>

You don't actually need to use the pytest API for this - since we're on 
python 2.7 and you have already subclassed `TestCase` you can use 
[self.assertRaises](https://docs.python.org/2/library/unittest.html#unittest.TestCase.assertRaises).


- Kevin Sweeney


On March 9, 2015, 1:39 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31869/
> ---
> 
> (Updated March 9, 2015, 1:39 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Zameer Manji.
> 
> 
> Bugs: AURORA-1176
> https://issues.apache.org/jira/browse/AURORA-1176
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> As indicated by some changes in tests - there were legitimate issues hiding 
> behind this catch.  After this change, the client will allow unhandled 
> exceptions to surface in their full glory.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 194629f61192c1d7d5e7064e9226adf26d03e890 
>   src/main/python/apache/aurora/client/base.py 
> d550c8eeed91f0967e281957b71fcefb0b4cf3b8 
>   src/main/python/apache/aurora/client/cli/__init__.py 
> 4d9ef09749e3075b9d9e2ae1db311e60f7bdb4ee 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 286b2182d5fe25703882f0b367739ad03d6c8fe8 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> b855c3c2d74125738d2106e18a9e9b0ebed6ac4b 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 459d157155f74b6a3d140b85d3b7f0364367 
>   src/test/python/apache/aurora/client/cli/test_kill.py 
> 7aad34a2fe5591937c5bca890751073439e3a1a6 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> 1806769426a196793481f948892f5474df8dd665 
>   src/test/python/apache/aurora/client/cli/util.py 
> b65970a2717a1f36767c61e5e09c980b04895f01 
> 
> Diff: https://reviews.apache.org/r/31869/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31814: Include messages with internal job updater state transitions.

[Kevin Sweeney]

> On March 9, 2015, 10:50 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java,
> >  line 291
> > 
> >
> > What's the motivation behind dropping the "Aurora Updater" here?
> 
> Bill Farner wrote:
> I think the magic user value abuses a field that serves a different 
> purpose.  IMHO an API consumer should be able to programmatically determine 
> that the scheduler independently performed an action without resorting to 
> string matching on the magic value of a field.
> 
> In this particular case, i don't think the user field adds signal to what 
> is already present in the state enum value.
> 
> Maxim Khutornenko wrote:
> I still think having a special "Aurora Updater" user clearly visible in 
> the UI makes it way easier to grasp the event origin rather than trying to 
> decipher a particular state meaning. Users may not and should not be familar 
> with the update state diagram in order to understand the cause of the 
> transition. This is especially true in a multi-actor environment where state 
> transitions may come from users (pause/resume/abort), external monitoring 
> service (pause) or the updater itself (pulse expired).
> 
> Bill Farner wrote:
> > I still think having a special "Aurora Updater" user clearly visible in 
> the UI
> 
> I completely agree, but the UI should make that decision, not the 
> scheduler.
> 
> Maxim Khutornenko wrote:
> I see, sure I am fine with that. Do you propose to address it separately? 
> If so, mind filing a ticket?

+1 to removal of the magic string - hacking the ui should be done in the ui.


- Kevin


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


On March 6, 2015, 5:06 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31814/
> ---
> 
> (Updated March 6, 2015, 5:06 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1077
> https://issues.apache.org/jira/browse/AURORA-1077
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Include messages with internal job updater state transitions.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> 09f2a74c9e6f6dbf79f83aa7a387587c593b9f0e 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  acdade3dca807a221b4da975d0310c91884ee752 
>   src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 
> 27a5b9026f5ac3b3bdeb32813b10435bc3dab173 
>   src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java 
> 27e0654bfb90f48b407edda5a0c914e595d9c552 
>   src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
> 66b0e4b7a9b23e3e6c772a21f5adc39e1d1461ad 
>   src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
> 4db0080547d61af1511a4fb62bf88b3bbf819f1e 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> e24d6bde5f3479a75522e825cce4ec6c30c117aa 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> 7d0a7438b4a517e5e0d44f4e99aceb1a6d19f987 
> 
> Diff: https://reviews.apache.org/r/31814/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31869: Catch only known Exception types in the client.

[Kevin Sweeney]

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



src/main/python/apache/aurora/client/api/__init__.py
<https://reviews.apache.org/r/31869/#comment123019>

Why this change (removing the root Error class)?



src/main/python/apache/aurora/client/base.py
<https://reviews.apache.org/r/31869/#comment123016>

Override isn't needed here. You can just leave the class body blank.



src/main/python/apache/aurora/client/cli/jobs.py
<https://reviews.apache.org/r/31869/#comment123024>

Rather than pass this have it use the instance field or make it a 
classmethod.


- Kevin Sweeney


On March 9, 2015, 1:10 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31869/
> ---
> 
> (Updated March 9, 2015, 1:10 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Zameer Manji.
> 
> 
> Bugs: AURORA-1176
> https://issues.apache.org/jira/browse/AURORA-1176
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> As indicated by some changes in tests - there were legitimate issues hiding 
> behind this catch.  After this change, the client will allow unhandled 
> exceptions to surface in their full glory.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 194629f61192c1d7d5e7064e9226adf26d03e890 
>   src/main/python/apache/aurora/client/base.py 
> d550c8eeed91f0967e281957b71fcefb0b4cf3b8 
>   src/main/python/apache/aurora/client/cli/__init__.py 
> 4d9ef09749e3075b9d9e2ae1db311e60f7bdb4ee 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 286b2182d5fe25703882f0b367739ad03d6c8fe8 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> b855c3c2d74125738d2106e18a9e9b0ebed6ac4b 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 459d157155f74b6a3d140b85d3b7f0364367 
>   src/test/python/apache/aurora/client/cli/test_kill.py 
> 7aad34a2fe5591937c5bca890751073439e3a1a6 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> 1806769426a196793481f948892f5474df8dd665 
>   src/test/python/apache/aurora/client/cli/util.py 
> b65970a2717a1f36767c61e5e09c980b04895f01 
> 
> Diff: https://reviews.apache.org/r/31869/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31828: Upgrade gradle and a few plugins.

[Kevin Sweeney]

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

Ship it!


LGTM once reviewbot is happy. It might be choking on the binary diff.

- Kevin Sweeney


On March 7, 2015, 9:54 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31828/
> ---
> 
> (Updated March 7, 2015, 9:54 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Gradle 2.3: http://gradle.org/docs/2.3/release-notes
> Checkstyle: http://checkstyle.sourceforge.net/releasenotes.html
> Can't tell what, if anything, changed in the versions plugin: 
> https://github.com/ben-manes/gradle-versions-plugin/releases
> 
> 
> Diffs
> -
> 
>   build.gradle b723118e84038a237b368ef4e3fe9549cd4b2854 
>   buildSrc/gradle.properties 73341fa90f20d73edf34fc3c945b686dd14bf941 
>   gradle/wrapper/gradle-wrapper.jar 2322723c7ed5f591adfa4c87b6c51932f591249a 
>   gradle/wrapper/gradle-wrapper.properties 
> 531b41ced8b99d418636a18f4f97297722944ea3 
> 
> Diff: https://reviews.apache.org/r/31828/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31525: Improving NearestFit reporting accuracy.

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On March 2, 2015, 3:40 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31525/
> ---
> 
> (Updated March 2, 2015, 3:40 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-1159
> https://issues.apache.org/jira/browse/AURORA-1159
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Instead of relying on a fixed MAX_SCORE, vetoes are now graded by their 
> relative weight (severity) with a dedicated constraint mismatch ranked higest 
> and an insufficient resource mismatch - lowest. Together with "break early" 
> rule in SchedulingFilter, this ensures "heavier" vetoes are given proper 
> attention in the NearestFit. This simplifies `NearestFit` logic while also 
> improving pending reason reporting accuracy and scheduling performance.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
> 3313bd2f9ed5a88375d88715dea5da1e9ad8b963 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> a020ce50d578fba7f32b370f448a49eb1c284147 
>   src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java 
> c3097e49c0f6588ea765aa4fab69dd35e3d90e8b 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  b00668423a53a8cf6f4da3456bce3354f1fd2424 
>   src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 
> 78a236c0f9074692b67ce18e6e03f18fe4529e02 
> 
> Diff: https://reviews.apache.org/r/31525/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Review Request 31820: Support HTTP Basic auth and shiro.ini configuration

[Kevin Sweeney]

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

Review request for Aurora, Joshua Cohen and Bill Farner.


Bugs: AURORA-809 and AURORA-811
https://issues.apache.org/jira/browse/AURORA-809
https://issues.apache.org/jira/browse/AURORA-811


Repository: aurora


Description
---

* Add dependency on Apache Shiro.
* HTTP Basic Authentication.
* Authorization based on shiro.ini.
* Sample shiro.ini for local mode.


Diffs
-

  build.gradle b723118e84038a237b368ef4e3fe9549cd4b2854 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
24b61c1e4f615295acf28d904588e1512972d3f4 
  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
8a59d89c07b406ce98076ca7ee51b958599a39ec 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
3e7483b1e4e674397fd093f1e301d9cb2d3ca166 
  src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
1e4ba014804b56a2ea02770d09beb63faaabf684 
  src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 
640acdf4e73f99418473ca97bcdc4f5f4c190f10 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
52fe0ea063dbc7a71a20926630bf449dbd936306 
  src/test/resources/org/apache/aurora/scheduler/app/local/shiro.ini 
PRE-CREATION 

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


Testing
---

./gradlew -Pq build

Local testing in the UI and with cURL.

Updates to e2e test and Vagrant environment to follow.


Thanks,

Kevin Sweeney

Re: Review Request 31754: Break out API servlet configuration into its own module.

[Kevin Sweeney]

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

(Updated March 5, 2015, 12:03 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

Fix JettyServerModuleTest to use production=false.


Repository: aurora


Description
---

Break out API servlet configuration into its own module.

This is necessary to make a follow-up patch introducing HTTP Basic auth 
testable.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
8a59d89c07b406ce98076ca7ee51b958599a39ec 
  src/main/java/org/apache/aurora/scheduler/http/SchedulerAPIServlet.java 
33ad43b3202e5e9ef5be919b6abc5cbc7f62b660 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
d1ab9b18394ad37fe9dcb131816fcfb2952bf8b6 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
9804bf5549cd2589fc68c57b3895efd89923169b 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
8ec5f9a3810b4deae981988487c6a46a20db72a2 
  src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
80beb258d9f2786668d29db85b1295163a402d42 
  src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
47d54e3c3bb1ba5e0fb26379792f515f25316480 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
5019094333f9807c64a49c29569ade191ee61824 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java PRE-CREATION 

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


Testing
---


Thanks,

Kevin Sweeney

Re: Review Request 31754: Break out API servlet configuration into its own module.

[Kevin Sweeney]

> On March 4, 2015, 7:18 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java, line 141
> > <https://reviews.apache.org/r/31754/diff/1/?file=885268#file885268line141>
> >
> > There seems to be implementation detail leaking here.  Can you make 
> > JettyServerModule hide this away, and force the leak into the unit test 
> > instead?
> 
> Kevin Sweeney wrote:
> Any thoughts on how that should look? Possibly an @VisibleForTesting 
> constructor to JettyServerModule with a boolean production? The test needs to 
> replace the binding for ServletContextListener, which needs a parent Injector 
> handle, since binding a Module isn't allowed due to guice internals.
> 
> Bill Farner wrote:
> +1 to @VisibleForTesting constructor overload.

Done.


> On March 4, 2015, 7:18 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java, line 
> > 178
> > <https://reviews.apache.org/r/31754/diff/1/?file=885269#file885269line178>
> >
> > line break style - arg per line

done.


> On March 4, 2015, 7:18 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/http/SchedulerAPIServlet.java, 
> > line 26
> > <https://reviews.apache.org/r/31754/diff/1/?file=885270#file885270line26>
> >
> > static final

good catch, done.


> On March 4, 2015, 7:18 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java, 
> > line 107
> > <https://reviews.apache.org/r/31754/diff/1/?file=885273#file885273line107>
> >
> > Do you think 'child module' is sufficiently descriptive?  It threw me 
> > off at first since parent/child already has a meaning in guice, so i 
> > thought there was some real black magic here.
> 
> Kevin Sweeney wrote:
> It is actually a child module in the guice sense (it's a module used to 
> seed the ServletContextListener's child injector). So there is some black 
> magic.

made it `getChildServletModule`.


- Kevin


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


On March 4, 2015, 5:55 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31754/
> ---
> 
> (Updated March 4, 2015, 5:55 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Break out API servlet configuration into its own module.
> 
> This is necessary to make a follow-up patch introducing HTTP Basic auth 
> testable.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 5f6a019e4d6401e1efd075b72c049fa245cc0d0a 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 8a59d89c07b406ce98076ca7ee51b958599a39ec 
>   src/main/java/org/apache/aurora/scheduler/http/SchedulerAPIServlet.java 
> 33ad43b3202e5e9ef5be919b6abc5cbc7f62b660 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
> d1ab9b18394ad37fe9dcb131816fcfb2952bf8b6 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
> 80beb258d9f2786668d29db85b1295163a402d42 
>   src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
> 47d54e3c3bb1ba5e0fb26379792f515f25316480 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
> 5019094333f9807c64a49c29569ade191ee61824 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiTServletTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31754/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 31754: Break out API servlet configuration into its own module.

[Kevin Sweeney]

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

(Updated March 5, 2015, 11:52 a.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

Addressed review feedback.


Repository: aurora


Description
---

Break out API servlet configuration into its own module.

This is necessary to make a follow-up patch introducing HTTP Basic auth 
testable.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
8a59d89c07b406ce98076ca7ee51b958599a39ec 
  src/main/java/org/apache/aurora/scheduler/http/SchedulerAPIServlet.java 
33ad43b3202e5e9ef5be919b6abc5cbc7f62b660 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
d1ab9b18394ad37fe9dcb131816fcfb2952bf8b6 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
9804bf5549cd2589fc68c57b3895efd89923169b 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
8ec5f9a3810b4deae981988487c6a46a20db72a2 
  src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
80beb258d9f2786668d29db85b1295163a402d42 
  src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
47d54e3c3bb1ba5e0fb26379792f515f25316480 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
5019094333f9807c64a49c29569ade191ee61824 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java PRE-CREATION 

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


Testing
---


Thanks,

Kevin Sweeney

Re: Review Request 31754: Break out API servlet configuration into its own module.

[Kevin Sweeney]

> On March 5, 2015, 10:40 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/http/SchedulerAPIServlet.java, 
> > line 25
> > <https://reviews.apache.org/r/31754/diff/1/?file=885270#file885270line25>
> >
> > Any reason not to call this `ApiThriftServlet` to be more explicit?

Done. Also noticed the subclass was unnecessary - changed to Provider.


> On March 5, 2015, 10:40 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java, line 57
> > <https://reviews.apache.org/r/31754/diff/1/?file=885272#file885272line57>
> >
> > one arg per line.

Fixed.


> On March 5, 2015, 10:40 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java, lines 66-81
> > <https://reviews.apache.org/r/31754/diff/1/?file=885268#file885268line66>
> >
> > Is this import move correct? We should treat com.twitter just like any 
> > other com.* import, right?

fixed IDE settings.


> On March 5, 2015, 10:40 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java, line 
> > 44
> > <https://reviews.apache.org/r/31754/diff/1/?file=885269#file885269line44>
> >
> > Same here re: import re-ordering.

same.


- Kevin


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


On March 4, 2015, 5:55 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31754/
> ---
> 
> (Updated March 4, 2015, 5:55 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Break out API servlet configuration into its own module.
> 
> This is necessary to make a follow-up patch introducing HTTP Basic auth 
> testable.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 5f6a019e4d6401e1efd075b72c049fa245cc0d0a 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 8a59d89c07b406ce98076ca7ee51b958599a39ec 
>   src/main/java/org/apache/aurora/scheduler/http/SchedulerAPIServlet.java 
> 33ad43b3202e5e9ef5be919b6abc5cbc7f62b660 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
> d1ab9b18394ad37fe9dcb131816fcfb2952bf8b6 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
> 80beb258d9f2786668d29db85b1295163a402d42 
>   src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
> 47d54e3c3bb1ba5e0fb26379792f515f25316480 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
> 5019094333f9807c64a49c29569ade191ee61824 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiTServletTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31754/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 31754: Break out API servlet configuration into its own module.

[Kevin Sweeney]

> On March 4, 2015, 7:18 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java, line 
> > 294
> > <https://reviews.apache.org/r/31754/diff/1/?file=885269#file885269line294>
> >
> > ISTR this being necessary for static asset serving.  Is this no longer 
> > necesary just because of two ServletModules?

No longer needed - we now pass an instance of DefaultServlet to serve() instead 
with different init-params. Changed this elsewhere as well - we only need to 
bind the class form of a servlet or filter if it has an Inject-constructor.


> On March 4, 2015, 7:18 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java, 
> > line 107
> > <https://reviews.apache.org/r/31754/diff/1/?file=885273#file885273line107>
> >
> > Do you think 'child module' is sufficiently descriptive?  It threw me 
> > off at first since parent/child already has a meaning in guice, so i 
> > thought there was some real black magic here.

It is actually a child module in the guice sense (it's a module used to seed 
the ServletContextListener's child injector). So there is some black magic.


> On March 4, 2015, 7:18 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/http/api/ApiTServletTest.java, 
> > lines 42-49
> > <https://reviews.apache.org/r/31754/diff/1/?file=885276#file885276line42>
> >
> > I assume this is a demonstration of how servlet tests should look going 
> > forward - where they provide the specific bindings needed?  If so, i like 
> > it!

Yes, individual servlets can provide just their bindings (plus mock 
dependencies). This is the major purpose of this diff.


- Kevin


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


On March 4, 2015, 5:55 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31754/
> ---
> 
> (Updated March 4, 2015, 5:55 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Break out API servlet configuration into its own module.
> 
> This is necessary to make a follow-up patch introducing HTTP Basic auth 
> testable.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 5f6a019e4d6401e1efd075b72c049fa245cc0d0a 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 8a59d89c07b406ce98076ca7ee51b958599a39ec 
>   src/main/java/org/apache/aurora/scheduler/http/SchedulerAPIServlet.java 
> 33ad43b3202e5e9ef5be919b6abc5cbc7f62b660 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
> d1ab9b18394ad37fe9dcb131816fcfb2952bf8b6 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
> 80beb258d9f2786668d29db85b1295163a402d42 
>   src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
> 47d54e3c3bb1ba5e0fb26379792f515f25316480 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
> 5019094333f9807c64a49c29569ade191ee61824 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiTServletTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31754/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 31754: Break out API servlet configuration into its own module.

[Kevin Sweeney]

> On March 4, 2015, 7:18 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java, line 141
> > <https://reviews.apache.org/r/31754/diff/1/?file=885268#file885268line141>
> >
> > There seems to be implementation detail leaking here.  Can you make 
> > JettyServerModule hide this away, and force the leak into the unit test 
> > instead?

Any thoughts on how that should look? Possibly an @VisibleForTesting 
constructor to JettyServerModule with a boolean production? The test needs to 
replace the binding for ServletContextListener, which needs a parent Injector 
handle, since binding a Module isn't allowed due to guice internals.


- Kevin


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


On March 4, 2015, 5:55 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31754/
> ---
> 
> (Updated March 4, 2015, 5:55 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Break out API servlet configuration into its own module.
> 
> This is necessary to make a follow-up patch introducing HTTP Basic auth 
> testable.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 5f6a019e4d6401e1efd075b72c049fa245cc0d0a 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 8a59d89c07b406ce98076ca7ee51b958599a39ec 
>   src/main/java/org/apache/aurora/scheduler/http/SchedulerAPIServlet.java 
> 33ad43b3202e5e9ef5be919b6abc5cbc7f62b660 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
> d1ab9b18394ad37fe9dcb131816fcfb2952bf8b6 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
> 80beb258d9f2786668d29db85b1295163a402d42 
>   src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
> 47d54e3c3bb1ba5e0fb26379792f515f25316480 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
> 5019094333f9807c64a49c29569ade191ee61824 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiTServletTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31754/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 31754: Break out API servlet configuration into its own module.

[Kevin Sweeney]

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


Looks like my IDE was a little overzealous applying a different styleguide - 
updated diff to follow.

- Kevin Sweeney


On March 4, 2015, 5:55 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31754/
> ---
> 
> (Updated March 4, 2015, 5:55 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Break out API servlet configuration into its own module.
> 
> This is necessary to make a follow-up patch introducing HTTP Basic auth 
> testable.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 5f6a019e4d6401e1efd075b72c049fa245cc0d0a 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 8a59d89c07b406ce98076ca7ee51b958599a39ec 
>   src/main/java/org/apache/aurora/scheduler/http/SchedulerAPIServlet.java 
> 33ad43b3202e5e9ef5be919b6abc5cbc7f62b660 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
> d1ab9b18394ad37fe9dcb131816fcfb2952bf8b6 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
> 80beb258d9f2786668d29db85b1295163a402d42 
>   src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
> 47d54e3c3bb1ba5e0fb26379792f515f25316480 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
> 5019094333f9807c64a49c29569ade191ee61824 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiTServletTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31754/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Review Request 31754: Break out API servlet configuration into its own module.

[Kevin Sweeney]

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

Review request for Aurora.


Repository: aurora


Description
---

Break out API servlet configuration into its own module.

This is necessary to make a follow-up patch introducing HTTP Basic auth 
testable.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
5f6a019e4d6401e1efd075b72c049fa245cc0d0a 
  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
8a59d89c07b406ce98076ca7ee51b958599a39ec 
  src/main/java/org/apache/aurora/scheduler/http/SchedulerAPIServlet.java 
33ad43b3202e5e9ef5be919b6abc5cbc7f62b660 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
d1ab9b18394ad37fe9dcb131816fcfb2952bf8b6 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
80beb258d9f2786668d29db85b1295163a402d42 
  src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
47d54e3c3bb1ba5e0fb26379792f515f25316480 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
5019094333f9807c64a49c29569ade191ee61824 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiTServletTest.java 
PRE-CREATION 

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


Testing
---


Thanks,

Kevin Sweeney

Re: Review Request 31350: Fix clusters.patch contextmanager cleanup

[Kevin Sweeney]

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


Stephan: I'm ready to commit this but it looks like you don't have an email 
address in reviewboard. Can you set one so that we can attribute this patch and 
ping this review?

```
  File 
"/Users/ksweeney/workspace/aurora/build-support/rbt.venv/lib/python2.7/site-packages/rbtools/clients/git.py",
 line 740, in create_commit
'--author="%s <%s>"' % (author.fullname, author.email)])
  File 
"/Users/ksweeney/workspace/aurora/build-support/rbt.venv/lib/python2.7/site-packages/rbtools/api/resource.py",
 line 299, in __getattr__
raise AttributeError
AttributeError
```

- Kevin Sweeney


On Feb. 24, 2015, 2:12 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31350/
> ---
> 
> (Updated Feb. 24, 2015, 2:12 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix clusters.patch contextmanager cleanup
> 
> Otherwise one failing testcase can affect the entire suite.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/common/clusters.py 
> c4b7fefca30313b281808478bf23158a9b7fbf85 
>   src/test/python/apache/aurora/common/test_clusters.py 
> 1bd696e9cd28d87d0cac68b33ab043407d796b61 
> 
> Diff: https://reviews.apache.org/r/31350/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/test/python/apache/aurora/common::
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 31659: Clean up end-to-end tests.

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On March 3, 2015, 12:55 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31659/
> ---
> 
> (Updated March 3, 2015, 12:55 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Major changes:
> - applying DRY by not invoking everything via `vagrant ssh` (making tests 
> easier to write and much faster to run)
> - pulled 'test cases' into individual functions
> - simplified (maybe?) setup for tests using SSH
> 
> This is being done in preparation for AURORA-1157.
> 
> 
> Diffs
> -
> 
>   src/test/sh/org/apache/aurora/e2e/http/http_example.aurora 
> 0f6423521ad2c85032b4825a5b9793dc522a3b70 
>   src/test/sh/org/apache/aurora/e2e/http/http_example_docker.aurora 
> 00fa2fb6c272fe01700c4696f92713f11d62230b 
>   src/test/sh/org/apache/aurora/e2e/http/http_example_docker_updated.aurora 
> d987d637da4061cd7946074243daabee4b9a150f 
>   src/test/sh/org/apache/aurora/e2e/http/http_example_updated.aurora 
> 072bbb76707e6696815509f21d50b6b5446241b3 
>   src/test/sh/org/apache/aurora/e2e/test_common.sh 
> b621975bab6f8bbf193e61360654e34337e36943 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> 1d599c3f71b7e07c1cb8bee01a4d3b5cfcf92631 
>   src/test/sh/org/apache/aurora/e2e/test_run.sh 
> ab91b95a60f7b459b95dc7e13a29692badca5ff7 
> 
> Diff: https://reviews.apache.org/r/31659/diff/
> 
> 
> Testing
> ---
> 
> Ran the script.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31525: Improving NearestFit reporting accuracy.

[Kevin Sweeney]

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



src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java
<https://reviews.apache.org/r/31525/#comment121550>

I don't think the Math.pow approach is necessary - why not make score an 
EnumMap (or just an EnumSet). A veto increments 
the counter in the bucket for its type (or sets the bit for it). Still very 
efficient and more type-safe.


- Kevin Sweeney


On Feb. 27, 2015, 1:05 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31525/
> ---
> 
> (Updated Feb. 27, 2015, 1:05 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-1148
> https://issues.apache.org/jira/browse/AURORA-1148
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Instead of relying on a fixed MAX_SCORE, vetoes are now graded by their 
> relative weight (severity) with a dedicated constraint mismatch ranked higest 
> and an insufficient resource mismatch - lowest. Together with "break early" 
> rule in SchedulingFilter, this ensures "heavier" vetoes are given proper 
> attention in the NearestFit. This simplifies `NearestFit` logic while also 
> improving pending reason reporting accuracy and scheduling performance.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
> 3313bd2f9ed5a88375d88715dea5da1e9ad8b963 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> a020ce50d578fba7f32b370f448a49eb1c284147 
>   src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java 
> c3097e49c0f6588ea765aa4fab69dd35e3d90e8b 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  b00668423a53a8cf6f4da3456bce3354f1fd2424 
>   src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 
> 78a236c0f9074692b67ce18e6e03f18fe4529e02 
> 
> Diff: https://reviews.apache.org/r/31525/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 31334: Fixing cron update quota checking.

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On Feb. 27, 2015, 9:18 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31334/
> ---
> 
> (Updated Feb. 27, 2015, 9:18 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-1134
> https://issues.apache.org/jira/browse/AURORA-1134
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Cron schedule updates were treated as template addition rather than 
> differential update, requiring extra quota capacity to clear the check.
> 
> Adding a new `checkCronUpdate()` QuotaManager interface method to correctly 
> handle cron job updates.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 8444d225e91c75e9a571049d28af7264b6d2d017 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  56398f33206cfa1fa45601a21760c7e23e2616bf 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> b79b07c23e00fab524aef21481070c5f09c9fa18 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d36418807ace4f61e8709a2827cea0e7bb8ac6b7 
> 
> Diff: https://reviews.apache.org/r/31334/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 31525: Improving NearestFit reporting accuracy.

[Kevin Sweeney]

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



src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java
<https://reviews.apache.org/r/31525/#comment121148>

How about making score an EnumSet so that you can do a typesafe 
bitwise OR?


- Kevin Sweeney


On Feb. 26, 2015, 7:19 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31525/
> ---
> 
> (Updated Feb. 26, 2015, 7:19 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-1148
> https://issues.apache.org/jira/browse/AURORA-1148
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Instead of relying on a fixed MAX_SCORE, vetoes are now graded by their 
> relative weight (severity) with a dedicated constraint mismatch ranked higest 
> and an insufficient resource mismatch - lowest. Together with "break early" 
> rule in SchedulingFilter, this ensures "heavier" vetoes are given proper 
> attention in the NearestFit. This simplifies `NearestFit` logic while also 
> improving pending reason reporting accuracy and scheduling performance.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
> 3313bd2f9ed5a88375d88715dea5da1e9ad8b963 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> a020ce50d578fba7f32b370f448a49eb1c284147 
>   src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java 
> c3097e49c0f6588ea765aa4fab69dd35e3d90e8b 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  b00668423a53a8cf6f4da3456bce3354f1fd2424 
>   src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 
> 78a236c0f9074692b67ce18e6e03f18fe4529e02 
> 
> Diff: https://reviews.apache.org/r/31525/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 31445: Expose more details about the tasks the preemptor is working for.

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On Feb. 25, 2015, 2:45 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31445/
> ---
> 
> (Updated Feb. 25, 2015, 2:45 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Zameer Manji.
> 
> 
> Bugs: AURORA-524
> https://issues.apache.org/jira/browse/AURORA-524
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Expose more details about the tasks the preemptor is working for.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
> 42af883721e4e5c0ae23ff65a5fb7dc285e48faa 
>   
> src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
>  44cd8f79493f0f247cd876ef78b30b4f813314c4 
>   src/test/java/org/apache/aurora/scheduler/testing/FakeStatsProvider.java 
> 768e78424268513742ecea22b2f5395dcb46da8c 
> 
> Diff: https://reviews.apache.org/r/31445/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31456: Only fetch host attributes once per preemption round.

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On Feb. 25, 2015, 6 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31456/
> ---
> 
> (Updated Feb. 25, 2015, 6 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1153
> https://issues.apache.org/jira/browse/AURORA-1153
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A small, but cheap performance improvement when the preemptor is engaged.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
> 42af883721e4e5c0ae23ff65a5fb7dc285e48faa 
> 
> Diff: https://reviews.apache.org/r/31456/diff/
> 
> 
> Testing
> ---
> 
> Before this diff:
> 
> ```
> o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
>avgt  100   5788175.295 ±  47410.511  ns/op
> o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
>  avgt  100   5046038.818 ±  38636.961  ns/op
> o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark
> avgt  100  64323073.493 ± 644053.437  ns/op
> ```
> 
> After this diff:
> ```
> o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
>avgt  100   5783859.480 ±  37291.662  ns/op
> o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
>  avgt  100   5259022.679 ±  30276.902  ns/op
> o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark
> avgt  100  58171450.071 ± 638305.308  ns/op
> ```
> 
> So, about 10% for ~free.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31421: Handle TASK_ERROR and TASK_STAGING states.

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On Feb. 25, 2015, 10:48 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31421/
> ---
> 
> (Updated Feb. 25, 2015, 10:48 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1001
> https://issues.apache.org/jira/browse/AURORA-1001
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is only a stop-gap to make sure we are compatible with mesos 0.22.  See 
> ticket for more detail.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/Conversions.java 
> 7b7650d74e32d6396ab75a413380666d5000eac3 
>   src/test/java/org/apache/aurora/scheduler/base/ConversionsTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31421/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31394: Renaming storage.thrift job store structs.

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On Feb. 24, 2015, 5:06 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31394/
> ---
> 
> (Updated Feb. 24, 2015, 5:06 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Renaming thrift structs and the `JobStore` to better reflect its cron-only 
> purpose.
> 
> Purely IDE-driven renaming.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> d3b50d7511f385797cbb539e66e83a851c6bccf7 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java 
> c8834215c6c621246e3436de734b95d9ced66537 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java 
> 916cb6dc4b8907ffaf6736f1d615bbc72611a589 
>   src/main/java/org/apache/aurora/scheduler/http/StructDump.java 
> e7ee0cf0895c1c75c98c993e14716c7419a9eeec 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 199dc1123cd1966b1c8075e7a486efdf627c45f3 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> fc8e2f4e75a33789b28588204cbbfd56b5d049a4 
>   src/main/java/org/apache/aurora/scheduler/storage/JobStore.java 
> 748b2cbce76258b3f4c454538b8d40a19e13e68e 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 41b7053b01f2105bbf9d5ee4616a0c3d0d91220a 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 7a99afa656a0978d05848a31c73c2658a98d6918 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> ea4afb49e52aed1134cf6ca44bf53693a4d999df 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 0229b920ebb36353b8bf9d1ea360d7ff6f4dbf97 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 72b9895214aa372c4fd75e5bb6dee8485b3594f6 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/StreamManagerImpl.java 
> ee88d7c03cc1d1f23ed6b2ef1e5cf5c7929a57c9 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> aa2226651ba6c5d1b77c4861f734b4194dcbecf3 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java 
> c9ffc3871e1824ea39961c4a4d416e50d7e1f078 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemStorage.java 
> bc3a7c5061d97e13c7702329a168d416c3dd1bcc 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemStorageModule.java 
> 31c25d04cd1b8f741649edea8424eb298738d895 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  059626f742ce806fded754ef04a83e2eb5405ff2 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java 
> 7bef09e8217750908eb56d9010df79bb5fd050de 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
> c3f8d1b4c96ae35ebfb157f46ae545c8d39c3602 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImplTest.java
>  d4c14afa7b7ea904f7a33a8747591ab8f1be5f9b 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> b48994f40edea5e257e2816b1950d8a495d89aa6 
>   src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 
> 91789e7c203753054406296c41248a1fbf30d019 
>   
> src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java
>  0c73a9f78d3dfe4764c0e8513b85611313067368 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 31b75f41ef3cf1d32824afabec6f7ea79285973d 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
>  343106c24395911162657538c99a05b81d5ee2dc 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java
>  36809ac2e7e8924187eb2d9cbd69da58e7866681 
>   src/test/java/org/apache/aurora/scheduler/storage/mem/MemJobStoreTest.java 
> b9ade45bae6f76520789821733a58075ea6d0bd0 
>   
> src/test/java/org/apache/aurora/scheduler/storage/testing/StorageTestUtil.java
>  6feddc3fc82da072d84bd69b16637144215415b7 
> 
> Diff: https://reviews.apache.org/r/31394/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> Also, test bidirectional upgrade in vagrant.
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 31350: Fix clusters.patch contextmanager cleanup

[Kevin Sweeney]

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

Ship it!



src/test/python/apache/aurora/common/test_clusters.py
<https://reviews.apache.org/r/31350/#comment120356>

I'm not sure this test case is valuable since it will run in a 
non-deterministic order.


- Kevin Sweeney


On Feb. 24, 2015, 2:12 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31350/
> ---
> 
> (Updated Feb. 24, 2015, 2:12 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix clusters.patch contextmanager cleanup
> 
> Otherwise one failing testcase can affect the entire suite.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/common/clusters.py 
> c4b7fefca30313b281808478bf23158a9b7fbf85 
>   src/test/python/apache/aurora/common/test_clusters.py 
> 1bd696e9cd28d87d0cac68b33ab043407d796b61 
> 
> Diff: https://reviews.apache.org/r/31350/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/test/python/apache/aurora/common::
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 31350: Fix clusters.patch contextmanager cleanup

[Kevin Sweeney]

> On Feb. 24, 2015, 11:12 a.m., Kevin Sweeney wrote:
> > This is awesome! If this fixes the full suite, can you also remove 
> > `--no-fast` from `build-support/jenkins/build.sh`?
> 
> Stephan Erb wrote:
> I doubt that this patch is sufficient to warrant the change of the build 
> script. There may be many more error conditions requiring a `no-fast`.
> 
> Background info: We are directly using the python client in our own 
> backend and are also writing some integration tests against it. In some of 
> these testcases we are using CLUSTERS.patch and recently discovered its 
> faulty cleanup behaviour.

That's unfortunate. Filed https://issues.apache.org/jira/browse/AURORA-1145 to 
follow-up.


- Kevin


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


On Feb. 24, 2015, 2:12 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31350/
> ---
> 
> (Updated Feb. 24, 2015, 2:12 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix clusters.patch contextmanager cleanup
> 
> Otherwise one failing testcase can affect the entire suite.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/common/clusters.py 
> c4b7fefca30313b281808478bf23158a9b7fbf85 
>   src/test/python/apache/aurora/common/test_clusters.py 
> 1bd696e9cd28d87d0cac68b33ab043407d796b61 
> 
> Diff: https://reviews.apache.org/r/31350/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/test/python/apache/aurora/common::
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.

[Kevin Sweeney]

> On Feb. 24, 2015, 11:35 a.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/async/OfferManager.java, line 317
> > <https://reviews.apache.org/r/30891/diff/7/?file=873086#file873086line317>
> >
> > Same as above - no need to hold the intrinsic lock while logging here.
> 
> Maxim Khutornenko wrote:
> Same here.

my shipit stands but you can use the same lock with a

```java
synchronized (this) {
. // logic
}
```
construct


- Kevin


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


On Feb. 24, 2015, 1:30 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30891/
> ---
> 
> (Updated Feb. 24, 2015, 1:30 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-909
> https://issues.apache.org/jira/browse/AURORA-909
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Offer filtering for static vetoes. Part 3 of 4: Filtering out statically 
> banned offers.
> 
> Will not apply cleanly: diffed with https://reviews.apache.org/r/30890 as a 
> parent.
> 
> Original RB: https://reviews.apache.org/r/28617/
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/OfferManager.java 
> b241d7b22c3d1ceca127b2671eb608ae41283bf3 
>   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
> 21ea7d2b9d2f8c76367d7ae985270402bb51ea26 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
> 5a0f7ddb7e8fa6869cbb0fdfd07c6881780c6917 
>   src/test/java/org/apache/aurora/scheduler/async/OfferManagerImplTest.java 
> 7ee2bb9bec6c59ba67b65d5b1229c64aca1277ff 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
> 9eef52a333e09454e8fd0026371c7e64472a883d 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
> b6d4d8e771c7d16a46e43c7d5c427b911f8b661d 
> 
> Diff: https://reviews.apache.org/r/30891/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 30895: Offer filtering for static vetoes. Part 4 of 4: Benchmarks.

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On Feb. 23, 2015, 2:47 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30895/
> ---
> 
> (Updated Feb. 23, 2015, 2:47 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to 
> support preemption toggling.
> 
> Original RB: https://reviews.apache.org/r/28617/
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java 
> PRE-CREATION 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> bacfbfeb237ecddf82f58679e05be012c5214e61 
>   src/jmh/java/org/apache/aurora/benchmark/Tasks.java 
> 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25 
> 
> Diff: https://reviews.apache.org/r/30895/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 31235: Refactoring CronJobManager interface.

[Kevin Sweeney]

> On Feb. 24, 2015, 11:27 a.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java, 
> > line 63
> > <https://reviews.apache.org/r/31235/diff/2/?file=873240#file873240line63>
> >
> > This TODO wasn't addressed - put it back?
> 
> Maxim Khutornenko wrote:
> The way I read this TODO is "don't fetch job configs, fetch job keys 
> instead". Is that correct? If so, I don't think we will ever have a store 
> interface to pull job keys. Since we store JobConfiugrations, pulling job 
> keys will also require pulling JobConfigurations, so filtering job keys seems 
> like a consumer thing to do.
> 
> Also, the code below appears to also require cron schedule rather than 
> just a job key:
> ```
> SanitizedCronJob cronJob = SanitizedCronJob.fromUnsanitized(job);
> cronJobManager.scheduleJob(
> cronJob.getCrontabEntry(),
> cronJob.getSanitizedConfig().getJobConfig().getKey());
> ```

Good point, it looks like we need job keys and cron schedules here.


- Kevin


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


On Feb. 23, 2015, 5:23 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31235/
> -------
> 
> (Updated Feb. 23, 2015, 5:23 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Removing job store "proxy" methods from CronJobManager and dropping job 
> manager concept from the storage. The job manager is long gone but we still 
> carry around the job manager ID.
> 
> Despite the diff's size the changes are mostly mechanical: replacing 
> CronJobManager occurrences with direct job store access.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 31b698131aeb87ea0a633b0cee33b49ea1d501b4 
>   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
> b8830d187de27533307a8a2e9be6385f5d3e2289 
>   src/main/java/org/apache/aurora/scheduler/cron/CronJobManager.java 
> fc6e4432d64e625a583d8c8a130d99e066fd232c 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java 
> efea7ad9f6efc99b600d071c3c20063b6bc4b211 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java 
> c5cb8cae43c86ae2378a0bef7688d400aa188e57 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java 
> 64d9486fade3b03b8db936fe60790ea0858212a9 
>   src/main/java/org/apache/aurora/scheduler/http/StructDump.java 
> 063170ac869dafc161c73f735b33f4cbe8e03ab6 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> dab8c0535bf9b36cdf7ca785c34d315f0bb4204c 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 4d83c58c8f0527813ef26477f08424104e0c29d9 
>   src/main/java/org/apache/aurora/scheduler/storage/JobStore.java 
> ad0d67a27628f46dedda2ae4e0e61025dff1e1fd 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 83a59ce5d5b2754b1d31354280eca31922d73cdd 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 52377bca8060720dc4bec884c911182c3e77bc52 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 384e9b536c3ee3edf7d90960aa51ef98948af088 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 27981393d700c84f6aaa791f12b24d0cec28b5df 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 262d72c781759bac58bfa6c0fcec66c513d8b79b 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java 
> c24a32e57b34a1fc41681fda9bdb4de38ed8896b 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> ee7618979ce94631af8aaf7ab3ecb2fbfb33fc38 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  3e491d999dc3f9e116d3502e4c08c04bd6c6bab9 
>   src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java 
> 97ecb742d6e0418890f875394ded8d9fdae2b1c2 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java 
> c99135ab9c55a42ab51f18cc5ea127b498f0721f 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
> 915d7c8294b3d8262021da

Re: Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.

[Kevin Sweeney]

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

Ship it!



src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java
<https://reviews.apache.org/r/30891/#comment118503>

as written, a caller needs to know FAILURE and FAILURE_STATIC_MISMATCH both 
mean failure, but it would be easy to just check for `== FAILURE` and miss that 
in code reviews. Would it make sense to add `isFailure()` and 
`isStaticMismatch()` here?

Also, consider fetching the VetoGroup in the constructor so that 
precondition checks will fail faster



src/main/java/org/apache/aurora/scheduler/async/OfferManager.java
<https://reviews.apache.org/r/30891/#comment120285>

No need to hold the intrinsic lock while logging here



src/main/java/org/apache/aurora/scheduler/async/OfferManager.java
<https://reviews.apache.org/r/30891/#comment120286>

Same as above - no need to hold the intrinsic lock while logging here.



src/test/java/org/apache/aurora/scheduler/async/OfferManagerImplTest.java
<https://reviews.apache.org/r/30891/#comment120290>

It would be good form to reset the level in the tearDown here.


- Kevin Sweeney


On Feb. 23, 2015, 12:36 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30891/
> ---
> 
> (Updated Feb. 23, 2015, 12:36 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-909
> https://issues.apache.org/jira/browse/AURORA-909
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Offer filtering for static vetoes. Part 3 of 4: Filtering out statically 
> banned offers.
> 
> Will not apply cleanly: diffed with https://reviews.apache.org/r/30890 as a 
> parent.
> 
> Original RB: https://reviews.apache.org/r/28617/
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/OfferManager.java 
> b241d7b22c3d1ceca127b2671eb608ae41283bf3 
>   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
> 21ea7d2b9d2f8c76367d7ae985270402bb51ea26 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
> 5a0f7ddb7e8fa6869cbb0fdfd07c6881780c6917 
>   src/test/java/org/apache/aurora/scheduler/async/OfferManagerImplTest.java 
> 7ee2bb9bec6c59ba67b65d5b1229c64aca1277ff 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
> 9eef52a333e09454e8fd0026371c7e64472a883d 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
> b6d4d8e771c7d16a46e43c7d5c427b911f8b661d 
> 
> Diff: https://reviews.apache.org/r/30891/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 29943: Uptime-driven scheduler job updates

[Kevin Sweeney]

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


Is this ready for review now?

- Kevin Sweeney


On Jan. 20, 2015, 1:12 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29943/
> ---
> 
> (Updated Jan. 20, 2015, 1:12 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman.
> 
> 
> Bugs: AURORA-1041
> https://issues.apache.org/jira/browse/AURORA-1041
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is the first take on implementing job uptime driven updates. In addition 
> to the olde good "batch_size", instances can now be dispatched in arbitrary 
> sequence depending on the overall uptime (health) of the job. 
> 
> The uptime is specified by a tuple of **waitForUptimeMs** and 
> **waitForUptimePercentInstances** values. An excerpt from api.thrift 
> explaining the feature:
> ```
> /**
>* The uptime-driven update throttles the number of instances being updated 
> at any given moment
>* according to the job uptime calculations. The "X% of instances up over Y 
> interval" invariant
>* is preserved over the entire job update lifetime. No new instances are 
> dispatched for update
>* unless that invariant is satisfied. Instances are dispatched in their 
> natural uptime order,
>* shortest uptime first.
>*
>* For example, when set as below the update will block until at least 90% 
> of job instances are in
>* RUNNING state for at least 1 minute:
>*waitForUptimeMs = 6
>*waitForUptimePercentInstances = 90
>*
>* When using uptime-driven update, it's expected that updateGroupSize is 
> left unset to allow job
>* uptime settings drive the update progress. However, if updateGroupSize 
> is set it will be
>* pre-applied before SLA uptime calculations to determine the update 
> working set. As a side
>* effect, the updateGroupSize results in a natural ordering of instances 
> taken for each group
>* (instances within a group are still updated in a "shortest uptime first" 
> order).
>*
>* For example, if set as below the number of instances being updated at 
> any given moment will
>* never exceed 5 even though the uptime calculations may allow more than 5:
>*updateGroupSize = 5
>*waitForUptimeMs = 6
>*waitForUptimePercentInstances = 90
>*
>* NOTE on update rollback: with the uptime-driven update, there is no 
> reliable way to ensure a
>* graceful throttled rollback as unstable/flapping instances may never 
> yield an acceptable uptime
>* to perform an uptime-coordinated rollback. As such, when 
> rollbackOnFailure=True AND the
>* updateGroupSize=0 the updater will dispatch all affected instances at 
> once.
>* Use rollbackOnFailure=True with caution for uptime-driven updates.
>*/
> ```
> 
> For reviewers: recommend starting with api.thrift and then proceeding to the 
> InstanceUptimeStrategy.java that implements the core algo.
> 
> TODO: 
> - vagrant e2e test
> - more corner case unit test coverage in JobUpdaterIT
> - client warning message in case uptime specs are used with client updater
> - docs
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 08ba1cdf88b712de22c26c04443079282db59ef9 
>   src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java 
> eae79d59b445ea58f46dc9e3107c03fbd83b6a95 
>   src/main/java/org/apache/aurora/scheduler/sla/SlaUtil.java 
> 156b9c0a2fa0c0ec4b7220d5ec2cc40c3e59d1d6 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  ac92959f34a3b0962d6aa018dc82a5ac72ea1b34 
>   
> src/main/java/org/apache/aurora/scheduler/updater/InstanceUptimeProviderImpl.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  a992938d4e12b20f81608be6bbdc24c0a211c3fd 
>   src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 
> 27a5b9026f5ac3b3bdeb32813b10435bc3dab173 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
> b53086169aa53d27a39a01cadf8d3c4a8ecb68de 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdaterModule.java 
> 5733da3daeacd8cb726310e5d9933635e3993687 
>   
> src/main/jav

Re: Review Request 31235: Refactoring CronJobManager interface.

[Kevin Sweeney]

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

Ship it!



src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java
<https://reviews.apache.org/r/31235/#comment120097>

rename to cronJobStore?



src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java
<https://reviews.apache.org/r/31235/#comment120098>

rename to getCronJobStore?



src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java
<https://reviews.apache.org/r/31235/#comment120099>

This TODO wasn't addressed - put it back?


- Kevin Sweeney


On Feb. 23, 2015, 5:23 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31235/
> ---
> 
> (Updated Feb. 23, 2015, 5:23 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Removing job store "proxy" methods from CronJobManager and dropping job 
> manager concept from the storage. The job manager is long gone but we still 
> carry around the job manager ID.
> 
> Despite the diff's size the changes are mostly mechanical: replacing 
> CronJobManager occurrences with direct job store access.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 31b698131aeb87ea0a633b0cee33b49ea1d501b4 
>   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
> b8830d187de27533307a8a2e9be6385f5d3e2289 
>   src/main/java/org/apache/aurora/scheduler/cron/CronJobManager.java 
> fc6e4432d64e625a583d8c8a130d99e066fd232c 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java 
> efea7ad9f6efc99b600d071c3c20063b6bc4b211 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java 
> c5cb8cae43c86ae2378a0bef7688d400aa188e57 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java 
> 64d9486fade3b03b8db936fe60790ea0858212a9 
>   src/main/java/org/apache/aurora/scheduler/http/StructDump.java 
> 063170ac869dafc161c73f735b33f4cbe8e03ab6 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> dab8c0535bf9b36cdf7ca785c34d315f0bb4204c 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 4d83c58c8f0527813ef26477f08424104e0c29d9 
>   src/main/java/org/apache/aurora/scheduler/storage/JobStore.java 
> ad0d67a27628f46dedda2ae4e0e61025dff1e1fd 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 83a59ce5d5b2754b1d31354280eca31922d73cdd 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 52377bca8060720dc4bec884c911182c3e77bc52 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 384e9b536c3ee3edf7d90960aa51ef98948af088 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 27981393d700c84f6aaa791f12b24d0cec28b5df 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 262d72c781759bac58bfa6c0fcec66c513d8b79b 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java 
> c24a32e57b34a1fc41681fda9bdb4de38ed8896b 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> ee7618979ce94631af8aaf7ab3ecb2fbfb33fc38 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  3e491d999dc3f9e116d3502e4c08c04bd6c6bab9 
>   src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java 
> 97ecb742d6e0418890f875394ded8d9fdae2b1c2 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java 
> c99135ab9c55a42ab51f18cc5ea127b498f0721f 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
> 915d7c8294b3d8262021da1c30324f55d8413ff9 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImplTest.java
>  701536fff8948ef233523e114e45043992175891 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> b0772f73f1a21da5828660bfd7d2b1f6b15cbf74 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> 493150b062eddf2581a048a9e13826205b8f2c15 
>   
> src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java
>  479c8bded75c61c36ac70cf1d8f945b96850087e 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7e5d0edefbed3f6711636

Re: Review Request 31350: Fix clusters.patch contextmanager cleanup

[Kevin Sweeney]

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


This is awesome! If this fixes the full suite, can you also remove `--no-fast` 
from `build-support/jenkins/build.sh`?

- Kevin Sweeney


On Feb. 24, 2015, 6:18 a.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31350/
> ---
> 
> (Updated Feb. 24, 2015, 6:18 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix clusters.patch contextmanager cleanup
> 
> Otherwise one failing testcase can affect the entire suite.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/common/clusters.py 
> c4b7fefca30313b281808478bf23158a9b7fbf85 
>   src/test/python/apache/aurora/common/test_clusters.py 
> 1bd696e9cd28d87d0cac68b33ab043407d796b61 
> 
> Diff: https://reviews.apache.org/r/31350/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/test/python/apache/aurora/common::
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 31176: Split out ReadOnlySchedulerImplTest.

[Kevin Sweeney]

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

(Updated Feb. 23, 2015, 2:49 p.m.)


Review request for Aurora and Bill Farner.


Changes
---

Bill's feedback.


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


Repository: aurora


Description
---

Split out ReadOnlySchedulerImplTest.

* Factored out a common Fixtures class for Thrift test data (not married to the 
name).
* Added a unit test for LoggingInterceptor (preferable to relying on 
SchedulerThriftInterfaceTest to test it indirectly).


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
6484642b6416466807f76267d66941c9cf7b3346 
  src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 15147757dfca889f5b5b5ec6967dda98ddfe3075 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptorTest.java
 PRE-CREATION 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney

Re: Review Request 31176: Split out ReadOnlySchedulerImplTest.

[Kevin Sweeney]

> On Feb. 23, 2015, 2:44 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java, line 73
> > <https://reviews.apache.org/r/31176/diff/2/?file=873063#file873063line73>
> >
> > how about NO_LOCK?

How would you feel about null? This was a tool-based refactor, but I don't see 
the value in holding a null reference here.


- Kevin


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


On Feb. 23, 2015, 2:41 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31176/
> ---
> 
> (Updated Feb. 23, 2015, 2:41 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-808
> https://issues.apache.org/jira/browse/AURORA-808
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Split out ReadOnlySchedulerImplTest.
> 
> * Factored out a common Fixtures class for Thrift test data (not married to 
> the name).
> * Added a unit test for LoggingInterceptor (preferable to relying on 
> SchedulerThriftInterfaceTest to test it indirectly).
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
> 6484642b6416466807f76267d66941c9cf7b3346 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  15147757dfca889f5b5b5ec6967dda98ddfe3075 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptorTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31176/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 31176: Split out ReadOnlySchedulerImplTest.

[Kevin Sweeney]

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

(Updated Feb. 23, 2015, 2:41 p.m.)


Review request for Aurora and Bill Farner.


Changes
---

+bug


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


Repository: aurora


Description
---

Split out ReadOnlySchedulerImplTest.

* Factored out a common Fixtures class for Thrift test data (not married to the 
name).
* Added a unit test for LoggingInterceptor (preferable to relying on 
SchedulerThriftInterfaceTest to test it indirectly).


Diffs
-

  src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
6484642b6416466807f76267d66941c9cf7b3346 
  src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 15147757dfca889f5b5b5ec6967dda98ddfe3075 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptorTest.java
 PRE-CREATION 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney

Re: Review Request 31176: Split out ReadOnlySchedulerImplTest.

[Kevin Sweeney]

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

(Updated Feb. 23, 2015, 2:39 p.m.)


Review request for Aurora and Bill Farner.


Changes
---

static import for Fixtures


Repository: aurora


Description
---

Split out ReadOnlySchedulerImplTest.

* Factored out a common Fixtures class for Thrift test data (not married to the 
name).
* Added a unit test for LoggingInterceptor (preferable to relying on 
SchedulerThriftInterfaceTest to test it indirectly).


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
6484642b6416466807f76267d66941c9cf7b3346 
  src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 15147757dfca889f5b5b5ec6967dda98ddfe3075 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptorTest.java
 PRE-CREATION 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney

Re: Review Request 30895: Offer filtering for static vetoes. Part 4 of 4: Benchmarks.

[Kevin Sweeney]

> On Feb. 23, 2015, 11:47 a.m., Bill Farner wrote:
> > src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java, line 33
> > <https://reviews.apache.org/r/30895/diff/2/?file=873059#file873059line33>
> >
> > This is bound to become a Texas constructor [1].
> > 
> > Please consider using the builder pattern instead to improve 
> > readability at the call sites.
> > 
> > Brief docs on these would be helpful too.  Specifically, 
> > `clusterUtilization` would benefit from a blurb.
> > 
> > [1] 
> > https://github.com/twitter/commons/blob/master/src/java/com/twitter/common/styleguide.md#stay-out-of-texas
> 
> Maxim Khutornenko wrote:
> I can't force myself into justifying Builder here quite yet but I am fine 
> with improving readabilty at call sites. Done.

FWIW there's an option in IntelliJ to do this: 
https://www.jetbrains.com/idea/help/replace-constructor-with-builder.html


- Kevin


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


On Feb. 23, 2015, 1:03 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30895/
> -----------
> 
> (Updated Feb. 23, 2015, 1:03 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to 
> support preemption toggling.
> 
> Original RB: https://reviews.apache.org/r/28617/
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java 
> PRE-CREATION 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> bacfbfeb237ecddf82f58679e05be012c5214e61 
>   src/jmh/java/org/apache/aurora/benchmark/Tasks.java 
> 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25 
> 
> Diff: https://reviews.apache.org/r/30895/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 31176: Split out ReadOnlySchedulerImplTest.

[Kevin Sweeney]

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

(Updated Feb. 23, 2015, 11:45 a.m.)


Review request for Aurora.


Changes
---

Fix checkstyle errors and rebase.


Repository: aurora


Description (updated)
---

Split out ReadOnlySchedulerImplTest.

* Factored out a common Fixtures class for Thrift test data (not married to the 
name).
* Added a unit test for LoggingInterceptor (preferable to relying on 
SchedulerThriftInterfaceTest to test it indirectly).


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
6484642b6416466807f76267d66941c9cf7b3346 
  src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 15147757dfca889f5b5b5ec6967dda98ddfe3075 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptorTest.java
 PRE-CREATION 

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


Testing (updated)
---

./gradlew -Pq build


Thanks,

Kevin Sweeney

Re: Review Request 31042: Added document describing how to build Aurora for CentOS

[Kevin Sweeney]

> On Feb. 18, 2015, 11:42 a.m., Kevin Sweeney wrote:
> > Hi Craig, thanks for the contribution. I'm wondering if this document might 
> > be better suited as a gist or blog post given that it's dated (0.7.1) and 
> > we don't have automated test coverage to ensure it doesn't diverge from the 
> > rest of the source tree. Alternatively if you'd like to provide a spec file 
> > and test coverage (e.g. docker run within our CI build) I think this would 
> > be much more palatable to commit. Also, while I personally love Digital 
> > Ocean I'm not sure what the rules on endorsing a specific vendor in 
> > official project documentation are.
> 
> Craig Wickesser wrote:
> I could post it as a blog post, where would a link to a blog post best go 
> in the docs?
> 
> Bill Farner wrote:
> I share Kevin's concern over bit rot here, simply because we have nothing 
> to exercise these.  However, i think Craig points out a big gap in figuring 
> out _what_ components are needed for a healthy Aurora cluster.  More 
> specifically, `README.md` has a link titled 'Deploying Aurora' that is 
> actually a guide for deploying the scheduler, and there is no equivalent for 
> other components.  (I've observed three people in IRC this week alone falling 
> into this trap.)
> 
> Craig - how would you feel about pulling relevant content from this diff 
> into deploying-aurora-scheduler.md [1] (renaming to deploying-aurora.md)?
> Kevin - does that sound reasonable?
> 
> 
> [1] 
> https://github.com/apache/incubator-aurora/blob/master/docs/deploying-aurora-scheduler.md
> 
> Craig Wickesser wrote:
> Bill, I think updating the existing "deploying" doc makes sense. If Kevin 
> agrees, I'll take that route.

+1


- Kevin


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


On Feb. 16, 2015, 11:21 a.m., Craig Wickesser wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31042/
> ---
> 
> (Updated Feb. 16, 2015, 11:21 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Added document describing how to build Aurora for CentOS
> 
> 
> Diffs
> -
> 
>   docs/build-for-centos.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31042/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Craig Wickesser
> 
>

Review Request 31176: Split out ReadOnlySchedulerImplTest.

[Kevin Sweeney]

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

Review request for Aurora.


Repository: aurora


Description
---

Split out ReadOnlySchedulerImplTest.

Posting for early bot feedback.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
6484642b6416466807f76267d66941c9cf7b3346 
  src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 06c8faa9de4d0ac8389dbf07d4e81934b503761b 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptorTest.java
 PRE-CREATION 

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


Testing
---

./gradlew build


Thanks,

Kevin Sweeney

Re: Review Request 31171: Saving backups asynchronously.

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On Feb. 18, 2015, 4:27 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31171/
> ---
> 
> (Updated Feb. 18, 2015, 4:27 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-1108
> https://issues.apache.org/jira/browse/AURORA-1108
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Wrapped backup file handling into Runnable to handle asynchronously. 
> 
> Refactoring somehow triggered a findbugs warning that I had to address as 
> well:
> "Exceptional return value of java.io.File.delete() ignored in 
> org.apache.aurora.scheduler.storage.backup.StorageBackup$StorageBackupImpl$1.run()"
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java 
> 71feb5779df5738a92e587f9f66f915961f52d1d 
>   src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java 
> a20378a01575c399c23c86aa784424fc6909c34e 
>   src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 
> 7602d112d29454608a97ec81de14b6bf0c45df68 
>   
> src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java
>  15fc4404fa2ace4391e4ddc7153c848bc91d43df 
> 
> Diff: https://reviews.apache.org/r/31171/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 31042: Added document describing how to build Aurora for CentOS

[Kevin Sweeney]

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


Hi Craig, thanks for the contribution. I'm wondering if this document might be 
better suited as a gist or blog post given that it's dated (0.7.1) and we don't 
have automated test coverage to ensure it doesn't diverge from the rest of the 
source tree. Alternatively if you'd like to provide a spec file and test 
coverage (e.g. docker run within our CI build) I think this would be much more 
palatable to commit. Also, while I personally love Digital Ocean I'm not sure 
what the rules on endorsing a specific vendor in official project documentation 
are.

- Kevin Sweeney


On Feb. 16, 2015, 11:21 a.m., Craig Wickesser wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31042/
> ---
> 
> (Updated Feb. 16, 2015, 11:21 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Added document describing how to build Aurora for CentOS
> 
> 
> Diffs
> -
> 
>   docs/build-for-centos.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31042/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Craig Wickesser
> 
>

Re: Review Request 31144: Renaming OfferQueue into OfferManager.

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On Feb. 17, 2015, 6:11 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31144/
> ---
> 
> (Updated Feb. 17, 2015, 6:11 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Renaming OfferQueue into OfferManager to better reflect its meaning.
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 55bc2f7eeaae6d3ed6daaa1112460a9851579bc5 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 8c11ef8bd6609f3e4d97ca154d922898f8362446 
>   src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 
> 80f1d83fd32fe1a2313856dc50ff8c8a69221b99 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> 7f2c7605ad6e676dec3384913f23945e795dcb9e 
>   src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 
> 332338b7bef7d622333f8ea6508c4f5970b8e7c4 
>   
> src/main/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelay.java 
> 2accb4e1051638fb16ca0411ccc6985e4dbb3b65 
>   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
> ced3bdef1731354aedc82bb12f45ba6f040e1ab7 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
> 6e89075db3c2500a9546d9fbeefca717a3eb3f4c 
>   src/main/java/org/apache/aurora/scheduler/http/Offers.java 
> 6d75c3ae4693c62cb678dae9cc914e8d65295265 
>   src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java 
> bde45db33527118f5d53ad4ea9f71ddb6f84eee9 
>   src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 
> f96110c55011699768f17cc2d4afdc8bf7daa16c 
>   src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 
> 2b5dc4902f57e508d76f5e16997ae09d04464220 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
> 7963e473b83a04e29f70360833fc6e553646fc4c 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
> 74e31334bc139c47eb8b0beee46ee7bad62a2f80 
>   
> src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
>  4c2448ffba5c7e0f0ea59fc6484fbcdfc7df7f52 
>   src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
> 652d2477e0fd69d9f9f4d46a150145ad062cf5d2 
> 
> Diff: https://reviews.apache.org/r/31144/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.

[Kevin Sweeney]

> On Feb. 17, 2015, 4:59 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java, line 316
> > <https://reviews.apache.org/r/30891/diff/3/?file=862798#file862798line316>
> >
> > It seems like an abstraction violation for a thing named Queue to make 
> > logic decisions about its contents.  Have you considered having the 
> > `acceptor` own this behavior?
> 
> Maxim Khutornenko wrote:
> Well, I don't really see why it has "Queue" in its name in the first 
> place :) It does not really act like one. It accepts offers by iterating over 
> the entire collection of available offers and sends Mesos launchTask 
> requests. I'd rather rename it to OfferCache or OfferHandler if it helps.
> 
> Regarding the abstraction violation, I actually see the static ban 
> details belonging more to the OfferQueue than the acceptor as its more about 
> offer state wrt a given task group than a specific assignment request.
> 
> Bill Farner wrote:
> > I actually see the static ban details belonging more to the OfferQueue 
> than the acceptor as its more about offer state wrt a given task group than a 
> specific assignment request
> 
> That makes sense, otherwise you need to figure out a way to plumb 
> invalidation to the third party.
> 
> +1 to the rename

+1 to the rename. I vote for OfferManager to keep parity with other naming 
conventions.


- Kevin


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


On Feb. 12, 2015, 6:27 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30891/
> ---
> 
> (Updated Feb. 12, 2015, 6:27 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-909
> https://issues.apache.org/jira/browse/AURORA-909
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Offer filtering for static vetoes. Part 3 of 4: Filtering out statically 
> banned offers.
> 
> Will not apply cleanly: diffed with https://reviews.apache.org/r/30890 as a 
> parent.
> 
> Original RB: https://reviews.apache.org/r/28617/
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 
> 332338b7bef7d622333f8ea6508c4f5970b8e7c4 
>   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
> ced3bdef1731354aedc82bb12f45ba6f040e1ab7 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
> 5a0f7ddb7e8fa6869cbb0fdfd07c6881780c6917 
>   src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 
> 2b5dc4902f57e508d76f5e16997ae09d04464220 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
> d0e11932e8b5ba1393279137c8465a308e1d6bf5 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
> 74e31334bc139c47eb8b0beee46ee7bad62a2f80 
> 
> Diff: https://reviews.apache.org/r/30891/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 31136: Uniquely identify job updates in the database by JobUpdateKey.

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On Feb. 17, 2015, 3:06 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31136/
> ---
> 
> (Updated Feb. 17, 2015, 3:06 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is stage 1 of 2 or 3 to use `IJobUpdateKey` throughout the system to 
> uniquely identify job updates.  This change is localized to the database.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 2a77f28b847d4897315e63c62455e49b370ea5fa 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 39f72ccd8a61d7691d587f1597dc705c44d01ee2 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java
>  d5dd5a535a017e84c628a740bcc17cc5ad50e5cc 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  028eb7c0c70049c92f1060625b4a5c498a83d845 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.java
>  bbd2f465d68109a8d974830a9a4520f28f92a786 
>   src/main/java/org/apache/aurora/scheduler/storage/db/PruneVictim.java 
> PRE-CREATION 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.xml
>  1bc2a620661d766918e655307f76154488c66c5d 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  7fd3a86f636523328ef671b60486fb3f1c3178ec 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml
>  9f23c4b6cdc015465d68c219dfbecf3522f5c551 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 987596f733b7155fbce772e6c74a8095d5da1827 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  be5e7727f782200d633fee1cf17491272431f794 
> 
> Diff: https://reviews.apache.org/r/31136/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31136: Uniquely identify job updates in the database by JobUpdateKey.

[Kevin Sweeney]

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



api/src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/31136/#comment118879>

Should these be marked deprecated now (to be removed in 0.9.0?)



src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java
<https://reviews.apache.org/r/31136/#comment118896>

Consdier adding test coverage for these new branches



src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
<https://reviews.apache.org/r/31136/#comment118892>

is this a new convention for parameter names? the below ones use camelCase.



src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
<https://reviews.apache.org/r/31136/#comment118894>

why the underscores here?



src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql
<https://reviews.apache.org/r/31136/#comment118895>

Is there a good reason to reuse update_ids? Why not aim for 

UNIQUE(update_id),
UNIQUE(job_key_id, update_id),
?


- Kevin Sweeney


On Feb. 17, 2015, 2:01 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31136/
> ---
> 
> (Updated Feb. 17, 2015, 2:01 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is stage 1 of 2 or 3 to use `IJobUpdateKey` throughout the system to 
> uniquely identify job updates.  This change is localized to the database.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 2a77f28b847d4897315e63c62455e49b370ea5fa 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 39f72ccd8a61d7691d587f1597dc705c44d01ee2 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java
>  d5dd5a535a017e84c628a740bcc17cc5ad50e5cc 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  028eb7c0c70049c92f1060625b4a5c498a83d845 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.java
>  bbd2f465d68109a8d974830a9a4520f28f92a786 
>   src/main/java/org/apache/aurora/scheduler/storage/db/PruneVictim.java 
> PRE-CREATION 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.xml
>  1bc2a620661d766918e655307f76154488c66c5d 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  7fd3a86f636523328ef671b60486fb3f1c3178ec 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml
>  9f23c4b6cdc015465d68c219dfbecf3522f5c551 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 987596f733b7155fbce772e6c74a8095d5da1827 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  be5e7727f782200d633fee1cf17491272431f794 
> 
> Diff: https://reviews.apache.org/r/31136/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31135: Pin protobuf dependency to 2.6.1

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On Feb. 17, 2015, 1:15 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31135/
> ---
> 
> (Updated Feb. 17, 2015, 1:15 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1128
> https://issues.apache.org/jira/browse/AURORA-1128
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Pin protobuf dependency to 2.6.1
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt PRE-CREATION 
>   src/main/python/apache/aurora/executor/BUILD 
> 1ad8f82cdce85cf228c53e088171918e36ed536d 
>   src/main/python/apache/aurora/executor/bin/BUILD 
> aeb8aee6f50a0d89714626e933699c0a13b363d9 
>   src/main/python/apache/aurora/executor/common/BUILD 
> 335ebc4809096c5f128846cd846d33910a777968 
> 
> Diff: https://reviews.apache.org/r/31135/diff/
> 
> 
> Testing
> ---
> 
> $ ./pants binary src/main/python/apache/aurora/executor/bin:thermos_executor 
> && ./pants binary src/main/python/apache/aurora/executor/bin:gc_executor
> $ unzip gc_executor.pex -d gc_executor
> $ unzip thermos_executor.pex -d thermos_executor
> $ ls gc_executor/.deps/ | grep protobuf
> protobuf-2.6.1-py2.7.egg
> $ ls thermos_executor/.deps/ | grep protobuf
> protobuf-2.6.1-py2.7.egg
> $ ./pants test.pytest --no-fast src/test/python::
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 31101: Native Debian packaging for Aurora

[Kevin Sweeney]

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


This is great! If this is gonna be mainline and not "contrib", how would you 
feel about adding some tests for this to Jenkins so that reviewbot can check 
this? You should be able to edit build-support/jenkins/build.sh. The Jenkins 
slaves have docker available so it should be possible to run in a target 
environment.


debian/control
<https://reviews.apache.org/r/31101/#comment118857>

https



debian/control
<https://reviews.apache.org/r/31101/#comment118859>

"Aurora is a service scheduler..."

here and below



debian/embed-runner-in-executor.py
<https://reviews.apache.org/r/31101/#comment118861>

Any chance we can merge this with the vagrant build script or share this 
snippet of code between them some way?


- Kevin Sweeney


On Feb. 16, 2015, 4:20 p.m., Benjamin Staffin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31101/
> ---
> 
> (Updated Feb. 16, 2015, 4:20 p.m.)
> 
> 
> Review request for Aurora and Jake Farrell.
> 
> 
> Bugs: AURORA-951
> https://issues.apache.org/jira/browse/AURORA-951
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Native Debian packaging for Aurora
> 
> This uses Debian package dependencies to install gradle and thrift-compiler.  
> Gradle 2.2.1 isn't in Debian or Ubuntu yet, but you can build your own 
> package easily enough using https://github.com/benley/gradle-packaging
> 
> 
> Diffs
> -
> 
>   debian/aurora-doc.docs PRE-CREATION 
>   debian/aurora-doc.examples PRE-CREATION 
>   debian/aurora-executor.dirs PRE-CREATION 
>   debian/aurora-executor.install PRE-CREATION 
>   debian/aurora-executor.links PRE-CREATION 
>   debian/aurora-executor.thermos.default PRE-CREATION 
>   debian/aurora-executor.thermos.init PRE-CREATION 
>   debian/aurora-executor.thermos.upstart PRE-CREATION 
>   debian/aurora-scheduler.default PRE-CREATION 
>   debian/aurora-scheduler.init PRE-CREATION 
>   debian/aurora-scheduler.install PRE-CREATION 
>   debian/aurora-scheduler.links PRE-CREATION 
>   debian/aurora-scheduler.postinst PRE-CREATION 
>   debian/aurora-scheduler.upstart PRE-CREATION 
>   debian/aurora-tools.install PRE-CREATION 
>   debian/aurora-tools.links PRE-CREATION 
>   debian/changelog PRE-CREATION 
>   debian/clusters.json PRE-CREATION 
>   debian/compat PRE-CREATION 
>   debian/control PRE-CREATION 
>   debian/copyright PRE-CREATION 
>   debian/embed-runner-in-executor.py PRE-CREATION 
>   debian/pants.ini PRE-CREATION 
>   debian/rules PRE-CREATION 
>   debian/source/format PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31101/diff/
> 
> 
> Testing
> ---
> 
> Built debs in a clean Ubuntu 14.04 environment using git-buildpackage and 
> pbuilder. Have been using debs built from this branch on a testing cluster 
> for a few months now.
> 
> 
> Thanks,
> 
> Benjamin Staffin
> 
>

Re: Review Request 31123: Enable checkstyle indentation check, fix violations.

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On Feb. 17, 2015, 10 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31123/
> ---
> 
> (Updated Feb. 17, 2015, 10 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is the first step of many to align our configuration with checkstyle's 
> Google style config.  There are enough differences that a full switch will 
> touch every file in our code base, so i'd like to start with targeted changes.
> 
> 
> Diffs
> -
> 
>   config/checkstyle/checkstyle.xml c4c6c4457c879fb4becedfff51847ec840df9300 
>   src/main/java/org/apache/aurora/scheduler/configuration/Resources.java 
> b5a3140e3560f790d1db496dca3c2ee0dc96a195 
>   src/main/java/org/apache/aurora/scheduler/http/StructDump.java 
> 12b28be5a5c711edeed10d8e570e3d61cf9ccfbf 
>   src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java 
> eae79d59b445ea58f46dc9e3107c03fbd83b6a95 
>   src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java 
> 9158271f255eb5c04a1394bf4d470875e8e07b45 
>   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
> cb870f9a127b97d3274d07765a65bcb3eaa40dea 
>   src/main/java/org/apache/aurora/scheduler/storage/SnapshotStore.java 
> 49584a9e8158f80a45f2df34f917d4bf2229c87b 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 2055e11149ca18180a5f21a36e8c16099f0efa49 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 1b7985084d227ef09d483665ae934540a1512ef4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  2a9d36ab2c01960dc5384fc3ed90df4e11c0b12a 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
> 5647349854a5e04de749c4d809684a0066d4da06 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 5f08d00d39f016af9bc296e517ad49b66ab5a8de 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> bc26c4c742a7d2e022edae79e1b06f1982a27764 
> 
> Diff: https://reviews.apache.org/r/31123/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 30957: Extract ReadOnlyScheduler to its own implementation class

[Kevin Sweeney]

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


@ReviewBot retry

- Kevin Sweeney


On Feb. 12, 2015, 4:55 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30957/
> ---
> 
> (Updated Feb. 12, 2015, 4:55 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Extract ReadOnlyScheduler to its own implementation class and delegate from 
> the existing SchedulerThriftInterface to it.
> 
> This enables serving ReadOnlyScheduler from a separate HTTP endpoint (e.g. an 
> unauthenticated one).
> 
> This is almost a pure tool-driven refactor-rename change. Also renames Util 
> to Responses for improved ergonomics.
> 
> To boost confidence in this change tests have been left in place. I will 
> follow-up with a subsequent review to split out ReadOnlySchedulerImplTest.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
> 53ea03bac3784baebc630ad1ce235d263985cafe 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  7014d598d41f007b0dee0b2db1aa2d4cdd592be6 
>   src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
> 7b28eb87767d7cd19e9365e3287f3e943d87dea5 
>   src/main/java/org/apache/aurora/scheduler/thrift/Util.java 
> 55242d18e08ea5cb6dd297bd7f18744d952580b3 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java
>  e176a0df3141dcb088e05203cca4de3f0d3feea7 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
> cad63c77e8144bb64c6b2acaf1b9199be13e4145 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/UserCapabilityInterceptor.java
>  5e6577d64b1037c69c3a952008240dcafe3b9f94 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  491aa34d76a6f3fe91188bbc73a2cc7464f3644b 
> 
> Diff: https://reviews.apache.org/r/30957/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 30957: Extract ReadOnlyScheduler to its own implementation class

[Kevin Sweeney]

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


@ReviewBot retry

- Kevin Sweeney


On Feb. 12, 2015, 4:55 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30957/
> ---
> 
> (Updated Feb. 12, 2015, 4:55 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Extract ReadOnlyScheduler to its own implementation class and delegate from 
> the existing SchedulerThriftInterface to it.
> 
> This enables serving ReadOnlyScheduler from a separate HTTP endpoint (e.g. an 
> unauthenticated one).
> 
> This is almost a pure tool-driven refactor-rename change. Also renames Util 
> to Responses for improved ergonomics.
> 
> To boost confidence in this change tests have been left in place. I will 
> follow-up with a subsequent review to split out ReadOnlySchedulerImplTest.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
> 53ea03bac3784baebc630ad1ce235d263985cafe 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  7014d598d41f007b0dee0b2db1aa2d4cdd592be6 
>   src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
> 7b28eb87767d7cd19e9365e3287f3e943d87dea5 
>   src/main/java/org/apache/aurora/scheduler/thrift/Util.java 
> 55242d18e08ea5cb6dd297bd7f18744d952580b3 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java
>  e176a0df3141dcb088e05203cca4de3f0d3feea7 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
> cad63c77e8144bb64c6b2acaf1b9199be13e4145 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/UserCapabilityInterceptor.java
>  5e6577d64b1037c69c3a952008240dcafe3b9f94 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  491aa34d76a6f3fe91188bbc73a2cc7464f3644b 
> 
> Diff: https://reviews.apache.org/r/30957/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 30890: Offer filtering for static vetoes. Part 2 of 4: Veto groups.

[Kevin Sweeney]

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

Ship it!



src/main/java/org/apache/aurora/scheduler/TaskVars.java
<https://reviews.apache.org/r/30890/#comment118384>

Can you add a getCounterName() abstract method to the VetoGroup enum? That 
will make the compiler guarantee that this mapping is complete.



src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java
<https://reviews.apache.org/r/30890/#comment118385>

Can you use != here since this is an enum?



src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java
<https://reviews.apache.org/r/30890/#comment118386>

(Consider moving this test to compile-time with the above approach)


- Kevin Sweeney


On Feb. 12, 2015, 4:58 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30890/
> ---
> 
> (Updated Feb. 12, 2015, 4:58 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-909
> https://issues.apache.org/jira/browse/AURORA-909
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Offer filtering for static vetoes. Part 2 of 4: Exposing Veto groups and 
> refactoring TaskVars.
> 
> Original RB: https://reviews.apache.org/r/28617/
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/TaskVars.java 
> f017cdd26ca40138a7e141f21613ed567314c399 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
> 6a43bcd1719e8aa32fd3fcb7387d0318c3c0b804 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> f06fdaeb92e154d0982bdabed5df93e7bcba9048 
>   src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 
> 4e7efb3c1214c3d193afd61f162713490eb8effb 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  52ee7c1e3742d9315c7e7aaa77677121e1e9288d 
> 
> Diff: https://reviews.apache.org/r/30890/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 29117: Adding thrift API changes document.

[Kevin Sweeney]

> On Feb. 12, 2015, 4:41 p.m., David McLaughlin wrote:
> > docs/thrift-deprecation.md, line 20
> > 
> >
> > Sorry I missed this first review, but AFAIK this isn't true for dynamic 
> > languages (incl. our own scheduler UI)?
> 
> Maxim Khutornenko wrote:
> Can you elaborate? The above statement assumes renaming the field 
> throughout the codebase (including java, python and JS). Current +- 1 
> versions of client will still be able to communicate with the scheduler. UI 
> ships with the scheduler and thus will not have the renaming problelm. What 
> am I missing?

Yeah, and even an old version of the UI is still using TJSONProtocol so it's 
decoding the field number to its own mapping of names. So as long as it's 
consistent with itself it's fine.

If a downstream consumer updates its IDL then yes it'll need to change the name 
too.


- Kevin


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


On Feb. 12, 2015, 4:35 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29117/
> ---
> 
> (Updated Feb. 12, 2015, 4:35 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-973
> https://issues.apache.org/jira/browse/AURORA-973
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a first stab at documenting thrift deprecation. Any 
> suggestions/comments are welcome.
> 
> 
> Diffs
> -
> 
>   docs/developing-aurora-client.md e002d908ce07020513fa06c7e70c41f9e2f55b1d 
>   docs/developing-aurora-scheduler.md 
> 7f6cc2e6c8e01115a9b7a7dc7633bcd88ba02a0f 
>   docs/thrift-deprecation.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29117/diff/
> 
> 
> Testing
> ---
> 
> https://github.com/maxim111333/incubator-aurora/blob/populated_deprecation/docs/thrift-deprecation.md
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 30957: Extract ReadOnlyScheduler to its own implementation class

[Kevin Sweeney]

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

(Updated Feb. 12, 2015, 4:55 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

fix bad rebase.


Repository: aurora


Description
---

Extract ReadOnlyScheduler to its own implementation class and delegate from the 
existing SchedulerThriftInterface to it.

This enables serving ReadOnlyScheduler from a separate HTTP endpoint (e.g. an 
unauthenticated one).

This is almost a pure tool-driven refactor-rename change. Also renames Util to 
Responses for improved ergonomics.

To boost confidence in this change tests have been left in place. I will 
follow-up with a subsequent review to split out ReadOnlySchedulerImplTest.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
53ea03bac3784baebc630ad1ce235d263985cafe 
  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
7014d598d41f007b0dee0b2db1aa2d4cdd592be6 
  src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
7b28eb87767d7cd19e9365e3287f3e943d87dea5 
  src/main/java/org/apache/aurora/scheduler/thrift/Util.java 
55242d18e08ea5cb6dd297bd7f18744d952580b3 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java
 e176a0df3141dcb088e05203cca4de3f0d3feea7 
  src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
cad63c77e8144bb64c6b2acaf1b9199be13e4145 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/UserCapabilityInterceptor.java
 5e6577d64b1037c69c3a952008240dcafe3b9f94 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 491aa34d76a6f3fe91188bbc73a2cc7464f3644b 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney

Re: Review Request 30957: Extract ReadOnlyScheduler to its own implementation class

[Kevin Sweeney]

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

(Updated Feb. 12, 2015, 4:33 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

rebase


Repository: aurora


Description
---

Extract ReadOnlyScheduler to its own implementation class and delegate from the 
existing SchedulerThriftInterface to it.

This enables serving ReadOnlyScheduler from a separate HTTP endpoint (e.g. an 
unauthenticated one).

This is almost a pure tool-driven refactor-rename change. Also renames Util to 
Responses for improved ergonomics.

To boost confidence in this change tests have been left in place. I will 
follow-up with a subsequent review to split out ReadOnlySchedulerImplTest.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
53ea03bac3784baebc630ad1ce235d263985cafe 
  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
7014d598d41f007b0dee0b2db1aa2d4cdd592be6 
  src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
7b28eb87767d7cd19e9365e3287f3e943d87dea5 
  src/main/java/org/apache/aurora/scheduler/thrift/Util.java 
55242d18e08ea5cb6dd297bd7f18744d952580b3 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java
 e176a0df3141dcb088e05203cca4de3f0d3feea7 
  src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
cad63c77e8144bb64c6b2acaf1b9199be13e4145 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/UserCapabilityInterceptor.java
 5e6577d64b1037c69c3a952008240dcafe3b9f94 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 491aa34d76a6f3fe91188bbc73a2cc7464f3644b 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney

Review Request 30957: Extract ReadOnlyScheduler to its own implementation class

[Kevin Sweeney]

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

Review request for Aurora, Joshua Cohen and Bill Farner.


Repository: aurora


Description
---

Extract ReadOnlyScheduler to its own implementation class and delegate from the 
existing SchedulerThriftInterface to it.

This enables serving ReadOnlyScheduler from a separate HTTP endpoint (e.g. an 
unauthenticated one).

This is almost a pure tool-driven refactor-rename change. Also renames Util to 
Responses for improved ergonomics.

To boost confidence in this change tests have been left in place. I will 
follow-up with a subsequent review to split out ReadOnlySchedulerImplTest.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
53ea03bac3784baebc630ad1ce235d263985cafe 
  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
8c19f3b08135eb5f3098591ebf9931b42a086318 
  src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
7b28eb87767d7cd19e9365e3287f3e943d87dea5 
  src/main/java/org/apache/aurora/scheduler/thrift/Util.java 
55242d18e08ea5cb6dd297bd7f18744d952580b3 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java
 e176a0df3141dcb088e05203cca4de3f0d3feea7 
  src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
cad63c77e8144bb64c6b2acaf1b9199be13e4145 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/UserCapabilityInterceptor.java
 5e6577d64b1037c69c3a952008240dcafe3b9f94 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney

Re: Review Request 30888: Offer filtering for static vetoes. Part 1 of 4: TaskAssigner.

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On Feb. 11, 2015, 2:52 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30888/
> ---
> 
> (Updated Feb. 11, 2015, 2:52 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-909
> https://issues.apache.org/jira/browse/AURORA-909
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Offer filtering for static vetoes. Part 1 of 4: TaskAssigner changes to 
> return a new result object.
> 
> Original RB: https://reviews.apache.org/r/28617/
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 
> f66383830140e5eaba436f35ebb5192eee65947a 
>   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
> ce47ff152e303fd2116bc3b9e91c0c1a8f76f258 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
> e1c29747c9854cf75bf63f6f085cf40ca68989af 
>   src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 
> 4cf602ad32b972c18eb5a81e9b2f59c67859bdb2 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
> 5647349854a5e04de749c4d809684a0066d4da06 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
> 6cc13231560996b144101eba36577f49017aba06 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> 411a55a8d85f60bb2703468f2d69b64b2736eee4 
> 
> Diff: https://reviews.apache.org/r/30888/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 27182: Add a test for the thermos resource module

[Kevin Sweeney]

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


Please drop me from this review - wickman has more context here.

- Kevin Sweeney


On Feb. 6, 2015, 11:59 a.m., Joe Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27182/
> ---
> 
> (Updated Feb. 6, 2015, 11:59 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add a test for the thermos resource module
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/monitoring/monitor.py 
> 8f87f5ffc39c87e87ff78b941ea30df7138bd1ef 
>   src/test/python/apache/thermos/monitoring/BUILD 
> 33d6bba43aff6d62b2646491f004475c27ed99db 
>   src/test/python/apache/thermos/monitoring/test_resource.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/27182/diff/
> 
> 
> Testing
> ---
> 
> [tw-mbp-jsmith incubator-aurora (yasumoto/thermos_test)]$ ./pants test.pytest 
> ./src/test/python/apache/thermos/monitoring:test_resource
> 
> 21:15:56 00:00 [main]
>(To run a reporting server: ./pants server)
> 21:15:56 00:00   [bootstrap]
> 21:15:57 00:01   [setup]
> 21:15:57 00:01 [parse]
>Executing tasks in goals: bootstrap -> imports -> gen -> 
> check-exclusives -> resolve -> compile -> resources -> test
> 21:15:57 00:01   [bootstrap]
> 21:15:57 00:01 [bootstrap-jvm-tools]
> 21:15:57 00:01   [imports]
> 21:15:57 00:01 [ivy-imports]
> 21:15:57 00:01   [gen]
> 21:15:57 00:01 [thrift]
> 21:15:57 00:01 [scrooge]
> 21:15:57 00:01 [protoc]
> 21:15:57 00:01 [antlr]
> 21:15:57 00:01 [ragel]
> 21:15:57 00:01 [jaxb]
> 21:15:57 00:01 [wire]
> 21:15:57 00:01 [aapt]
> 21:15:57 00:01   [check-exclusives]
> 21:15:57 00:01 [check-exclusives]
> 21:15:57 00:01   [resolve]
> 21:15:57 00:01 [ivy]
> 21:15:57 00:01   [compile]
> 21:15:57 00:01 [jvm]
> 21:15:57 00:01   [jvm-compilers]
> 21:15:57 00:01   [resources]
> 21:15:57 00:01 [prepare]
> 21:15:57 00:01   [test]
> 21:15:57 00:01 [run_prep_command]
> 21:15:57 00:01 [pytest]
> 21:15:57 00:01   [run]
>  == test session starts ===
>  platform darwin -- Python 2.7.6 -- py-1.4.26 -- 
> pytest-2.6.4
>  plugins: cov, timeout
>  collected 5 items 
>  
>  
> src/test/python/apache/thermos/monitoring/test_resource.py .
>  
>   5 passed in 0.18 seconds 
>  
> 21:16:04 00:08 [junit]
> 21:16:04 00:08 [specs]
>SUCCESS
> 
> 
> Thanks,
> 
> Joe Smith
> 
>

Re: Review Request 30467: Update mesos lib to 0.21.1

[Kevin Sweeney]

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

Ship it!


LGTM pending a successful end-to-end test.

- Kevin Sweeney


On Feb. 3, 2015, 5:42 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30467/
> ---
> 
> (Updated Feb. 3, 2015, 5:42 p.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1080
> https://issues.apache.org/jira/browse/AURORA-1080
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is needed to take advantage of the task reconciliation API and new 
> status update fields.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 0c88e4c299ffe5886bb3ab7d83b0187cb4b7888a 
>   build-support/python/make-mesos-native-egg 
> 2cba8ede04e22aee1e6ec5c979161904a6f8fddb 
>   build.gradle f9f71a84493b782e9f6072e44e89a2c017cf2a09 
>   docs/deploying-aurora-scheduler.md 9bf5b5a92bf65b31b2f8fda102003b113f61186a 
>   examples/vagrant/provision-dev-cluster.sh 
> fa0de88314169bc9fe31aaf41126cedd0865ed5a 
> 
> Diff: https://reviews.apache.org/r/30467/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

[Kevin Sweeney]

> On Feb. 2, 2015, 4:16 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java,
> >  line 1535
> > <https://reviews.apache.org/r/30325/diff/3/?file=841928#file841928line1535>
> >
> > This is too restrictive.  It means that _only_ the update coordinator 
> > role may call this RPC, and that a user cannot build something to pulse 
> > their own updates.
> > 
> > You should instead do what `isAdmin` does and fall back to the 
> > coordinator role if direct auth fails.
> > 
> > This is an unfortunate state of affairs, and hopefully the move to 
> > shiro dramatically improves all this.
> 
> Maxim Khutornenko wrote:
> I don't see how it's necessarily better. Pulsing can always be done under 
> UPDATE_COORDINATOR membership, which is specifically covering heartbeats 
> only. The isAdmin() requires ROOT that opens up everything else, including 
> the ability to kill anyone's tasks in the claster. Isn't that more 
> restrictive in real life? We don't expect regular users having ROOT access, 
> meaning they will unlikely to get to write their own pulse routine due to 
> that. 
> 
> Or maybe you suggesting an approach similar to killTasks(), where an 
> admin check followed by a role validation? The problem here is that we don't 
> have a job key to extract the role for authorization. Perhaps we can change 
> the RPC to accept a job key instead but that would open up for a race we 
> don't want to see (e.g. late pulse arrives for a wrong update ID). We could 
> probably get away with both updateId and jobKey in the API to avoid 
> ambiguity, or perhpas just updateId and role? I am open to suggestions.
> 
> Kevin Sweeney wrote:
> In Shiro this will look something like
> 
> subject.checkPermission("update:resume:mesos:prod:labrat");
> 
> With role update_coordinator having:
> ```
> update:*
> ```
> 
> role root having:
> ```
> *
> ```
> 
> and user mesos having:
> ```
> *:*:mesos
> ```
> 
> So they'd all be allowed.
> 
> Bill Farner wrote:
> > Or maybe you suggesting an approach similar to killTasks(), where an 
> admin check followed by a role validation?
> 
> Yeah, this.  I'm not asking for update heartbeats to require ROOT.
> 
> > The problem here is that we don't have a job key to extract the role 
> for authorization.
> 
> We have an association between {{updateId}} and role owning the job, 
> right?
> 
> /**
>  * Fetches a read-only view of a job update.
>  *
>  * @param updateId Update ID to fetch.
>  * @return A read-only view of job update.
>  */
> Optional fetchJobUpdate(String updateId);
> 
> Maxim Khutornenko wrote:
> We do. However, it would require accessing the store in order to 
> authenticate the call. This is a new pattern we have not tried before and it 
> may potentially interfere with moving to pure AOP auth.

Operations scoped to an object owned by a role will not be enforcable with pure 
AOP, as many will typically require a storage lookup (usually by ID) to 
determine the owner of the object being operated on.

Operations that happen on "global" like `"snapshot:create"` will be enforceable 
with pure AOP.


- Kevin


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


On Jan. 30, 2015, 9:23 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30325/
> ---
> 
> (Updated Jan. 30, 2015, 9:23 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner.
> 
> 
> Bugs: AURORA-1009
> https://issues.apache.org/jira/browse/AURORA-1009
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implemented the `pulseJobUpdate` RPC. Also, moved it into AuroraAdmin 
> interface to support AOP capability validation. 
> 
> The RB is diffed against https://reviews.apache.org/r/30225/
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 4f603f9e7ed004e53937a45ea8edf7241e15f5cf 
>   src/main/java/org/apache/aurora/auth/CapabilityValidator.java 
> 45ef643ebe57c1517cdae373574331ea302a8b74 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  8c19f3b08135eb5f3098591ebf9931b42a086318 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  03d1fba76c23570c2c4102a48daf5ce035ecaaa3 
> 
> Diff: https://reviews.apache.org/r/30325/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 30535: Remove shard uniqueness check from scheduler recovery phase.

[Kevin Sweeney]

+1 to moving forward to this patch as-is

On Mon, Feb 2, 2015 at 5:39 PM, Bill Farner  wrote:

> They could, but I don't think it would be worth the effort here.  In this
> case, the code itself is of little value, it's a bonus that it is also a
> performance hog.
>
>
> On Monday, February 2, 2015, Zameer Manji  wrote:
>
>> Could the impact of this change be verified by our performance benchmarks?
>>
>> On Mon, Feb 2, 2015 at 5:06 PM, Aurora ReviewBot 
>> wrote:
>>
>>>
>>> ---
>>> This is an automatically generated e-mail. To reply, visit:
>>> https://reviews.apache.org/r/30535/#review70684
>>> ---
>>>
>>> Ship it!
>>>
>>>
>>> Master (a674581) 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 Feb. 3, 2015, 12:42 a.m., Bill Farner wrote:
>>> >
>>> > ---
>>> > This is an automatically generated e-mail. To reply, visit:
>>> > https://reviews.apache.org/r/30535/
>>> > ---
>>> >
>>> > (Updated Feb. 3, 2015, 12:42 a.m.)
>>> >
>>> >
>>> > Review request for Aurora, David McLaughlin, Kevin Sweeney, and Maxim
>>> Khutornenko.
>>> >
>>> >
>>> > Bugs: AURORA-1090
>>> > https://issues.apache.org/jira/browse/AURORA-1090
>>> >
>>> >
>>> > Repository: aurora
>>> >
>>> >
>>> > Description
>>> > ---
>>> >
>>> > Remove shard uniqueness check from scheduler recovery phase.
>>> >
>>> >
>>> > Diffs
>>> > -
>>> >
>>> >
>>>  src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java
>>> 1814658c044273f7c3a2348a16aea62e397cf860
>>> >
>>>  src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java
>>> 93773eb5ba3bee1b3296e69ea30eabb531eeb661
>>> >
>>> > Diff: https://reviews.apache.org/r/30535/diff/
>>> >
>>> >
>>> > Testing
>>> > ---
>>> >
>>> >
>>> > Thanks,
>>> >
>>> > Bill Farner
>>> >
>>> >
>>>
>>>
>>
>>
>> --
>> Zameer Manji
>>
>
>
> --
> -=Bill
>
>