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

2015-03-17 Thread Joshua Cohen 


> On March 17, 2015, 7:56 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java,
> >  line 39
> > 
> >
> > Can you add a corresponding comment to api.thrift that any new methods 
> > (or parameters?) added should be added (or updated?) here.
> 
> Kevin Sweeney wrote:
> I'll followup with a test case that ensures all 
> AuroraSchedulerManager.Iface methods are represented here.

Even better, thanks!


- Joshua


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


On March 17, 2015, 8: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, 8: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.

2015-03-17 Thread Aurora ReviewBot 

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

Ship it!


Master (38d0a6c) 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 March 17, 2015, 8: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, 8: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.

2015-03-17 Thread 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
> > 
> >
> > 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.

2015-03-17 Thread 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.

2015-03-17 Thread Joshua Cohen 

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

Ship it!



src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java


Can you add a corresponding comment to api.thrift that any new methods (or 
parameters?) added should be added (or updated?) here.


- Joshua Cohen


On March 17, 2015, 7:41 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, 7:41 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.

2015-03-17 Thread Aurora ReviewBot 

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


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

make[2]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thrift-0.9.1'
make[1]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thrift-0.9.1'
make: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift'
:api:classesThriftNote: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:api:checkPython
:api:generateThriftEntitiesJava
:api:classesThriftEntities
:api:compileJava UP-TO-DATE
:api:generateThriftResources
:api:processResources UP-TO-DATE
:api:classes
:api:jar
:compileJava/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java:20:
 error: cannot find symbol
import 
org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.EnableUpdater;
  ^
  symbol:   class EnableUpdater
  location: class SchedulerThriftInterface
Note: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/com/twitter/common/args/apt/cmdline.arg.info.txt.2
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java:20:
 error: cannot find symbol
import 
org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.EnableUpdater;
  ^
  symbol:   class EnableUpdater
  location: class SchedulerThriftInterface
1 error
 FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':compileJava'.
> Compilation failed; see the compiler error output for details.

* 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: 1 mins 23.181 secs


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

- Aurora ReviewBot


On March 17, 2015, 7:41 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, 7:41 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.

2015-03-17 Thread 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.

2015-03-17 Thread Aurora ReviewBot 

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


This patch does not apply cleanly on master (38d0a6c), do you need to rebase?

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

- Aurora ReviewBot


On March 17, 2015, 7:18 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, 7:18 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.

2015-03-17 Thread 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.

2015-03-17 Thread 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.

2015-03-17 Thread Maxim Khutornenko 

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

Ship it!



src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java


You probably want @Nullable here as well.



src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java


same here


- Maxim Khutornenko


On March 17, 2015, 6:55 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, 6:55 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.

2015-03-17 Thread 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.

2015-03-17 Thread 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
> > 
> >
> >  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.

2015-03-17 Thread 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
> > 
> >
> > 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
> 
>

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

2015-03-17 Thread Maxim Khutornenko 

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



src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java


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.


- Maxim Khutornenko


On March 17, 2015, 1:29 a.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:29 a.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.

2015-03-17 Thread Bill Farner 

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

Ship it!


LGTM mod nits below and satisfying the build bot.


src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java


 for breaks.  check out the rendered javadoc in intellij to see what i 
mean.

ditto in other files



src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java


Maybe s/Annotated/Authorized/?  So as to say - implementations of this 
interface can be assured authorization has happened.  I don't feel strongly 
about this.


- Bill Farner


On March 17, 2015, 1:29 a.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:29 a.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.

2015-03-16 Thread Aurora ReviewBot 

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


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

Note: Recompile with -Xlint:unchecked for details.

:api:checkPython
:api:generateThriftEntitiesJava
:api:classesThriftEntities
:api:compileJava UP-TO-DATE
:api:generateThriftResources
:api:processResources UP-TO-DATE
:api:classes
:api:jar
:compileJavaNote: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/com/twitter/common/args/apt/cmdline.arg.info.txt.2

:processResources
:classes
:jar
:startScripts
:distTar
:distZip
:assemble
:compileJmhJava
:processJmhResources UP-TO-DATE
:jmhClasses
:checkstyleJmh
:jsHint
:checkstyleMain[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java:67:37:
 '{' is not followed by whitespace.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java:1:
 Line does not match expected header line of '^\/\*\*$'.
 FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':checkstyleMain'.
> Checkstyle rule violations were found. See the report at: 
> file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.xml

* 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: 1 mins 47.784 secs


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

- Aurora ReviewBot


On March 17, 2015, 1:29 a.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:29 a.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.

2015-03-16 Thread 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