Re: Review Request 46795: Fixing e2e tests.

2016-04-28 Thread Maxim Khutornenko 


> On April 28, 2016, 6:31 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/migration/V005_CreateQuotaResourceTable.java,
> >  line 39
> > 
> >
> > What is value in the case of ports? I thought it would be the port 
> > name? In which case this constraint should still be valid?

Ports are not supported in ResourceAggregates, hence the ThriftBackfill changes 
in this patch to only allow CPUS, RAM and disk. This constraint was too 
permissive as it would allow something like 'CPUS, 1.0' and 'CPUS, 2.0', which 
is no longer accepted.


- Maxim


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


On April 28, 2016, 6:14 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46795/
> ---
> 
> (Updated April 28, 2016, 6:14 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Got too greedy converting pairs to maps during the last round of 
> https://reviews.apache.org/r/46716. Task resources must be handled via a list 
> of Pairs as there could be duplicate resource types there (e.g. ports).
> 
> Also, added more validation into ResourceAggregate backfilling to account for 
> invalid/missing resource types.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbQuotaStore.java 
> e7afbad33a92936f2949f00e60ade3703b97befe 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 07bdf22dcf90f6ca11a1d1800c59032ffad824ce 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 6e06af47c89ccb3b0dec1bda3683a76cd1effb81 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V005_CreateQuotaResourceTable.java
>  e9e5de10418d42035268c593541814705d145d26 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DBResourceAggregate.java
>  4f5c07e722ba580c1c3b45dcf83321694625bbaa 
>   src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java 
> ad45085e907d5386fb6d96f1297276c0a413ef23 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  6b5e1304d7213bd3a1483b4981e2558512b64a4e 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5d905db17ca478e789dd22c9c49e5c1e0d554a51 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java 
> f213b7f9a35a490e3d62283bb3b9e6b32ed4d624 
> 
> Diff: https://reviews.apache.org/r/46795/diff/
> 
> 
> Testing
> ---
> 
> unit and e2e tests
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 46795: Fixing e2e tests.

2016-04-28 Thread Joshua Cohen 


> On April 28, 2016, 6:31 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/migration/V005_CreateQuotaResourceTable.java,
> >  line 39
> > 
> >
> > What is value in the case of ports? I thought it would be the port 
> > name? In which case this constraint should still be valid?
> 
> Maxim Khutornenko wrote:
> Ports are not supported in ResourceAggregates, hence the ThriftBackfill 
> changes in this patch to only allow CPUS, RAM and disk. This constraint was 
> too permissive as it would allow something like 'CPUS, 1.0' and 'CPUS, 2.0', 
> which is no longer accepted.

Ahhh, gotcha, thanks for clarifying!


- Joshua


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


On April 28, 2016, 6:14 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46795/
> ---
> 
> (Updated April 28, 2016, 6:14 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Got too greedy converting pairs to maps during the last round of 
> https://reviews.apache.org/r/46716. Task resources must be handled via a list 
> of Pairs as there could be duplicate resource types there (e.g. ports).
> 
> Also, added more validation into ResourceAggregate backfilling to account for 
> invalid/missing resource types.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbQuotaStore.java 
> e7afbad33a92936f2949f00e60ade3703b97befe 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 07bdf22dcf90f6ca11a1d1800c59032ffad824ce 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 6e06af47c89ccb3b0dec1bda3683a76cd1effb81 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V005_CreateQuotaResourceTable.java
>  e9e5de10418d42035268c593541814705d145d26 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DBResourceAggregate.java
>  4f5c07e722ba580c1c3b45dcf83321694625bbaa 
>   src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java 
> ad45085e907d5386fb6d96f1297276c0a413ef23 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  6b5e1304d7213bd3a1483b4981e2558512b64a4e 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5d905db17ca478e789dd22c9c49e5c1e0d554a51 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java 
> f213b7f9a35a490e3d62283bb3b9e6b32ed4d624 
> 
> Diff: https://reviews.apache.org/r/46795/diff/
> 
> 
> Testing
> ---
> 
> unit and e2e tests
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 46795: Fixing e2e tests.

2016-04-28 Thread Aurora ReviewBot 

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


Ship it!




Master (e817eb1) 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 April 28, 2016, 6:14 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46795/
> ---
> 
> (Updated April 28, 2016, 6:14 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Got too greedy converting pairs to maps during the last round of 
> https://reviews.apache.org/r/46716. Task resources must be handled via a list 
> of Pairs as there could be duplicate resource types there (e.g. ports).
> 
> Also, added more validation into ResourceAggregate backfilling to account for 
> invalid/missing resource types.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbQuotaStore.java 
> e7afbad33a92936f2949f00e60ade3703b97befe 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 07bdf22dcf90f6ca11a1d1800c59032ffad824ce 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 6e06af47c89ccb3b0dec1bda3683a76cd1effb81 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V005_CreateQuotaResourceTable.java
>  e9e5de10418d42035268c593541814705d145d26 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DBResourceAggregate.java
>  4f5c07e722ba580c1c3b45dcf83321694625bbaa 
>   src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java 
> ad45085e907d5386fb6d96f1297276c0a413ef23 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  6b5e1304d7213bd3a1483b4981e2558512b64a4e 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5d905db17ca478e789dd22c9c49e5c1e0d554a51 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java 
> f213b7f9a35a490e3d62283bb3b9e6b32ed4d624 
> 
> Diff: https://reviews.apache.org/r/46795/diff/
> 
> 
> Testing
> ---
> 
> unit and e2e tests
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 46795: Fixing e2e tests.

2016-04-28 Thread Joshua Cohen 

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


Ship it!




lgtm assuming the question below is not an issue.


src/main/java/org/apache/aurora/scheduler/storage/db/migration/V005_CreateQuotaResourceTable.java
 (line 39)


What is value in the case of ports? I thought it would be the port name? In 
which case this constraint should still be valid?


- Joshua Cohen


On April 28, 2016, 6:14 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46795/
> ---
> 
> (Updated April 28, 2016, 6:14 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Got too greedy converting pairs to maps during the last round of 
> https://reviews.apache.org/r/46716. Task resources must be handled via a list 
> of Pairs as there could be duplicate resource types there (e.g. ports).
> 
> Also, added more validation into ResourceAggregate backfilling to account for 
> invalid/missing resource types.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbQuotaStore.java 
> e7afbad33a92936f2949f00e60ade3703b97befe 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 07bdf22dcf90f6ca11a1d1800c59032ffad824ce 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 6e06af47c89ccb3b0dec1bda3683a76cd1effb81 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V005_CreateQuotaResourceTable.java
>  e9e5de10418d42035268c593541814705d145d26 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DBResourceAggregate.java
>  4f5c07e722ba580c1c3b45dcf83321694625bbaa 
>   src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java 
> ad45085e907d5386fb6d96f1297276c0a413ef23 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  6b5e1304d7213bd3a1483b4981e2558512b64a4e 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5d905db17ca478e789dd22c9c49e5c1e0d554a51 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java 
> f213b7f9a35a490e3d62283bb3b9e6b32ed4d624 
> 
> Diff: https://reviews.apache.org/r/46795/diff/
> 
> 
> Testing
> ---
> 
> unit and e2e tests
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>