Re: Review Request 35954: Prevent quota from being set below current production reservation.

2015-06-30 Thread Bill Farner 

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

Ship it!


One nit, but i don't feel strongly about it.


src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java (line 156)


It would be nice if comparison logic resided in QuotaCheckResult alongside 
the quota overage checks.


- Bill Farner


On June 30, 2015, 12:06 a.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35954/
> ---
> 
> (Updated June 30, 2015, 12:06 a.m.)
> 
> 
> Review request for Aurora, Joe Smith, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1375
> https://issues.apache.org/jira/browse/AURORA-1375
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Having quota below current production resesrvation is illogical and the 
> scheduler should reject requests that would result in this.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 7453680af5a518012f9779f82d0349e897c04994 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  9af379c36d2e3e44f462ed5d431f8a497b2d09f6 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> 58ee226a3819d6796a169156320c10677e35611a 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  ee8f5423810c60b2075e6b98ba8cda36393ae5cc 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 2a2b499905a788eb59f5824bc66b487f91e838f1 
> 
> Diff: https://reviews.apache.org/r/35954/diff/
> 
> 
> Testing
> ---
> 
> ./rbt post -o
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 35954: Prevent quota from being set below current production reservation.

2015-06-29 Thread Aurora ReviewBot 

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

Ship it!


Master (a1af1f6) 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 June 30, 2015, 12:06 a.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35954/
> ---
> 
> (Updated June 30, 2015, 12:06 a.m.)
> 
> 
> Review request for Aurora, Joe Smith, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1375
> https://issues.apache.org/jira/browse/AURORA-1375
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Having quota below current production resesrvation is illogical and the 
> scheduler should reject requests that would result in this.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 7453680af5a518012f9779f82d0349e897c04994 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  9af379c36d2e3e44f462ed5d431f8a497b2d09f6 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> 58ee226a3819d6796a169156320c10677e35611a 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  ee8f5423810c60b2075e6b98ba8cda36393ae5cc 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 2a2b499905a788eb59f5824bc66b487f91e838f1 
> 
> Diff: https://reviews.apache.org/r/35954/diff/
> 
> 
> Testing
> ---
> 
> ./rbt post -o
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 35954: Prevent quota from being set below current production reservation.

2015-06-29 Thread Zameer Manji 

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

(Updated June 29, 2015, 5:06 p.m.)


Review request for Aurora, Joe Smith, Maxim Khutornenko, and Bill Farner.


Changes
---

Style change from Maxim.


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


Repository: aurora


Description
---

Having quota below current production resesrvation is illogical and the 
scheduler should reject requests that would result in this.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
7453680af5a518012f9779f82d0349e897c04994 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
9af379c36d2e3e44f462ed5d431f8a497b2d09f6 
  src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
58ee226a3819d6796a169156320c10677e35611a 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 ee8f5423810c60b2075e6b98ba8cda36393ae5cc 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
2a2b499905a788eb59f5824bc66b487f91e838f1 

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


Testing
---

./rbt post -o


Thanks,

Zameer Manji

Re: Review Request 35954: Prevent quota from being set below current production reservation.

2015-06-29 Thread Maxim Khutornenko 

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

Ship it!



src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java (line 160)


Pull this to previous line.


- Maxim Khutornenko


On June 29, 2015, 9:15 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35954/
> ---
> 
> (Updated June 29, 2015, 9:15 p.m.)
> 
> 
> Review request for Aurora, Joe Smith, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1375
> https://issues.apache.org/jira/browse/AURORA-1375
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Having quota below current production resesrvation is illogical and the 
> scheduler should reject requests that would result in this.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 7453680af5a518012f9779f82d0349e897c04994 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  9af379c36d2e3e44f462ed5d431f8a497b2d09f6 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> 58ee226a3819d6796a169156320c10677e35611a 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  ee8f5423810c60b2075e6b98ba8cda36393ae5cc 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 2a2b499905a788eb59f5824bc66b487f91e838f1 
> 
> Diff: https://reviews.apache.org/r/35954/diff/
> 
> 
> Testing
> ---
> 
> ./rbt post -o
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 35954: Prevent quota from being set below current production reservation.

