date:20150903

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

2015-09-03 Thread Maxim Khutornenko



> On Sept. 3, 2015, 1:27 a.m., Suman Karumuri wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 369
> > 
> >
> > I think it will be better if we split this into 
> > nonProdSharedConsumption and nonProdDedicatedConsumption.
> 
> Maxim Khutornenko wrote:
> This may be an option to consider in future but it does not fit the scope 
> of this change.
> 
> Suman Karumuri wrote:
> Is there a strong reason, not to split the non prod consumption into 
> shared and dedicated pools since we already do it for prod jobs? I think we 
> should expose this info now when we are making the split between dedicated 
> and shared pool. Delaying it will involve a lot more effort later. If we 
> choose not to expose this infortion separately in the UI, the UI can add up 
> nonProdDedicatedConsumption and nonProdSharedPoolConsumption into the same 
> field. But going the other way around would be a lot more work. 
> 
> I also feel we should decide on the UI design now and implement the API 
> accordingly.

| Is there a strong reason, not to split the non prod consumption into shared 
and dedicated pools since we already do it for prod jobs?

I just don't want to increase the size of this diff and add changes irrelevant 
to the subject matter. I am open to consider adding it separately.

Also, the approach of adding prod/non-prod dedicated consumption does not scale 
given our upcoming migration to task tiers. There is a good chance this 
structure will have to be dropped entirely in favor of something like this 
(just to illustrate the direction):
```
struct TierResources {
  1: string taskTier
  2: ResourceAggregate quota
  3: ResourceAggregate consumption
}

struct GetQuotaResultNew {
  1: map resourcesByTier
}
```

In the above example, a "non-prod dedicated" could be a completely separate 
tier (though does not have to be). This is the reason I was hesistant adding 
prod/nonprod consumption split in the first place, hence my comment in the 
description. I wonder what others think?

| But going the other way around would be a lot more work. I also feel we 
should decide on the UI design now and implement the API accordingly.

I don't buy this argument. There is nothing preventing us from implementing the 
UI the way we want and iterate accordingly.


- Maxim


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


