You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2016/04/28 20:14:59 UTC
Review Request 46795: Fixing e2e tests.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46795/
-----------------------------------------------------------
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.
Posted by Maxim Khutornenko <ma...@apache.org>.
> 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
> > <https://reviews.apache.org/r/46795/diff/1/?file=1364957#file1364957line39>
> >
> > 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.
Posted by Joshua Cohen <jc...@apache.org>.
> 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
> > <https://reviews.apache.org/r/46795/diff/1/?file=1364957#file1364957line39>
> >
> > 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.
Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/46795/#comment194872>
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
>
>
Re: Review Request 46795: Fixing e2e tests.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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
>
>