2015-06-29 Thread Aurora ReviewBot 

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

Ship it!


Master (a1af1f6) 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 June 29, 2015, 9:15 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35954/
> ---
> 
> (Updated June 29, 2015, 9:15 p.m.)
> 
> 
> Review request for Aurora, Joe Smith, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1375
> https://issues.apache.org/jira/browse/AURORA-1375
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Having quota below current production resesrvation is illogical and the 
> scheduler should reject requests that would result in this.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 7453680af5a518012f9779f82d0349e897c04994 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  9af379c36d2e3e44f462ed5d431f8a497b2d09f6 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> 58ee226a3819d6796a169156320c10677e35611a 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  ee8f5423810c60b2075e6b98ba8cda36393ae5cc 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 2a2b499905a788eb59f5824bc66b487f91e838f1 
> 
> Diff: https://reviews.apache.org/r/35954/diff/
> 
> 
> Testing
> ---
> 
> ./rbt post -o
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 35954: Prevent quota from being set below current production reservation.

2015-06-29 Thread Zameer Manji 

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

(Updated June 29, 2015, 2:15 p.m.)


Review request for Aurora, Joe Smith, Maxim Khutornenko, and Bill Farner.


Changes
---

Remove unused imports.


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


Repository: aurora


Description
---

Having quota below current production resesrvation is illogical and the 
scheduler should reject requests that would result in this.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
7453680af5a518012f9779f82d0349e897c04994 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
9af379c36d2e3e44f462ed5d431f8a497b2d09f6 
  src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
58ee226a3819d6796a169156320c10677e35611a 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 ee8f5423810c60b2075e6b98ba8cda36393ae5cc 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
2a2b499905a788eb59f5824bc66b487f91e838f1 

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


Testing
---

./rbt post -o


Thanks,

Zameer Manji

Re: Review Request 35954: Prevent quota from being set below current production reservation.

2015-06-29 Thread Aurora ReviewBot 

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


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

: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
:compileJmhJavaNote: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java
 uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:processJmhResources UP-TO-DATE