On Sept. 3, 2015, 12:46 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38081/
> ---
> 
> (Updated Sept. 3, 2015, 12:46 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. Feel free to pushback on the latter 
> but I think users will find it immediately confusing when we change the "Prod 
> consumption" to something like "Prod shared pool consumption" in the 
> UI/client and not provide the missing dedicated rollup numbers.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 21137bbbdc3010c6b1e2cc0ebb3b99bfa8490563 
>   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/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
>

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

2015-09-03 Thread Zameer Manji


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

Ship it!


I think there should be a `NEWS` file entry about this change and documentation 
updates. I'm comfortable with documentation changes being in another review but 
I think this commit should also update the `NEWS` file.


src/main/java/org/apache/aurora/scheduler/quota/QuotaInfo.java (line 97)


I think you need to `prodDedicatedConsumption` to the set of hashed fields.


- Zameer Manji


On Sept. 2, 2015, 5:46 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38081/
> ---
> 
> (Updated Sept. 2, 2015, 5:46 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. Feel free to pushback on the latter 
> but I think users will find it immediately confusing when we change the "Prod 
> consumption" to something like "Prod shared pool consumption" in the 
> UI/client and not provide the missing dedicated rollup numbers.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 21137bbbdc3010c6b1e2cc0ebb3b99bfa8490563 
>   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/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
> 
>

Review Request 38106: Make it possible to link directly to individual tabs on the job page.

2015-09-03 Thread Joshua Cohen


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

Review request for Aurora, David McLaughlin and Zameer Manji.


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


Repository: aurora


Description
---

Make it possible to link directly to individual tabs on the job page.


Diffs
-

  src/main/resources/scheduler/assets/job.html 
bfe51ab71e443615510d60915a28109070ba3501 
  src/main/resources/scheduler/assets/js/app.js 
310aa35c2ede799de693b86012e943fadfdf03f5 
  src/main/resources/scheduler/assets/js/controllers.js 
98920196db34f2eb4dcad93429274517e7383efe 

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


Testing
---


Thanks,

Joshua Cohen

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

2015-09-03 Thread Suman Karumuri



> On Sept. 3, 2015, 1:27 a.m., Suman Karumuri wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 369
> > 
> >
> > I think it will be better if we split this into 
> > nonProdSharedConsumption and nonProdDedicatedConsumption.
> 
> Maxim Khutornenko wrote:
> This may be an option to consider in future but it does not fit the scope 
> of this change.
> 
> Suman Karumuri wrote:
> Is there a strong reason, not to split the non prod consumption into 
> shared and dedicated pools since we already do it for prod jobs? I think we 
> should expose this info now when we are making the split between dedicated 
> and shared pool. Delaying it will involve a lot more effort later. If we 
> choose not to expose this infortion separately in the UI, the UI can add up 
> nonProdDedicatedConsumption and nonProdSharedPoolConsumption into the same 
> field. But going the other way around would be a lot more work. 
> 
> I also feel we should decide on the UI design now and implement the API 
> accordingly.
> 
> Maxim Khutornenko wrote:
> | Is there a strong reason, not to split the non prod consumption into 
> shared and dedicated pools since we already do it for prod jobs?
> 
> I just don't want to increase the size of this diff and add changes 
> irrelevant to the subject matter. I am open to consider adding it separately.
> 
> Also, the approach of adding prod/non-prod dedicated consumption does not 
> scale given our upcoming migration to task tiers. There is a good chance this 
> structure will have to be dropped entirely in favor of something like this 
> (just to illustrate the direction):
> ```
> struct TierResources {
>   1: string taskTier
>   2: ResourceAggregate quota
>   3: ResourceAggregate consumption
> }
> 
> struct GetQuotaResultNew {
>   1: map resourcesByTier
> }
> ```
> 
> In the above example, a "non-prod dedicated" could be a completely 
> separate tier (though does not have to be). This is the reason I was 
> hesistant adding prod/nonprod consumption split in the first place, hence my 
> comment in the description. I wonder what others think?
> 
> | But going the other way around would be a lot more work. I also feel we 
> should decide on the UI design now and implement the API accordingly.
> 
> I don't buy this argument. There is nothing preventing us from 
> implementing the UI the way we want and iterate accordingly.
> 
> Zameer Manji wrote:
> Maxim,
> 
> Is this possible change tracked apart of the tier work? If not, I really 
> think a ticket for this should be created so we don't forget about this.

Thanks for the explanation. I like the TieredResources struct approach. As a 
service owner though, I would like to see non-prod dedicated consumption and 
prod dedicated consumption. That would clearly tell me the amount of resources 
I am using for prod and non-prod.

+1 for tracking this in a ticket as part of task tiers implementation.


- Suman


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


On Sept. 3, 2015, 12:46 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38081/
> ---
> 
> (Updated Sept. 3, 2015, 12:46 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. Feel free to pushback on the latter 
> but I think users will find it immediately confusing when we change the "Prod 
> consumption" to something like "Prod shared pool consumption" in the 
> UI/client and not provide the missing dedicated rollup numbers.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 21137bbbdc3010c6b1e2cc0ebb3b99bfa8490563 
>   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 
>

Re: Review Request 38106: Make it possible to link directly to individual tabs on the job page.

2015-09-03 Thread Aurora ReviewBot


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


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

However, it appears that it might lack test coverage.

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

- Aurora ReviewBot


On Sept. 3, 2015, 7:14 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38106/
> ---
> 
> (Updated Sept. 3, 2015, 7:14 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: AURORA-696
> https://issues.apache.org/jira/browse/AURORA-696
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Make it possible to link directly to individual tabs on the job page.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/job.html 
> bfe51ab71e443615510d60915a28109070ba3501 
>   src/main/resources/scheduler/assets/js/app.js 
> 310aa35c2ede799de693b86012e943fadfdf03f5 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 98920196db34f2eb4dcad93429274517e7383efe 
> 
> Diff: https://reviews.apache.org/r/38106/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

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

2015-09-03 Thread Suman Karumuri



> On Sept. 3, 2015, 1:27 a.m., Suman Karumuri wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 369
> > 
> >
> > I think it will be better if we split this into 
> > nonProdSharedConsumption and nonProdDedicatedConsumption.
> 
> Maxim Khutornenko wrote:
> This may be an option to consider in future but it does not fit the scope 
> of this change.

Is there a strong reason, not to split the non prod consumption into shared and 
dedicated pools since we already do it for prod jobs? I think we should expose 
this info now when we are making the split between dedicated and shared pool. 
Delaying it will involve a lot more effort later. If we choose not to expose 
this infortion separately in the UI, the UI can add up 
nonProdDedicatedConsumption and nonProdSharedPoolConsumption into the same 
field. But going the other way around would be a lot more work. 

I also feel we should decide on the UI design now and implement the API 
accordingly.


- Suman


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


On Sept. 3, 2015, 12:46 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38081/
> ---
> 
> (Updated Sept. 3, 2015, 12:46 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. Feel free to pushback on the latter 
> but I think users will find it immediately confusing when we change the "Prod 
> consumption" to something like "Prod shared pool consumption" in the 
> UI/client and not provide the missing dedicated rollup numbers.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 21137bbbdc3010c6b1e2cc0ebb3b99bfa8490563 
>   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/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-03 Thread Maxim Khutornenko



> On Sept. 3, 2015, 1:27 a.m., Suman Karumuri wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 369
> > 
> >
> > I think it will be better if we split this into 
> > nonProdSharedConsumption and nonProdDedicatedConsumption.

This may be an option to consider in future but it does not fit the scope of 
this change.


> On Sept. 3, 2015, 1:27 a.m., Suman Karumuri wrote:
> > src/main/resources/scheduler/assets/js/controllers.js, line 161
> > 
> >
> > I think we should see if the role has jobs running on dedicated boxes. 
> > If it does, show nonProdDedicatedConsumption and prodDedicatedConsumption 
> > else hide it.
> > 
> > If we expose prod and non prod dedicated consumption, we also have to 
> > rename the current quota titles to Production shared consumption or 
> > something similar. 
> > 
> > I prefer not exposing the dedicated aspect to jobs running in shared 
> > pool since users may take it as an encouagement to run on dedicated boxes. 
> > For jobs running in shared pool, we don't have to change the terminology.
> > 
> > It may be simpler to just implement as 2 different tables one for 
> > shared pool jobs and one for jobs involving mixed workloads.

I would defer this discussion to AURORA-1461. This touches UI only to reflect 
field renaming.


- Maxim


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


On Sept. 3, 2015, 12:46 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38081/
> ---
> 
> (Updated Sept. 3, 2015, 12:46 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. Feel free to pushback on the latter 
> but I think users will find it immediately confusing when we change the "Prod 
> consumption" to something like "Prod shared pool consumption" in the 
> UI/client and not provide the missing dedicated rollup numbers.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 21137bbbdc3010c6b1e2cc0ebb3b99bfa8490563 
>   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/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-03 Thread Joshua Cohen


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

Ship it!


Ship It!

- Joshua Cohen


On Sept. 3, 2015, 12:46 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38081/
> ---
> 
> (Updated Sept. 3, 2015, 12:46 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. Feel free to pushback on the latter 
> but I think users will find it immediately confusing when we change the "Prod 
> consumption" to something like "Prod shared pool consumption" in the 
> UI/client and not provide the missing dedicated rollup numbers.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 21137bbbdc3010c6b1e2cc0ebb3b99bfa8490563 
>   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/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 38106: Make it possible to link directly to individual tabs on the job page.

2015-09-03 Thread David McLaughlin


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

Ship it!


Ship It!

- David McLaughlin


On Sept. 3, 2015, 7:14 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38106/
> ---
> 
> (Updated Sept. 3, 2015, 7:14 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: AURORA-696
> https://issues.apache.org/jira/browse/AURORA-696
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Make it possible to link directly to individual tabs on the job page.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/job.html 
> bfe51ab71e443615510d60915a28109070ba3501 
>   src/main/resources/scheduler/assets/js/app.js 
> 310aa35c2ede799de693b86012e943fadfdf03f5 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 98920196db34f2eb4dcad93429274517e7383efe 
> 
> Diff: https://reviews.apache.org/r/38106/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Review Request 38111: Disable mimetype guessing in the observer chroot browser.

2015-09-03 Thread Zameer Manji


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

Review request for Aurora and Joshua Cohen.


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


Repository: aurora


Description
---

This disables the mimetype guessing of files served by the chroot browser and 
sets it to the standard `application/octet-stream` mimetype. This prevents 
browsers from trying to decompress gzipped files or other surprising behaviour 
for users.


Diffs
-

  src/main/python/apache/thermos/observer/http/file_browser.py 
1750f5bd0937f8ce411f976db488bf5d14930577 

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


Testing
---

./pants test src/test/python/apache/thermos/observer::
Manual inspection of Content-Type header via vagrant.


Thanks,

Zameer Manji

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

2015-09-03 Thread Renan DelValle



> On Sept. 2, 2015, 4:45 p.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.

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?


- Renan


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


On Sept. 2, 2015, 2:35 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37818/
> ---
> 
> (Updated Sept. 2, 2015, 2:35 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> 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 
>   src/test/java/org/apache/aurora/scheduler/base/CommandUtilTest.java 
> cd0295780d41bc4e914583f195b37eaed28a46dc 
>   
>

Re: Review Request 38014: Remove StartupRegistry.

2015-09-03 Thread Bill Farner


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

(Updated Sept. 3, 2015, 4:38 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/SchedulerMain.java 
8047622e206c9827e5cd8e40152a278d495bd0ff 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
1a25924d789295c5950947f5e302e1d1fbec68f2 
  
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 38112: Alter thrift wrapper generator to use default primitive values and empty collections.

2015-09-03 Thread Bill Farner


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

(Updated Sept. 3, 2015, 4: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 (updated)
---

End-to-end tests pass


Thanks,

Bill Farner

Re: Review Request 38014: Remove StartupRegistry.

2015-09-03 Thread Aurora ReviewBot


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


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

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

- Aurora ReviewBot


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/38014/
> ---
> 
> (Updated Sept. 3, 2015, 11:38 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 
> 1a25924d789295c5950947f5e302e1d1fbec68f2 
>   
> 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 38111: Disable mimetype guessing in the observer chroot browser.

2015-09-03 Thread Aurora ReviewBot


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


Master (9c0b1b2) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

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

- Aurora ReviewBot


On Sept. 3, 2015, 10:15 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38111/
> ---
> 
> (Updated Sept. 3, 2015, 10:15 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Bugs: AURORA-1479
> https://issues.apache.org/jira/browse/AURORA-1479
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This disables the mimetype guessing of files served by the chroot browser and 
> sets it to the standard `application/octet-stream` mimetype. This prevents 
> browsers from trying to decompress gzipped files or other surprising 
> behaviour for users.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/observer/http/file_browser.py 
> 1750f5bd0937f8ce411f976db488bf5d14930577 
> 
> Diff: https://reviews.apache.org/r/38111/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/observer::
> Manual inspection of Content-Type header via vagrant.
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 38111: Disable mimetype guessing in the observer chroot browser.

2015-09-03 Thread Zameer Manji



> On Sept. 3, 2015, 4:03 p.m., Kevin Sweeney wrote:
> > Are you sure this works? AIUI it's Transfer-Encoding: gzip that we need to 
> > worry about, not content-encoding (which is part of the entity being 
> > served). Most web frameworks have a facility whereby they will locate 
> > pre-compressed assets, usually with a .gz suffix.
> > 
> > For example, if a directory contains `a.js` and `a.js.gz`, a client's 
> > request for a.js with the header `'accept-encoding: gzip'` will result in 
> > the server responding with the contents of `a.js.gz`, and the headers 
> > `transfer-encoding: gzip` and `content-type: application/javascript`. The 
> > client will generally decompress the result on the fly. OTOH, a request for 
> > `a.js.gz` should result in the unmodified file with `content-encoding: 
> > application/gzip`. See this stackoverflow post: [1]
> > 
> > [1] 
> > http://stackoverflow.com/questions/11641923/transfer-encoding-gzip-vs-content-encoding-gzip

Bottle doesn't support setting `Transfer-Encoding` it only sets 
`Content-Encoding`. If we explicitly specify a `mimetype` argument it doesn't 
[guess](https://github.com/bottlepy/bottle/blob/534a2e08ac0ef55dd542a3a83b1118188c6a399b/bottle.py#L2518)
 resulting in the desired behaviour. Regardless I think we should serve all 
data as `application/octet-stream` to signal that it is opaque data that should 
not be handled by the browser.


- Zameer


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


On Sept. 3, 2015, 3:15 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38111/
> ---
> 
> (Updated Sept. 3, 2015, 3:15 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Bugs: AURORA-1479
> https://issues.apache.org/jira/browse/AURORA-1479
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This disables the mimetype guessing of files served by the chroot browser and 
> sets it to the standard `application/octet-stream` mimetype. This prevents 
> browsers from trying to decompress gzipped files or other surprising 
> behaviour for users.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/observer/http/file_browser.py 
> 1750f5bd0937f8ce411f976db488bf5d14930577 
> 
> Diff: https://reviews.apache.org/r/38111/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/observer::
> Manual inspection of Content-Type header via vagrant.
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

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

2015-09-03 Thread Aurora ReviewBot


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

Ship it!


Master (9c0b1b2) 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. 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 38111: Disable mimetype guessing in the observer chroot browser.

2015-09-03 Thread Kevin Sweeney

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

Are you sure this works? AIUI it's Transfer-Encoding: gzip that we need to 
worry about, not content-encoding (which is part of the entity being served). 
Most web frameworks have a facility whereby they will locate pre-compressed 
assets, usually with a .gz suffix.

For example, if a directory contains `a.js` and `a.js.gz`, a client's request 
for a.js with the header `'accept-encoding: gzip'` will result in the server 
responding with the contents of `a.js.gz`, and the headers `transfer-encoding: 
gzip` and `content-type: application/javascript`. The client will generally 
decompress the result on the fly. OTOH, a request for `a.js.gz` should result 
in the unmodified file with `content-encoding: application/gzip`. See this 
stackoverflow post: [1]

[1] 
http://stackoverflow.com/questions/11641923/transfer-encoding-gzip-vs-content-encoding-gzip

- Kevin Sweeney

On Sept. 3, 2015, 3:15 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38111/
> ---
> 
> (Updated Sept. 3, 2015, 3:15 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Bugs: AURORA-1479
> https://issues.apache.org/jira/browse/AURORA-1479
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This disables the mimetype guessing of files served by the chroot browser and 
> sets it to the standard `application/octet-stream` mimetype. This prevents 
> browsers from trying to decompress gzipped files or other surprising 
> behaviour for users.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/observer/http/file_browser.py 
> 1750f5bd0937f8ce411f976db488bf5d14930577 
> 
> Diff: https://reviews.apache.org/r/38111/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/observer::
> Manual inspection of Content-Type header via vagrant.
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 38106: Make it possible to link directly to individual tabs on the job page.

2015-09-03 Thread Zameer Manji


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

Ship it!


Ship It!

- Zameer Manji


On Sept. 3, 2015, 12:14 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38106/
> ---
> 
> (Updated Sept. 3, 2015, 12:14 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: AURORA-696
> https://issues.apache.org/jira/browse/AURORA-696
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Make it possible to link directly to individual tabs on the job page.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/job.html 
> bfe51ab71e443615510d60915a28109070ba3501 
>   src/main/resources/scheduler/assets/js/app.js 
> 310aa35c2ede799de693b86012e943fadfdf03f5 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 98920196db34f2eb4dcad93429274517e7383efe 
> 
> Diff: https://reviews.apache.org/r/38106/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

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

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

Review Request 38106: Make it possible to link directly to individual tabs on the job page.

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

Re: Review Request 38106: Make it possible to link directly to individual tabs on the job page.

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

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

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

Re: Review Request 38106: Make it possible to link directly to individual tabs on the job page.

Review Request 38111: Disable mimetype guessing in the observer chroot browser.

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

Re: Review Request 38014: Remove StartupRegistry.

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

Re: Review Request 38014: Remove StartupRegistry.

Re: Review Request 38111: Disable mimetype guessing in the observer chroot browser.

Re: Review Request 38111: Disable mimetype guessing in the observer chroot browser.

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

Re: Review Request 38111: Disable mimetype guessing in the observer chroot browser.

Re: Review Request 38106: Make it possible to link directly to individual tabs on the job page.

19 matches

Site Navigation

Mail list logo

Footer information