Re: Review Request 38081: Dropping quota check for dedicated jobs and exposing dedicated consumption.

2015-09-08 Thread Bill Farner 

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

Ship it!



NEWS (line 16)


s/enforced for/consumed by/


- Bill Farner


On Sept. 8, 2015, 9:53 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38081/
> ---
> 
> (Updated Sept. 8, 2015, 9:53 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1460
> https://issues.apache.org/jira/browse/AURORA-1460
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This change drops quota enforcement for dedicated jobs and adds dedicated 
> resource consumption into the thrift API. There is now a shared/dedicated 
> consumption split for both prod and non-prod tasks.
> 
> NOTE: the GetQuotaResult will have to be deprecated and/or replaced with a 
> tier-based mapping when AURORA-1443 is addressed.
> 
> 
> Diffs
> -
> 
>   NEWS 83a1213e2ea4ab6b3706cd70c015ba9f16735520 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 21137bbbdc3010c6b1e2cc0ebb3b99bfa8490563 
>   src/main/java/org/apache/aurora/scheduler/ResourceAggregates.java 
> df796fcc91265c588872051f16bd2f10f73dc718 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaInfo.java 
> b40ecd0af7c1d1bb9372bd89c741622ce4c9040c 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 8787aeaa6655cfab1e0a6d5719f9e08a89df7631 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> 3d89e43659750de63d7588f8574e7a350caea04b 
>   src/main/python/apache/aurora/client/api/quota_check.py 
> 75406ac1d3ec9ba655daf7c6125f208f74290cfb 
>   src/main/python/apache/aurora/client/cli/quota.py 
> e8aa010f5ecce0464a8ad6b072bccba589fe16d7 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 98920196db34f2eb4dcad93429274517e7383efe 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> db60cd21d06d636505202bac7277a13dc24d46e6 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
>  a8df44612b605dff512d1ca686f0789b84ef9012 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> e0110e7ebe631b7b66b2341cedc10490da00a2ab 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  4d4e752088f7dca99675cc66782ae046bbd516d6 
>   src/test/python/apache/aurora/admin/test_admin.py 
> d793293acc7f77c0081968334c38b984d865fbc8 
>   src/test/python/apache/aurora/client/api/test_quota_check.py 
> 6c9bc373d1868bd6e0cae2f2218261f39fd7ab8f 
>   src/test/python/apache/aurora/client/cli/test_quota.py 
> 3573e4c4575577e2232d2b99ca781b06a03d48d7 
> 
> Diff: https://reviews.apache.org/r/38081/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 38169: Refactoring resources table to show dedicated.

2015-09-08 Thread Maxim Khutornenko 

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

(Updated Sept. 8, 2015, 9:10 p.m.)


Review request for Aurora, David McLaughlin and Joshua Cohen.


Changes
---

Joshua's comments.


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


Repository: aurora


Description
---

Making Resources table dynamic to display only non-zero resource vectors (see 
pics attached). Table width is now dependent on the number of visible columns. 

Not particularly happy about ng-class approach to control table width, so any 
suggestions how to make it better are welcome.

May not apply cleanly, diffed against 38081.


Diffs (updated)
-

  src/main/resources/scheduler/assets/js/controllers.js 
98920196db34f2eb4dcad93429274517e7383efe 
  src/main/resources/scheduler/assets/role.html 
2572c4b4497aa78d3fa31457a5dc7c57e6565027 

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


Testing
---


File Attachments


resources_full.png
  
https://reviews.apache.org/media/uploaded/files/2015/09/08/ada720ce-bb97-4321-99bb-0574bf14b653__resources_full.png
resources_partial.png
  
https://reviews.apache.org/media/uploaded/files/2015/09/08/a6e4aa7e-ca7e-4ca4-900e-fda6cfdcaff3__resources_partial.png
resources_min.png
  
https://reviews.apache.org/media/uploaded/files/2015/09/08/c44964f2-d8d7-41b0-8a4f-995a1e3538f1__resources_min.png


Thanks,

Maxim Khutornenko

Re: Review Request 38014: Remove StartupRegistry.

2015-09-08 Thread Aurora ReviewBot 

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

Ship it!


Master (2773826) 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 Sept. 8, 2015, 9:49 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38014/
> ---
> 
> (Updated Sept. 8, 2015, 9:49 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove StartupRegistry.
> 
> 
> Diffs
> -
> 
>   commons/src/main/java/org/apache/aurora/common/application/AppLauncher.java 
> 78ed7d0aae50c976eba9045b17587258edc588b0 
>   
> commons/src/main/java/org/apache/aurora/common/application/StartupRegistry.java
>  997ee77dd8f6e02c2becddf98e16cbbbeec4cb4f 
>   
> commons/src/main/java/org/apache/aurora/common/application/StartupStage.java 
> 80509010f37894108881d40cad9d8ade8610f309 
>   
> commons/src/main/java/org/apache/aurora/common/application/modules/LifecycleModule.java
>  1b6bd086a5a434cb7e4b92ed9cfd252e2518ba66 
>   
> commons/src/main/java/org/apache/aurora/common/application/modules/StatsModule.java
>  3959ce3d688dd50399185925d91f0014fc1c43f9 
>   
> commons/src/main/java/org/apache/aurora/common/stats/TimeSeriesRepository.java
>  6928e48073d152915ca42b6f46236b21c0882086 
>   
> commons/src/main/java/org/apache/aurora/common/stats/TimeSeriesRepositoryImpl.java
>  387e379a9c84d663f2af23e4760754a023219860 
>   config/legacy_untested_classes.txt 4bae43a43d00f71456f37f001fcd21ce6a2fb841 
>   src/main/java/org/apache/aurora/scheduler/SchedulerServicesModule.java 
> 1077816b696c4d2e97aafa59900b6acf2adce064 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6892a70042e25fd672475517325b4e4b69a0adab 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 8047622e206c9827e5cd8e40152a278d495bd0ff 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> b2ec13f40c12e5ee5663f4465734d6a80f3587cd 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
>  4af49d5dcb1925c4055f5ada8601f6fcab5d7d00 
>   src/test/java/org/apache/aurora/scheduler/events/PubsubEventModuleTest.java 
> cb38c3e047efac483445f43b941c7eea8862cc9c 
>   src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
> ed8e8119ac3dc41c18316a1ca6e34c178916b09d 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorModuleTest.java 
> 3f045ff38d672266ce2e2bb26f729b0ca4657e81 
> 
> Diff: https://reviews.apache.org/r/38014/diff/
> 
> 
> Testing
> ---
> 
> test suite
> ./gradlew run
> end-to-end tests
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 38169: Refactoring resources table to show dedicated.

2015-09-08 Thread Aurora ReviewBot 

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


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

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

- Aurora ReviewBot


On Sept. 8, 2015, 9:10 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38169/
> ---
> 
> (Updated Sept. 8, 2015, 9:10 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Joshua Cohen.
> 
> 
> Bugs: AURORA-1461
> https://issues.apache.org/jira/browse/AURORA-1461
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Making Resources table dynamic to display only non-zero resource vectors (see 
> pics attached). Table width is now dependent on the number of visible 
> columns. 
> 
> Not particularly happy about ng-class approach to control table width, so any 
> suggestions how to make it better are welcome.
> 
> May not apply cleanly, diffed against 38081.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 98920196db34f2eb4dcad93429274517e7383efe 
>   src/main/resources/scheduler/assets/role.html 
> 2572c4b4497aa78d3fa31457a5dc7c57e6565027 
> 
> Diff: https://reviews.apache.org/r/38169/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> resources_full.png
>   
> https://reviews.apache.org/media/uploaded/files/2015/09/08/ada720ce-bb97-4321-99bb-0574bf14b653__resources_full.png
> resources_partial.png
>   
> https://reviews.apache.org/media/uploaded/files/2015/09/08/a6e4aa7e-ca7e-4ca4-900e-fda6cfdcaff3__resources_partial.png
> resources_min.png
>   
> https://reviews.apache.org/media/uploaded/files/2015/09/08/c44964f2-d8d7-41b0-8a4f-995a1e3538f1__resources_min.png
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 38169: Refactoring resources table to show dedicated.

2015-09-08 Thread David McLaughlin 

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



src/main/resources/scheduler/assets/js/controllers.js (line 183)


Since the max length of 6 assumes a column can fit into 2 bootstrap 
columns, the logic can just be col-md-{columns.length * 2} here.



src/main/resources/scheduler/assets/js/controllers.js (line 206)


Style nit, but declare the function before you call it.


- David McLaughlin


On Sept. 8, 2015, 9:10 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38169/
> ---
> 
> (Updated Sept. 8, 2015, 9:10 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Joshua Cohen.
> 
> 
> Bugs: AURORA-1461
> https://issues.apache.org/jira/browse/AURORA-1461
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Making Resources table dynamic to display only non-zero resource vectors (see 
> pics attached). Table width is now dependent on the number of visible 
> columns. 
> 
> Not particularly happy about ng-class approach to control table width, so any 
> suggestions how to make it better are welcome.
> 
> May not apply cleanly, diffed against 38081.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 98920196db34f2eb4dcad93429274517e7383efe 
>   src/main/resources/scheduler/assets/role.html 
> 2572c4b4497aa78d3fa31457a5dc7c57e6565027 
> 
> Diff: https://reviews.apache.org/r/38169/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> resources_full.png
>   
> https://reviews.apache.org/media/uploaded/files/2015/09/08/ada720ce-bb97-4321-99bb-0574bf14b653__resources_full.png
> resources_partial.png
>   
> https://reviews.apache.org/media/uploaded/files/2015/09/08/a6e4aa7e-ca7e-4ca4-900e-fda6cfdcaff3__resources_partial.png
> resources_min.png
>   
> https://reviews.apache.org/media/uploaded/files/2015/09/08/c44964f2-d8d7-41b0-8a4f-995a1e3538f1__resources_min.png
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 38112: Alter thrift wrapper generator to use default primitive values and empty collections.

2015-09-08 Thread Maxim Khutornenko 


> On Sept. 8, 2015, 10:04 p.m., Maxim Khutornenko wrote:
> > Waiting for the aforementioned rationale write up and related risk analysis 
> > (e.g.: non-stable mutable->immutable->mutable loop).
> 
> Bill Farner wrote:
> Working on that now, but i'm not sure exactly what you're looking for 
> w.r.t. risk analysis other than to make a statement about it.

Not looking for anything fancy. Just calling out the downsides explicitly and 
why you think they are acceptable tradeoffs.


- Maxim


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


On Sept. 3, 2015, 11:38 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38112/
> ---
> 
> (Updated Sept. 3, 2015, 11:38 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1476
> https://issues.apache.org/jira/browse/AURORA-1476
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Alter thrift wrapper generator to use default primitive values and empty 
> collections.
> 
> Reviewers - i will go through and add notes explaining rationale for changes 
> in different areas.  Feel free to wait for me to do that before you review, 
> if you wish.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  a952797315a3421695748f09b9a6abb552cbb668 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 8787aeaa6655cfab1e0a6d5719f9e08a89df7631 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> f1b167bacbfce4f753fc0bbb2b860e3024b9843f 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  e12ad3e3868472ba84e379986bd1aa97bca42ffe 
>   src/main/python/apache/aurora/tools/java/thrift_wrapper_codegen.py 
> b5f2bc9e54b525a6a782d8873c9112f6496cd3f2 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> b2ec13f40c12e5ee5663f4465734d6a80f3587cd 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  f3b62cc957186bc9673060830572bc1cc073ac49 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  5d8bd1b72927786df95c972df64d68c78f25dad0 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
> 08e1284ac1ef08b7649ed83df0a55be04cfeb88f 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java
>  9213b88ab4ce5063ca0fb055851ae5632616155e 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> db60cd21d06d636505202bac7277a13dc24d46e6 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 295974a9f97e020dce11474d500a1bcd40d9f5d5 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> 7ccc273204d20c84bbb576958e832b6f4a29f607 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbAttributeStoreTest.java
>  0e0a847f46c4d1d833a3c610e8a5f752f368c0d5 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java
>  3e78c097a7a9252ded8a4a7fc6609ecf5d61c5b5 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> e0110e7ebe631b7b66b2341cedc10490da00a2ab 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  4d4e752088f7dca99675cc66782ae046bbd516d6 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  4685aa157be77502ad0e4e648ad333ee286f3de5 
> 
> Diff: https://reviews.apache.org/r/38112/diff/
> 
> 
> Testing
> ---
> 
> End-to-end tests pass
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 38014: Remove StartupRegistry.

2015-09-08 Thread Bill Farner 

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

(Updated Sept. 8, 2015, 2:49 p.m.)


Review request for Aurora, Kevin Sweeney and Zameer Manji.


Changes
---

Rebase + ping.


Repository: aurora


Description
---

Remove StartupRegistry.


Diffs (updated)
-

  commons/src/main/java/org/apache/aurora/common/application/AppLauncher.java 
78ed7d0aae50c976eba9045b17587258edc588b0 
  
commons/src/main/java/org/apache/aurora/common/application/StartupRegistry.java 
997ee77dd8f6e02c2becddf98e16cbbbeec4cb4f 
  commons/src/main/java/org/apache/aurora/common/application/StartupStage.java 
80509010f37894108881d40cad9d8ade8610f309 
  
commons/src/main/java/org/apache/aurora/common/application/modules/LifecycleModule.java
 1b6bd086a5a434cb7e4b92ed9cfd252e2518ba66 
  
commons/src/main/java/org/apache/aurora/common/application/modules/StatsModule.java
 3959ce3d688dd50399185925d91f0014fc1c43f9 
  
commons/src/main/java/org/apache/aurora/common/stats/TimeSeriesRepository.java 
6928e48073d152915ca42b6f46236b21c0882086 
  
commons/src/main/java/org/apache/aurora/common/stats/TimeSeriesRepositoryImpl.java
 387e379a9c84d663f2af23e4760754a023219860 
  config/legacy_untested_classes.txt 4bae43a43d00f71456f37f001fcd21ce6a2fb841 
  src/main/java/org/apache/aurora/scheduler/SchedulerServicesModule.java 
1077816b696c4d2e97aafa59900b6acf2adce064 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
6892a70042e25fd672475517325b4e4b69a0adab 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
8047622e206c9827e5cd8e40152a278d495bd0ff 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
b2ec13f40c12e5ee5663f4465734d6a80f3587cd 
  
src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
 4af49d5dcb1925c4055f5ada8601f6fcab5d7d00 
  src/test/java/org/apache/aurora/scheduler/events/PubsubEventModuleTest.java 
cb38c3e047efac483445f43b941c7eea8862cc9c 
  src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
ed8e8119ac3dc41c18316a1ca6e34c178916b09d 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorModuleTest.java 
3f045ff38d672266ce2e2bb26f729b0ca4657e81 

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


Testing
---

test suite
./gradlew run
end-to-end tests


Thanks,

Bill Farner

Re: Review Request 38014: Remove StartupRegistry.

2015-09-08 Thread Bill Farner 

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


@ReviewBot retry

- Bill Farner


On Sept. 8, 2015, 4:21 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38014/
> ---
> 
> (Updated Sept. 8, 2015, 4:21 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove StartupRegistry.
> 
> 
> Diffs
> -
> 
>   commons/src/main/java/org/apache/aurora/common/application/AppLauncher.java 
> 78ed7d0aae50c976eba9045b17587258edc588b0 
>   
> commons/src/main/java/org/apache/aurora/common/application/StartupRegistry.java
>  997ee77dd8f6e02c2becddf98e16cbbbeec4cb4f 
>   
> commons/src/main/java/org/apache/aurora/common/application/StartupStage.java 
> 80509010f37894108881d40cad9d8ade8610f309 
>   
> commons/src/main/java/org/apache/aurora/common/application/modules/LifecycleModule.java
>  1b6bd086a5a434cb7e4b92ed9cfd252e2518ba66 
>   
> commons/src/main/java/org/apache/aurora/common/application/modules/StatsModule.java
>  3959ce3d688dd50399185925d91f0014fc1c43f9 
>   
> commons/src/main/java/org/apache/aurora/common/stats/TimeSeriesRepository.java
>  6928e48073d152915ca42b6f46236b21c0882086 
>   
> commons/src/main/java/org/apache/aurora/common/stats/TimeSeriesRepositoryImpl.java
>  387e379a9c84d663f2af23e4760754a023219860 
>   config/legacy_untested_classes.txt 4bae43a43d00f71456f37f001fcd21ce6a2fb841 
>   src/main/java/org/apache/aurora/scheduler/SchedulerServicesModule.java 
> 1077816b696c4d2e97aafa59900b6acf2adce064 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6892a70042e25fd672475517325b4e4b69a0adab 
>   src/main/java/org/apache/aurora/scheduler/app/LifecycleModule.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 8047622e206c9827e5cd8e40152a278d495bd0ff 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> b2ec13f40c12e5ee5663f4465734d6a80f3587cd 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
>  4af49d5dcb1925c4055f5ada8601f6fcab5d7d00 
>   src/test/java/org/apache/aurora/scheduler/async/AsyncModuleTest.java 
> d0faf13e3f1e14b6e70d230fea0fb484f2105873 
>   src/test/java/org/apache/aurora/scheduler/events/PubsubEventModuleTest.java 
> cb38c3e047efac483445f43b941c7eea8862cc9c 
>   src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
> ed8e8119ac3dc41c18316a1ca6e34c178916b09d 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorModuleTest.java 
> 3f045ff38d672266ce2e2bb26f729b0ca4657e81 
>   src/test/java/org/apache/aurora/scheduler/reconciliation/KillRetryTest.java 
> 2f8fc10278e291fb28011b86bfa7c83905c6307c 
>   src/test/java/org/apache/aurora/scheduler/sla/SlaModuleTest.java 
> 63716459a4e21aa035b683ad46ef0a31620cfd98 
> 
> Diff: https://reviews.apache.org/r/38014/diff/
> 
> 
> Testing
> ---
> 
> test suite
> ./gradlew run
> end-to-end tests
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 38112: Alter thrift wrapper generator to use default primitive values and empty collections.

2015-09-08 Thread Kevin Sweeney 

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

Ship it!


Ship It!

- Kevin Sweeney


On Sept. 8, 2015, 3:47 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38112/
> ---
> 
> (Updated Sept. 8, 2015, 3:47 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1476
> https://issues.apache.org/jira/browse/AURORA-1476
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Alter thrift wrapper generator to use default primitive values and empty 
> collections.
> 
> Reviewers - please see my self-review commentary in specific parts of the 
> patch.  The biggest shift with this change is that an impedance mismatch that 
> existed when inserting/fetching a record from the database is now lifted to 
> the thrift wrapper layer.
> 
> This means the following test is not guaranteed to pass:
> ```java
> TaskConfig original = new TaskConfig(...);
> assertEquals(original, ITaskConfig.build(original).newBuilder());  // won't 
> always pass
> ```
> 
> Most specifically, the wrapped/upwrapped `TaskConfig` will have null 
> collections replaced with empty ones, and will not honor set/unset flags for 
> primitives.  The best practice, therefore, should be to treat our wrapper 
> classes as the canonical form, and only prefer with the underlying thrift 
> types when absolutely necessary.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  a952797315a3421695748f09b9a6abb552cbb668 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 8787aeaa6655cfab1e0a6d5719f9e08a89df7631 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> f1b167bacbfce4f753fc0bbb2b860e3024b9843f 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  e12ad3e3868472ba84e379986bd1aa97bca42ffe 
>   src/main/python/apache/aurora/tools/java/thrift_wrapper_codegen.py 
> b5f2bc9e54b525a6a782d8873c9112f6496cd3f2 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> b2ec13f40c12e5ee5663f4465734d6a80f3587cd 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  f3b62cc957186bc9673060830572bc1cc073ac49 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  5d8bd1b72927786df95c972df64d68c78f25dad0 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
> 08e1284ac1ef08b7649ed83df0a55be04cfeb88f 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java
>  9213b88ab4ce5063ca0fb055851ae5632616155e 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> db60cd21d06d636505202bac7277a13dc24d46e6 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 295974a9f97e020dce11474d500a1bcd40d9f5d5 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> 7ccc273204d20c84bbb576958e832b6f4a29f607 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbAttributeStoreTest.java
>  0e0a847f46c4d1d833a3c610e8a5f752f368c0d5 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java
>  3e78c097a7a9252ded8a4a7fc6609ecf5d61c5b5 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> e0110e7ebe631b7b66b2341cedc10490da00a2ab 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  4d4e752088f7dca99675cc66782ae046bbd516d6 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  4685aa157be77502ad0e4e648ad333ee286f3de5 
> 
> Diff: https://reviews.apache.org/r/38112/diff/
> 
> 
> Testing
> ---
> 
> End-to-end tests pass
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 38014: Remove StartupRegistry.

2015-09-08 Thread Bill Farner 

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

(Updated Sept. 8, 2015, 4:21 p.m.)


Review request for Aurora, Kevin Sweeney and Zameer Manji.


Repository: aurora


Description
---

Remove StartupRegistry.


Diffs (updated)
-

  commons/src/main/java/org/apache/aurora/common/application/AppLauncher.java 
78ed7d0aae50c976eba9045b17587258edc588b0 
  
commons/src/main/java/org/apache/aurora/common/application/StartupRegistry.java 
997ee77dd8f6e02c2becddf98e16cbbbeec4cb4f 
  commons/src/main/java/org/apache/aurora/common/application/StartupStage.java 
80509010f37894108881d40cad9d8ade8610f309 
  
commons/src/main/java/org/apache/aurora/common/application/modules/LifecycleModule.java
 1b6bd086a5a434cb7e4b92ed9cfd252e2518ba66 
  
commons/src/main/java/org/apache/aurora/common/application/modules/StatsModule.java
 3959ce3d688dd50399185925d91f0014fc1c43f9 
  
commons/src/main/java/org/apache/aurora/common/stats/TimeSeriesRepository.java 
6928e48073d152915ca42b6f46236b21c0882086 
  
commons/src/main/java/org/apache/aurora/common/stats/TimeSeriesRepositoryImpl.java
 387e379a9c84d663f2af23e4760754a023219860 
  config/legacy_untested_classes.txt 4bae43a43d00f71456f37f001fcd21ce6a2fb841 
  src/main/java/org/apache/aurora/scheduler/SchedulerServicesModule.java 
1077816b696c4d2e97aafa59900b6acf2adce064 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
6892a70042e25fd672475517325b4e4b69a0adab 
  src/main/java/org/apache/aurora/scheduler/app/LifecycleModule.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
8047622e206c9827e5cd8e40152a278d495bd0ff 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
b2ec13f40c12e5ee5663f4465734d6a80f3587cd 
  
src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
 4af49d5dcb1925c4055f5ada8601f6fcab5d7d00 
  src/test/java/org/apache/aurora/scheduler/async/AsyncModuleTest.java 
d0faf13e3f1e14b6e70d230fea0fb484f2105873 
  src/test/java/org/apache/aurora/scheduler/events/PubsubEventModuleTest.java 
cb38c3e047efac483445f43b941c7eea8862cc9c 
  src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
ed8e8119ac3dc41c18316a1ca6e34c178916b09d 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorModuleTest.java 
3f045ff38d672266ce2e2bb26f729b0ca4657e81 
  src/test/java/org/apache/aurora/scheduler/reconciliation/KillRetryTest.java 
2f8fc10278e291fb28011b86bfa7c83905c6307c 
  src/test/java/org/apache/aurora/scheduler/sla/SlaModuleTest.java 
63716459a4e21aa035b683ad46ef0a31620cfd98 

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


Testing
---

test suite
./gradlew run
end-to-end tests


Thanks,

Bill Farner

Re: Review Request 38169: Refactoring resources table to show dedicated.

2015-09-08 Thread Maxim Khutornenko 

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

(Updated Sept. 8, 2015, 10:54 p.m.)


Review request for Aurora, David McLaughlin and Joshua Cohen.


Changes
---

David's comments.


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


Repository: aurora


Description
---

Making Resources table dynamic to display only non-zero resource vectors (see 
pics attached). Table width is now dependent on the number of visible columns. 

Not particularly happy about ng-class approach to control table width, so any 
suggestions how to make it better are welcome.

May not apply cleanly, diffed against 38081.


Diffs (updated)
-

  src/main/resources/scheduler/assets/js/controllers.js 
98920196db34f2eb4dcad93429274517e7383efe 
  src/main/resources/scheduler/assets/role.html 
2572c4b4497aa78d3fa31457a5dc7c57e6565027 

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


Testing
---


File Attachments


resources_full.png
  
https://reviews.apache.org/media/uploaded/files/2015/09/08/ada720ce-bb97-4321-99bb-0574bf14b653__resources_full.png
resources_partial.png
  
https://reviews.apache.org/media/uploaded/files/2015/09/08/a6e4aa7e-ca7e-4ca4-900e-fda6cfdcaff3__resources_partial.png
resources_min.png
  
https://reviews.apache.org/media/uploaded/files/2015/09/08/c44964f2-d8d7-41b0-8a4f-995a1e3538f1__resources_min.png


Thanks,

Maxim Khutornenko

Re: Review Request 38169: Refactoring resources table to show dedicated.

2015-09-08 Thread Aurora ReviewBot 

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


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

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

- Aurora ReviewBot


On Sept. 8, 2015, 10:54 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38169/
> ---
> 
> (Updated Sept. 8, 2015, 10:54 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Joshua Cohen.
> 
> 
> Bugs: AURORA-1461
> https://issues.apache.org/jira/browse/AURORA-1461
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Making Resources table dynamic to display only non-zero resource vectors (see 
> pics attached). Table width is now dependent on the number of visible 
> columns. 
> 
> Not particularly happy about ng-class approach to control table width, so any 
> suggestions how to make it better are welcome.
> 
> May not apply cleanly, diffed against 38081.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 98920196db34f2eb4dcad93429274517e7383efe 
>   src/main/resources/scheduler/assets/role.html 
> 2572c4b4497aa78d3fa31457a5dc7c57e6565027 
> 
> Diff: https://reviews.apache.org/r/38169/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> resources_full.png
>   
> https://reviews.apache.org/media/uploaded/files/2015/09/08/ada720ce-bb97-4321-99bb-0574bf14b653__resources_full.png
> resources_partial.png
>   
> https://reviews.apache.org/media/uploaded/files/2015/09/08/a6e4aa7e-ca7e-4ca4-900e-fda6cfdcaff3__resources_partial.png
> resources_min.png
>   
> https://reviews.apache.org/media/uploaded/files/2015/09/08/c44964f2-d8d7-41b0-8a4f-995a1e3538f1__resources_min.png
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 38086: Add test environments for RPMs and debs.

2015-09-08 Thread Bill Farner 

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


Ping

- Bill Farner


On Sept. 2, 2015, 7:59 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38086/
> ---
> 
> (Updated Sept. 2, 2015, 7:59 p.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> Add test environments for RPMs and debs.
> 
> 
> Diffs
> -
> 
>   .gitignore 849ddff3b7ec917b5f4563e9a6d3ea63ea512a70 
>   test/deb/ubuntu-trusty/README.md PRE-CREATION 
>   test/deb/ubuntu-trusty/Vagrantfile PRE-CREATION 
>   test/deb/ubuntu-trusty/provision.sh PRE-CREATION 
>   test/rpm/centos-7/README.md PRE-CREATION 
>   test/rpm/centos-7/Vagrantfile PRE-CREATION 
>   test/rpm/centos-7/provision.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38086/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 38014: Remove StartupRegistry.

2015-09-08 Thread Zameer Manji 

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

Ship it!


I'm not very familiar with this code but after a careful review this LGTM.

- Zameer Manji


On Sept. 8, 2015, 4:21 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38014/
> ---
> 
> (Updated Sept. 8, 2015, 4:21 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove StartupRegistry.
> 
> 
> Diffs
> -
> 
>   commons/src/main/java/org/apache/aurora/common/application/AppLauncher.java 
> 78ed7d0aae50c976eba9045b17587258edc588b0 
>   
> commons/src/main/java/org/apache/aurora/common/application/StartupRegistry.java
>  997ee77dd8f6e02c2becddf98e16cbbbeec4cb4f 
>   
> commons/src/main/java/org/apache/aurora/common/application/StartupStage.java 
> 80509010f37894108881d40cad9d8ade8610f309 
>   
> commons/src/main/java/org/apache/aurora/common/application/modules/LifecycleModule.java
>  1b6bd086a5a434cb7e4b92ed9cfd252e2518ba66 
>   
> commons/src/main/java/org/apache/aurora/common/application/modules/StatsModule.java
>  3959ce3d688dd50399185925d91f0014fc1c43f9 
>   
> commons/src/main/java/org/apache/aurora/common/stats/TimeSeriesRepository.java
>  6928e48073d152915ca42b6f46236b21c0882086 
>   
> commons/src/main/java/org/apache/aurora/common/stats/TimeSeriesRepositoryImpl.java
>  387e379a9c84d663f2af23e4760754a023219860 
>   config/legacy_untested_classes.txt 4bae43a43d00f71456f37f001fcd21ce6a2fb841 
>   src/main/java/org/apache/aurora/scheduler/SchedulerServicesModule.java 
> 1077816b696c4d2e97aafa59900b6acf2adce064 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6892a70042e25fd672475517325b4e4b69a0adab 
>   src/main/java/org/apache/aurora/scheduler/app/LifecycleModule.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 8047622e206c9827e5cd8e40152a278d495bd0ff 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> b2ec13f40c12e5ee5663f4465734d6a80f3587cd 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
>  4af49d5dcb1925c4055f5ada8601f6fcab5d7d00 
>   src/test/java/org/apache/aurora/scheduler/async/AsyncModuleTest.java 
> d0faf13e3f1e14b6e70d230fea0fb484f2105873 
>   src/test/java/org/apache/aurora/scheduler/events/PubsubEventModuleTest.java 
> cb38c3e047efac483445f43b941c7eea8862cc9c 
>   src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
> ed8e8119ac3dc41c18316a1ca6e34c178916b09d 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorModuleTest.java 
> 3f045ff38d672266ce2e2bb26f729b0ca4657e81 
>   src/test/java/org/apache/aurora/scheduler/reconciliation/KillRetryTest.java 
> 2f8fc10278e291fb28011b86bfa7c83905c6307c 
>   src/test/java/org/apache/aurora/scheduler/sla/SlaModuleTest.java 
> 63716459a4e21aa035b683ad46ef0a31620cfd98 
> 
> Diff: https://reviews.apache.org/r/38014/diff/
> 
> 
> Testing
> ---
> 
> test suite
> ./gradlew run
> end-to-end tests
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Review Request 38200: Adding notes on changing the scheduler quorum size

2015-09-08 Thread Jeffrey Schroeder 

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

Review request for Aurora.


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


Repository: aurora


Description
---

Adding notes on changing the scheduler quorum size


Diffs
-

  docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 

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


Testing
---

Manually built the documentation using mkdocs and verified the links were 
correct and the TOC rendered.


Thanks,

Jeffrey Schroeder

Re: Review Request 38200: Adding notes on changing the scheduler quorum size

2015-09-08 Thread Aurora ReviewBot 

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

Ship it!


Master (2773826) 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 Sept. 9, 2015, 2:37 a.m., Jeffrey Schroeder wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38200/
> ---
> 
> (Updated Sept. 9, 2015, 2:37 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-1484
> https://issues.apache.org/jira/browse/AURORA-1484
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding notes on changing the scheduler quorum size
> 
> 
> Diffs
> -
> 
>   docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
> 
> Diff: https://reviews.apache.org/r/38200/diff/
> 
> 
> Testing
> ---
> 
> Manually built the documentation using mkdocs and verified the links were 
> correct and the TOC rendered.
> 
> 
> Thanks,
> 
> Jeffrey Schroeder
> 
>

Re: Review Request 38200: Adding notes on changing the scheduler quorum size

2015-09-08 Thread Maxim Khutornenko 

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

Ship it!



docs/deploying-aurora-scheduler.md (line 297)


s/follows/followed


- Maxim Khutornenko


On Sept. 9, 2015, 2:37 a.m., Jeffrey Schroeder wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38200/
> ---
> 
> (Updated Sept. 9, 2015, 2:37 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-1484
> https://issues.apache.org/jira/browse/AURORA-1484
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding notes on changing the scheduler quorum size
> 
> 
> Diffs
> -
> 
>   docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
> 
> Diff: https://reviews.apache.org/r/38200/diff/
> 
> 
> Testing
> ---
> 
> Manually built the documentation using mkdocs and verified the links were 
> correct and the TOC rendered.
> 
> 
> Thanks,
> 
> Jeffrey Schroeder
> 
>

Review Request 38202: Replace BoundedQueue with Guava's EvictingQueue.

2015-09-08 Thread Zameer Manji 

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

Review request for Aurora and Bill Farner.


Repository: aurora


Description
---

Replace BoundedQueue with Guava's EvictingQueue.


Diffs
-

  commons/src/main/java/org/apache/aurora/common/collections/BoundedQueue.java 
3e2d5fa7dd66db4a3c798334c68a92b707660618 
  
commons/src/main/java/org/apache/aurora/common/stats/TimeSeriesRepositoryImpl.java
 387e379a9c84d663f2af23e4760754a023219860 
  
commons/src/test/java/org/apache/aurora/common/collections/BoundedQueueTest.java
 d6e3c211542e14406785ff200076f77ac1dc4823 

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


Testing
---

./gradlew build -Pq


Thanks,

Zameer Manji

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-08 Thread Bill Farner 


> On Sept. 2, 2015, 9:45 a.m., Bill Farner wrote:
> > examples/vagrant/executors-config.json, lines 4-7
> > 
> >
> > The code later converts this array into a single command string.  I 
> > suggest we just make this a string here (and save some code down the line).
> 
> Renan DelValle wrote:
> We can go ahead and do that. Kevin thought it should be the array way, 
> but I'm not sure how strongly he feels about it. The extra code isn't too bad 
> because we can use String.join() form Java 8. Either way, I'm fine with 
> either. The single string may eliminate any suspicions that we're breaking 
> something in a command when joining the array.
> 
> Kevin Sweeney wrote:
> Bill, I prefer the array mode as it's more explicit for dealing with 
> magic characters in shell commands (like " (){}$").
> 
> You can still explicitly do
> 
> ```
> ["sh", "-c", "PATH=/opt/aurora/executor/bin:$PATH thermos"]
> ```
> 
> When you prefer the old form, but this form will not automatically expand 
> environment variables or do other shell magic for you.
> 
> Renan - can you update CommandUtil to use the `arguments`[1] field 
> instead of `shell` and `value` (or drop a TODO for a later patch)? Changing 
> back to `String.join` defeats part of the purpose of the array form here.
> 
> [1] 
> https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L408-L423
> 
> Bill Farner wrote:
> Aha - the `arguments` field in mesos.proto was the missing detail for me. 
>  Since mesos allows us to do the right thing here, we should definitely use 
> an array.
> 
> Renan DelValle wrote:
> I gave this a shot, and it works fine for no container tasks. But, when 
> running a Docker container job, since the $MESOS_SANDBOX string is no longer 
> automatically expanding, it breaks. Should we just create a new CommandInfo 
> builder for docker that sets "sh" as the value and "-c" as the first argument 
> or is this a bad idea?

Kevin - any opinion on this?


- Bill


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


On Sept. 8, 2015, 9:32 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37818/
> ---
> 
> (Updated Sept. 8, 2015, 9:32 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is the first stage in a series of patches to create support for custom 
> executors. In an effort to expedite the review process, I have decided to 
> break down my patch into multiple pieces that when/if commited won't break 
> the trunk.
> 
> This patch includes the ability to load configuration from a JSON file. A 
> JSON example file is included in examples/vagrant/executors-config.json
> 
> Command line arguments have been eliminated and moved over to the JSON file. 
> GSON is leveraged and does most of the work with the aid of a few custom 
> deserializers that were needed. 
> 
> Note that right now a global container mount that does not follow 
> specification will cause the scheduler to detect the error an exit early. It 
> is up for discussion if this is the desired behavior or if we should just 
> ignore said mount.
> 
> 
> Diffs
> -
> 
>   examples/vagrant/executors-config.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 4f43892723db4744db205ea7dd107e9e9ce9d5db 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> e909451892f117e9e6eb80994079661827a0914c 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> c210c0db07bb1f4b3f76668178dcd7e2de56a4ac 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 197184b6edc0768d677636341b5737f262abdf7d 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 8047622e206c9827e5cd8e40152a278d495bd0ff 
>   src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java 
> aa5ce8b2f14c7dbd0eae120018ee41387c26059f 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> b3c913892248e4a9a8111412307463985f5ca97f 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> f6ba2c40aea555d3e0ab774218bfe08d7e1c984b 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 6fad3344042dc6a75cdf74ce79d388fcd4fc9861 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 1a25924d789295c5950947f5e302e1d1fbec68f2 
>

Re: Review Request 38081: Dropping quota check for dedicated jobs and exposing dedicated consumption.

2015-09-08 Thread Aurora ReviewBot 

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

Ship it!


Master (2773826) 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 Sept. 8, 2015, 4:53 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38081/
> ---
> 
> (Updated Sept. 8, 2015, 4:53 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1460
> https://issues.apache.org/jira/browse/AURORA-1460
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This change drops quota enforcement for dedicated jobs and adds dedicated 
> resource consumption into the thrift API. There is now a shared/dedicated 
> consumption split for both prod and non-prod tasks.
> 
> NOTE: the GetQuotaResult will have to be deprecated and/or replaced with a 
> tier-based mapping when AURORA-1443 is addressed.
> 
> 
> Diffs
> -
> 
>   NEWS 83a1213e2ea4ab6b3706cd70c015ba9f16735520 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 21137bbbdc3010c6b1e2cc0ebb3b99bfa8490563 
>   src/main/java/org/apache/aurora/scheduler/ResourceAggregates.java 
> df796fcc91265c588872051f16bd2f10f73dc718 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaInfo.java 
> b40ecd0af7c1d1bb9372bd89c741622ce4c9040c 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 8787aeaa6655cfab1e0a6d5719f9e08a89df7631 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> 3d89e43659750de63d7588f8574e7a350caea04b 
>   src/main/python/apache/aurora/client/api/quota_check.py 
> 75406ac1d3ec9ba655daf7c6125f208f74290cfb 
>   src/main/python/apache/aurora/client/cli/quota.py 
> e8aa010f5ecce0464a8ad6b072bccba589fe16d7 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 98920196db34f2eb4dcad93429274517e7383efe 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> db60cd21d06d636505202bac7277a13dc24d46e6 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
>  a8df44612b605dff512d1ca686f0789b84ef9012 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> e0110e7ebe631b7b66b2341cedc10490da00a2ab 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  4d4e752088f7dca99675cc66782ae046bbd516d6 
>   src/test/python/apache/aurora/admin/test_admin.py 
> d793293acc7f77c0081968334c38b984d865fbc8 
>   src/test/python/apache/aurora/client/api/test_quota_check.py 
> 6c9bc373d1868bd6e0cae2f2218261f39fd7ab8f 
>   src/test/python/apache/aurora/client/cli/test_quota.py 
> 3573e4c4575577e2232d2b99ca781b06a03d48d7 
> 
> Diff: https://reviews.apache.org/r/38081/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 38081: Dropping quota check for dedicated jobs and exposing dedicated consumption.

2015-09-08 Thread Maxim Khutornenko 


> On Sept. 8, 2015, 3:55 p.m., Bill Farner wrote:
> > Looks good!  Holding back the ship until a `NEWS` entry is included in the 
> > patch.

Thanks for reminding! Added.


> On Sept. 8, 2015, 3:55 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java,
> >  line 313
> > 
> >
> > All the fields are checked here, how about a check on the 
> > `GetQuotaResult` itself?
> > 
> > ```
> > assertEquals(expectedResult, actualResult);
> > ```

Done.


- Maxim


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


On Sept. 7, 2015, 11:51 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38081/
> ---
> 
> (Updated Sept. 7, 2015, 11:51 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Bugs: AURORA-1460
> https://issues.apache.org/jira/browse/AURORA-1460
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This change drops quota enforcement for dedicated jobs and adds dedicated 
> resource consumption into the thrift API. There is now a shared/dedicated 
> consumption split for both prod and non-prod tasks.
> 
> NOTE: the GetQuotaResult will have to be deprecated and/or replaced with a 
> tier-based mapping when AURORA-1443 is addressed.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 21137bbbdc3010c6b1e2cc0ebb3b99bfa8490563 
>   src/main/java/org/apache/aurora/scheduler/ResourceAggregates.java 
> df796fcc91265c588872051f16bd2f10f73dc718 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaInfo.java 
> b40ecd0af7c1d1bb9372bd89c741622ce4c9040c 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 8787aeaa6655cfab1e0a6d5719f9e08a89df7631 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> 3d89e43659750de63d7588f8574e7a350caea04b 
>   src/main/python/apache/aurora/client/api/quota_check.py 
> 75406ac1d3ec9ba655daf7c6125f208f74290cfb 
>   src/main/python/apache/aurora/client/cli/quota.py 
> e8aa010f5ecce0464a8ad6b072bccba589fe16d7 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 98920196db34f2eb4dcad93429274517e7383efe 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> db60cd21d06d636505202bac7277a13dc24d46e6 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
>  a8df44612b605dff512d1ca686f0789b84ef9012 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> e0110e7ebe631b7b66b2341cedc10490da00a2ab 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  4d4e752088f7dca99675cc66782ae046bbd516d6 
>   src/test/python/apache/aurora/admin/test_admin.py 
> d793293acc7f77c0081968334c38b984d865fbc8 
>   src/test/python/apache/aurora/client/api/test_quota_check.py 
> 6c9bc373d1868bd6e0cae2f2218261f39fd7ab8f 
>   src/test/python/apache/aurora/client/cli/test_quota.py 
> 3573e4c4575577e2232d2b99ca781b06a03d48d7 
> 
> Diff: https://reviews.apache.org/r/38081/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 38169: Refactoring resources table to show dedicated.

2015-09-08 Thread Joshua Cohen 

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



src/main/resources/scheduler/assets/js/controllers.js (line 178)


A nit really, but I always feel like the safety of calling `hasOwnProperty` 
is overkill when we know ahead of time that there's no inheritance and we're 
not going to be using a name that will conflict with something higher up the 
prototype chain. The cleaner/simpler:

var vector = consumption[column.map];
return !vector || (vector.numCpus > 0 || ...);



src/main/resources/scheduler/assets/js/controllers.js (lines 198 - 205)


The repetition between the CPU, RAM, and Disk sections is enhanced by the 
addition of the additional types of consumption. Can you extract a function to 
reduce this?



src/main/resources/scheduler/assets/role.html (lines 35 - 36)


I'd do this in the controller instead of the view. Add something like:

$scope.resourcesClass = ...;

Then in the view just do:



I don't *think* you need/want to use ng-class, since the value isn't going 
to change after render.


- Joshua Cohen


On Sept. 8, 2015, 3:14 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38169/
> ---
> 
> (Updated Sept. 8, 2015, 3:14 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Joshua Cohen.
> 
> 
> Bugs: AURORA-1461
> https://issues.apache.org/jira/browse/AURORA-1461
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Making Resources table dynamic to display only non-zero resource vectors (see 
> pics attached). Table width is now dependent on the number of visible 
> columns. 
> 
> Not particularly happy about ng-class approach to control table width, so any 
> suggestions how to make it better are welcome.
> 
> May not apply cleanly, diffed against 38081.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 98920196db34f2eb4dcad93429274517e7383efe 
>   src/main/resources/scheduler/assets/role.html 
> 2572c4b4497aa78d3fa31457a5dc7c57e6565027 
> 
> Diff: https://reviews.apache.org/r/38169/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> resources_full.png
>   
> https://reviews.apache.org/media/uploaded/files/2015/09/08/ada720ce-bb97-4321-99bb-0574bf14b653__resources_full.png
> resources_partial.png
>   
> https://reviews.apache.org/media/uploaded/files/2015/09/08/a6e4aa7e-ca7e-4ca4-900e-fda6cfdcaff3__resources_partial.png
> resources_min.png
>   
> https://reviews.apache.org/media/uploaded/files/2015/09/08/c44964f2-d8d7-41b0-8a4f-995a1e3538f1__resources_min.png
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 38202: Replace BoundedQueue with Guava's EvictingQueue.

2015-09-08 Thread Aurora ReviewBot 

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


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

 > self.update_and_expect_ok()
 
 
src/test/python/apache/aurora/client/api/test_updater.py:905: 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
src/test/python/apache/aurora/client/api/test_updater.py:335: in 
update_and_expect_ok
 self.update_and_expect_response(ResponseCode.OK, 
instances)
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
 self = 
 expected_code = 1, instances = None
 message = None
 
 def update_and_expect_response(self, 
expected_code, instances=None, message=None):
   resp = self._updater.update(instances)
 > assert expected_code == resp.responseCode, (
 'Expected response:%s Actual response:%s' % 
(expected_code, resp.responseCode))
 E AssertionError: Expected response:1 Actual 
response:2
 E assert 1 == 2
 E  +  where 2 = 
Response(details=[ResponseDetail(message='Aborting update without rollback! 
Fa...obMonitor.terminate() -> None')], result=None, responseCode=2, 
serverInfo=None).responseCode
 
 
src/test/python/apache/aurora/client/api/test_updater.py:339: AssertionError
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/src.test.python.apache.aurora.client.api.updater.xml
 
 == 1 failed, 31 passed in 1.07 seconds 
===
 src.test.python.apache.aurora.client.api.updater   
 .   FAILURE
 src.test.python.apache.aurora.client.cli.config
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.cron  
 .   SUCCESS
 src.test.python.apache.aurora.client.hooks.non_hooked_api  
 .   SUCCESS
 src.test.python.apache.aurora.common.test_http_signaler
 .   SUCCESS
 
src.test.python.apache.aurora.executor.common.path_detector 
.   SUCCESS
 src.test.python.apache.aurora.executor.common.task_info
 .   SUCCESS
 src.test.python.apache.thermos.cli.commands.commands   
 .   SUCCESS
 src.test.python.apache.thermos.cli.common  
 .   SUCCESS
 src.test.python.apache.thermos.cli.main
 .   SUCCESS
 src.test.python.apache.thermos.common.test_pathspec
 .   SUCCESS
 src.test.python.apache.thermos.monitoring.test_disk
 .   SUCCESS
 
FAILURE


   FAILURE


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

- Aurora ReviewBot


On Sept. 9, 2015, 3:33 a.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38202/
> ---
> 
> (Updated Sept. 9, 2015, 3:33 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Replace BoundedQueue with Guava's EvictingQueue.
> 
> 
> Diffs
> -
> 
>   
> commons/src/main/java/org/apache/aurora/common/collections/BoundedQueue.java 
> 3e2d5fa7dd66db4a3c798334c68a92b707660618 
>   
> commons/src/main/java/org/apache/aurora/common/stats/TimeSeriesRepositoryImpl.java
>  387e379a9c84d663f2af23e4760754a023219860 
>   
> commons/src/test/java/org/apache/aurora/common/collections/BoundedQueueTest.java
>  d6e3c211542e14406785ff200076f77ac1dc4823 
> 
> Diff: https://reviews.apache.org/r/38202/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>