:jmhClasses
:checkstyleJmh
:jsHint
:checkstyleMain[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java:39:8:
 Unused import - org.apache.aurora.scheduler.storage.QuotaStore.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java:40:8:
 Unused import - org.apache.aurora.scheduler.storage.Storage.
 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 59.232 secs


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

- Aurora ReviewBot


On June 29, 2015, 8:59 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35954/
> ---
> 
> (Updated June 29, 2015, 8:59 p.m.)
> 
> 
> Review request for Aurora, Joe Smith, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1375
> https://issues.apache.org/jira/browse/AURORA-1375
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Having quota below current production resesrvation is illogical and the 
> scheduler should reject requests that would result in this.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 7453680af5a518012f9779f82d0349e897c04994 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  9af379c36d2e3e44f462ed5d431f8a497b2d09f6 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> 58ee226a3819d6796a169156320c10677e35611a 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  ee8f5423810c60b2075e6b98ba8cda36393ae5cc 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 2a2b499905a788eb59f5824bc66b487f91e838f1 
> 
> Diff: https://reviews.apache.org/r/35954/diff/
> 
> 
> Testing
> ---
> 
> ./rbt post -o
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 35954: Prevent quota from being set below current production reservation.

2015-06-29 Thread Zameer Manji 


> On June 29, 2015, 8:35 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, lines 
> > 81-82
> > 
> >
> > I understand what you are trying to do here but we usually don't go 
> > that specific to control read/write access. Should just take a 
> > MutableStoreProvider instead.

Done.


> On June 29, 2015, 8:35 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, lines 
> > 162-163
> > 
> >
> > String.format() may be easier to read here.

Done.


- Zameer


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


On June 29, 2015, 1:59 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35954/
> ---
> 
> (Updated June 29, 2015, 1:59 p.m.)
> 
> 
> Review request for Aurora, Joe Smith, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1375
> https://issues.apache.org/jira/browse/AURORA-1375
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Having quota below current production resesrvation is illogical and the 
> scheduler should reject requests that would result in this.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 7453680af5a518012f9779f82d0349e897c04994 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  9af379c36d2e3e44f462ed5d431f8a497b2d09f6 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> 58ee226a3819d6796a169156320c10677e35611a 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  ee8f5423810c60b2075e6b98ba8cda36393ae5cc 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 2a2b499905a788eb59f5824bc66b487f91e838f1 
> 
> Diff: https://reviews.apache.org/r/35954/diff/
> 
> 
> Testing
> ---
> 
> ./rbt post -o
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 35954: Prevent quota from being set below current production reservation.

2015-06-29 Thread Zameer Manji 

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

(Updated June 29, 2015, 1:59 p.m.)


Review request for Aurora, Joe Smith, Maxim Khutornenko, and Bill Farner.


Changes
---

Maxim's feedback.


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


Repository: aurora


Description
---

Having quota below current production resesrvation is illogical and the 
scheduler should reject requests that would result in this.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
7453680af5a518012f9779f82d0349e897c04994 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
9af379c36d2e3e44f462ed5d431f8a497b2d09f6 
  src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
58ee226a3819d6796a169156320c10677e35611a 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 ee8f5423810c60b2075e6b98ba8cda36393ae5cc 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
2a2b499905a788eb59f5824bc66b487f91e838f1 

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


Testing
---

./rbt post -o


Thanks,

Zameer Manji

Re: Review Request 35954: Prevent quota from being set below current production reservation.

2015-06-29 Thread Maxim Khutornenko 

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



src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java (lines 81 - 
82)


I understand what you are trying to do here but we usually don't go that 
specific to control read/write access. Should just take a MutableStoreProvider 
instead.



src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java (lines 162 - 
163)


String.format() may be easier to read here.


- Maxim Khutornenko


On June 26, 2015, 11:54 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35954/
> ---
> 
> (Updated June 26, 2015, 11:54 p.m.)
> 
> 
> Review request for Aurora, Joe Smith, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1375
> https://issues.apache.org/jira/browse/AURORA-1375
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Having quota below current production resesrvation is illogical and the 
> scheduler should reject requests that would result in this.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 7453680af5a518012f9779f82d0349e897c04994 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  9af379c36d2e3e44f462ed5d431f8a497b2d09f6 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> 58ee226a3819d6796a169156320c10677e35611a 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  ee8f5423810c60b2075e6b98ba8cda36393ae5cc 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 2a2b499905a788eb59f5824bc66b487f91e838f1 
> 
> Diff: https://reviews.apache.org/r/35954/diff/
> 
> 
> Testing
> ---
> 
> ./rbt post -o
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 35954: Prevent quota from being set below current production reservation.

2015-06-26 Thread Aurora ReviewBot 

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

Ship it!


Master (2ef6a05) 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 June 26, 2015, 11:54 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35954/
> ---
> 
> (Updated June 26, 2015, 11:54 p.m.)
> 
> 
> Review request for Aurora, Joe Smith, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1375
> https://issues.apache.org/jira/browse/AURORA-1375
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Having quota below current production resesrvation is illogical and the 
> scheduler should reject requests that would result in this.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 7453680af5a518012f9779f82d0349e897c04994 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  9af379c36d2e3e44f462ed5d431f8a497b2d09f6 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> 58ee226a3819d6796a169156320c10677e35611a 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  ee8f5423810c60b2075e6b98ba8cda36393ae5cc 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 2a2b499905a788eb59f5824bc66b487f91e838f1 
> 
> Diff: https://reviews.apache.org/r/35954/diff/
> 
> 
> Testing
> ---
> 
> ./rbt post -o
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>