You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2013/12/24 06:57:23 UTC
Review Request 16444: Client side changes for the client quota check.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16444/
-----------------------------------------------------------
Review request for Aurora, Mark Chu-Carroll, Bill Farner, and Brian Wickman.
Repository: aurora
Description
-------
Quota checks should be done outside of populateJobConfig(). This is needed to properly account for the released resources to prevent failures in job updates approaching their quota limits.
Part 1: schema and client related changes.
Diffs
-----
src/main/python/twitter/aurora/client/api/BUILD 4935b8a5da5621a581a825ea9da913279521ee7b
src/main/python/twitter/aurora/client/api/quota_check.py PRE-CREATION
src/main/python/twitter/aurora/client/api/updater.py 6e8e8a9fdfcf63a8fe0558d5ac36298601fd0552
src/main/python/twitter/aurora/client/commands/core.py 712c7ea04de61f1d66cab1db5c527c407393ec30
src/main/thrift/com/twitter/aurora/gen/api.thrift 9b3680b7f219e2805bdc37f2e7560cdca41a5913
src/test/python/twitter/aurora/client/api/BUILD 2850ece7280010d86348b2bc001e86258ef217a8
src/test/python/twitter/aurora/client/api/test_quota_check.py PRE-CREATION
src/test/python/twitter/aurora/client/api/test_updater.py ae4de2b57be16f8daf030210abda1ba05f9f30a0
src/test/python/twitter/aurora/client/commands/test_update.py 9aa2ba721d5c63803ca09bb3df24dc64f4583f74
Diff: https://reviews.apache.org/r/16444/diff/
Testing
-------
./pants src/test/python/twitter/aurora/client:all
Thanks,
Maxim Khutornenko
Re: Review Request 16444: Client side changes for the client quota check.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Jan. 3, 2014, 9:29 p.m., Brian Wickman wrote:
> > src/main/python/apache/aurora/client/api/quota_check.py, line 13
> > <https://reviews.apache.org/r/16444/diff/4/?file=414953#file414953line13>
> >
> > quota=None
Done.
> On Jan. 3, 2014, 9:29 p.m., Brian Wickman wrote:
> > src/main/python/apache/aurora/client/api/quota_check.py, lines 43-45
> > <https://reviews.apache.org/r/16444/diff/4/?file=414953#file414953line43>
> >
> > @classmethods at top before __init__
Moved.
> On Jan. 3, 2014, 9:29 p.m., Brian Wickman wrote:
> > src/main/python/apache/aurora/client/api/updater.py, line 283
> > <https://reviews.apache.org/r/16444/diff/4/?file=414954#file414954line283>
> >
> > or CapacityRequest()
> >
Done.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16444/#review31162
-----------------------------------------------------------
On Jan. 3, 2014, 9:21 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16444/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2014, 9:21 p.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Quota checks should be done outside of populateJobConfig(). This is needed to properly account for the released resources to prevent failures in job updates approaching their quota limits.
>
> Part 1: schema and client related changes.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c1a11bdb91c5e764864324d26248d1783af8048b
> src/main/python/apache/aurora/client/api/BUILD 9af74e9746500e7b388fc0b7fe7186fca0daefd3
> src/main/python/apache/aurora/client/api/quota_check.py PRE-CREATION
> src/main/python/apache/aurora/client/api/updater.py e364f26005d5194f2c8fea8c267fe2708c59d50b
> src/main/python/apache/aurora/client/commands/BUILD 556d00aab633106a98cbfb4b1ee2d8e755627c04
> src/main/python/apache/aurora/client/commands/core.py 45204b58ed14fdb83d31afe0fa4598f1b539062b
> src/main/thrift/org/apache/aurora/gen/api.thrift 480b8f472bcfbe547a91c41638250350a0110334
> src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 62fc8045f6a5fda234df73452685bd04e3142aaf
> src/test/python/apache/aurora/client/api/BUILD c5326752c9841a3f3cb8c9cc57f29f675e345788
> src/test/python/apache/aurora/client/api/test_quota_check.py PRE-CREATION
> src/test/python/apache/aurora/client/api/test_updater.py 5843bb4d3698abc5c4a48ee6eaf2be85e9415a06
> src/test/python/apache/aurora/client/commands/test_update.py ccc3ee9d97de4373b05ad2de8d88939510c2f052
>
> Diff: https://reviews.apache.org/r/16444/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/twitter/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 16444: Client side changes for the client quota check.
Posted by Brian Wickman <wi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16444/#review31162
-----------------------------------------------------------
Ship it!
minor nits
src/main/python/apache/aurora/client/api/quota_check.py
<https://reviews.apache.org/r/16444/#comment59536>
quota=None
src/main/python/apache/aurora/client/api/quota_check.py
<https://reviews.apache.org/r/16444/#comment59535>
@classmethods at top before __init__
src/main/python/apache/aurora/client/api/updater.py
<https://reviews.apache.org/r/16444/#comment59537>
or CapacityRequest()
- Brian Wickman
On Jan. 3, 2014, 9:21 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16444/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2014, 9:21 p.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Quota checks should be done outside of populateJobConfig(). This is needed to properly account for the released resources to prevent failures in job updates approaching their quota limits.
>
> Part 1: schema and client related changes.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c1a11bdb91c5e764864324d26248d1783af8048b
> src/main/python/apache/aurora/client/api/BUILD 9af74e9746500e7b388fc0b7fe7186fca0daefd3
> src/main/python/apache/aurora/client/api/quota_check.py PRE-CREATION
> src/main/python/apache/aurora/client/api/updater.py e364f26005d5194f2c8fea8c267fe2708c59d50b
> src/main/python/apache/aurora/client/commands/BUILD 556d00aab633106a98cbfb4b1ee2d8e755627c04
> src/main/python/apache/aurora/client/commands/core.py 45204b58ed14fdb83d31afe0fa4598f1b539062b
> src/main/thrift/org/apache/aurora/gen/api.thrift 480b8f472bcfbe547a91c41638250350a0110334
> src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 62fc8045f6a5fda234df73452685bd04e3142aaf
> src/test/python/apache/aurora/client/api/BUILD c5326752c9841a3f3cb8c9cc57f29f675e345788
> src/test/python/apache/aurora/client/api/test_quota_check.py PRE-CREATION
> src/test/python/apache/aurora/client/api/test_updater.py 5843bb4d3698abc5c4a48ee6eaf2be85e9415a06
> src/test/python/apache/aurora/client/commands/test_update.py ccc3ee9d97de4373b05ad2de8d88939510c2f052
>
> Diff: https://reviews.apache.org/r/16444/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/twitter/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 16444: Client side changes for the client quota check.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16444/#review31418
-----------------------------------------------------------
Thanks. Submitted.
- Maxim Khutornenko
On Jan. 4, 2014, 2:01 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16444/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2014, 2:01 a.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Quota checks should be done outside of populateJobConfig(). This is needed to properly account for the released resources to prevent failures in job updates approaching their quota limits.
>
> Part 1: schema and client related changes.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/BUILD 9af74e9746500e7b388fc0b7fe7186fca0daefd3
> src/main/python/apache/aurora/client/api/quota_check.py PRE-CREATION
> src/main/python/apache/aurora/client/api/updater.py e364f26005d5194f2c8fea8c267fe2708c59d50b
> src/main/python/apache/aurora/client/commands/BUILD 556d00aab633106a98cbfb4b1ee2d8e755627c04
> src/main/python/apache/aurora/client/commands/core.py 45204b58ed14fdb83d31afe0fa4598f1b539062b
> src/main/thrift/org/apache/aurora/gen/api.thrift 480b8f472bcfbe547a91c41638250350a0110334
> src/test/python/apache/aurora/client/api/BUILD c5326752c9841a3f3cb8c9cc57f29f675e345788
> src/test/python/apache/aurora/client/api/test_quota_check.py PRE-CREATION
> src/test/python/apache/aurora/client/api/test_updater.py 5843bb4d3698abc5c4a48ee6eaf2be85e9415a06
> src/test/python/apache/aurora/client/commands/test_update.py ccc3ee9d97de4373b05ad2de8d88939510c2f052
>
> Diff: https://reviews.apache.org/r/16444/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/twitter/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 16444: Client side changes for the client quota check.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Jan. 7, 2014, 5:14 p.m., Mark Chu-Carroll wrote:
> > src/main/python/apache/aurora/client/api/updater.py, line 281
> > <https://reviews.apache.org/r/16444/diff/6/?file=415186#file415186line281>
> >
> > nitpick, but doesn't the style guide say that local functions should be declared at the top of their enclosing method?
Could not find any definitive guidelines regarding positioning of class functions vs. local ones. Keeping broader scope methods at the top feels more natural and similar to inner classes ordering rules.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16444/#review31300
-----------------------------------------------------------
On Jan. 4, 2014, 2:01 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16444/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2014, 2:01 a.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Quota checks should be done outside of populateJobConfig(). This is needed to properly account for the released resources to prevent failures in job updates approaching their quota limits.
>
> Part 1: schema and client related changes.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/BUILD 9af74e9746500e7b388fc0b7fe7186fca0daefd3
> src/main/python/apache/aurora/client/api/quota_check.py PRE-CREATION
> src/main/python/apache/aurora/client/api/updater.py e364f26005d5194f2c8fea8c267fe2708c59d50b
> src/main/python/apache/aurora/client/commands/BUILD 556d00aab633106a98cbfb4b1ee2d8e755627c04
> src/main/python/apache/aurora/client/commands/core.py 45204b58ed14fdb83d31afe0fa4598f1b539062b
> src/main/thrift/org/apache/aurora/gen/api.thrift 480b8f472bcfbe547a91c41638250350a0110334
> src/test/python/apache/aurora/client/api/BUILD c5326752c9841a3f3cb8c9cc57f29f675e345788
> src/test/python/apache/aurora/client/api/test_quota_check.py PRE-CREATION
> src/test/python/apache/aurora/client/api/test_updater.py 5843bb4d3698abc5c4a48ee6eaf2be85e9415a06
> src/test/python/apache/aurora/client/commands/test_update.py ccc3ee9d97de4373b05ad2de8d88939510c2f052
>
> Diff: https://reviews.apache.org/r/16444/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/twitter/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 16444: Client side changes for the client quota check.
Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16444/#review31300
-----------------------------------------------------------
Ship it!
Ship It!
src/main/python/apache/aurora/client/api/updater.py
<https://reviews.apache.org/r/16444/#comment59760>
nitpick, but doesn't the style guide say that local functions should be declared at the top of their enclosing method?
- Mark Chu-Carroll
On Jan. 3, 2014, 9:01 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16444/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2014, 9:01 p.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Quota checks should be done outside of populateJobConfig(). This is needed to properly account for the released resources to prevent failures in job updates approaching their quota limits.
>
> Part 1: schema and client related changes.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/BUILD 9af74e9746500e7b388fc0b7fe7186fca0daefd3
> src/main/python/apache/aurora/client/api/quota_check.py PRE-CREATION
> src/main/python/apache/aurora/client/api/updater.py e364f26005d5194f2c8fea8c267fe2708c59d50b
> src/main/python/apache/aurora/client/commands/BUILD 556d00aab633106a98cbfb4b1ee2d8e755627c04
> src/main/python/apache/aurora/client/commands/core.py 45204b58ed14fdb83d31afe0fa4598f1b539062b
> src/main/thrift/org/apache/aurora/gen/api.thrift 480b8f472bcfbe547a91c41638250350a0110334
> src/test/python/apache/aurora/client/api/BUILD c5326752c9841a3f3cb8c9cc57f29f675e345788
> src/test/python/apache/aurora/client/api/test_quota_check.py PRE-CREATION
> src/test/python/apache/aurora/client/api/test_updater.py 5843bb4d3698abc5c4a48ee6eaf2be85e9415a06
> src/test/python/apache/aurora/client/commands/test_update.py ccc3ee9d97de4373b05ad2de8d88939510c2f052
>
> Diff: https://reviews.apache.org/r/16444/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/twitter/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 16444: Client side changes for the client quota check.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16444/
-----------------------------------------------------------
(Updated Jan. 4, 2014, 2:01 a.m.)
Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
Changes
-------
Dropping Lock from scheduler.getQuota().
Repository: aurora
Description
-------
Quota checks should be done outside of populateJobConfig(). This is needed to properly account for the released resources to prevent failures in job updates approaching their quota limits.
Part 1: schema and client related changes.
Diffs (updated)
-----
src/main/python/apache/aurora/client/api/BUILD 9af74e9746500e7b388fc0b7fe7186fca0daefd3
src/main/python/apache/aurora/client/api/quota_check.py PRE-CREATION
src/main/python/apache/aurora/client/api/updater.py e364f26005d5194f2c8fea8c267fe2708c59d50b
src/main/python/apache/aurora/client/commands/BUILD 556d00aab633106a98cbfb4b1ee2d8e755627c04
src/main/python/apache/aurora/client/commands/core.py 45204b58ed14fdb83d31afe0fa4598f1b539062b
src/main/thrift/org/apache/aurora/gen/api.thrift 480b8f472bcfbe547a91c41638250350a0110334
src/test/python/apache/aurora/client/api/BUILD c5326752c9841a3f3cb8c9cc57f29f675e345788
src/test/python/apache/aurora/client/api/test_quota_check.py PRE-CREATION
src/test/python/apache/aurora/client/api/test_updater.py 5843bb4d3698abc5c4a48ee6eaf2be85e9415a06
src/test/python/apache/aurora/client/commands/test_update.py ccc3ee9d97de4373b05ad2de8d88939510c2f052
Diff: https://reviews.apache.org/r/16444/diff/
Testing
-------
./pants src/test/python/twitter/aurora/client:all
Thanks,
Maxim Khutornenko
Re: Review Request 16444: Client side changes for the client quota check.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16444/
-----------------------------------------------------------
(Updated Jan. 3, 2014, 9:39 p.m.)
Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
Changes
-------
CR comments addressed.
Repository: aurora
Description
-------
Quota checks should be done outside of populateJobConfig(). This is needed to properly account for the released resources to prevent failures in job updates approaching their quota limits.
Part 1: schema and client related changes.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c1a11bdb91c5e764864324d26248d1783af8048b
src/main/python/apache/aurora/client/api/BUILD 9af74e9746500e7b388fc0b7fe7186fca0daefd3
src/main/python/apache/aurora/client/api/quota_check.py PRE-CREATION
src/main/python/apache/aurora/client/api/updater.py e364f26005d5194f2c8fea8c267fe2708c59d50b
src/main/python/apache/aurora/client/commands/BUILD 556d00aab633106a98cbfb4b1ee2d8e755627c04
src/main/python/apache/aurora/client/commands/core.py 45204b58ed14fdb83d31afe0fa4598f1b539062b
src/main/thrift/org/apache/aurora/gen/api.thrift 480b8f472bcfbe547a91c41638250350a0110334
src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 62fc8045f6a5fda234df73452685bd04e3142aaf
src/test/python/apache/aurora/client/api/BUILD c5326752c9841a3f3cb8c9cc57f29f675e345788
src/test/python/apache/aurora/client/api/test_quota_check.py PRE-CREATION
src/test/python/apache/aurora/client/api/test_updater.py 5843bb4d3698abc5c4a48ee6eaf2be85e9415a06
src/test/python/apache/aurora/client/commands/test_update.py ccc3ee9d97de4373b05ad2de8d88939510c2f052
Diff: https://reviews.apache.org/r/16444/diff/
Testing
-------
./pants src/test/python/twitter/aurora/client:all
Thanks,
Maxim Khutornenko
Re: Review Request 16444: Client side changes for the client quota check.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16444/
-----------------------------------------------------------
(Updated Jan. 3, 2014, 9:21 p.m.)
Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
Changes
-------
Adding Lock param into SchedulerThriftInterface to make build happy.
Repository: aurora
Description
-------
Quota checks should be done outside of populateJobConfig(). This is needed to properly account for the released resources to prevent failures in job updates approaching their quota limits.
Part 1: schema and client related changes.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c1a11bdb91c5e764864324d26248d1783af8048b
src/main/python/apache/aurora/client/api/BUILD 9af74e9746500e7b388fc0b7fe7186fca0daefd3
src/main/python/apache/aurora/client/api/quota_check.py PRE-CREATION
src/main/python/apache/aurora/client/api/updater.py e364f26005d5194f2c8fea8c267fe2708c59d50b
src/main/python/apache/aurora/client/commands/BUILD 556d00aab633106a98cbfb4b1ee2d8e755627c04
src/main/python/apache/aurora/client/commands/core.py 45204b58ed14fdb83d31afe0fa4598f1b539062b
src/main/thrift/org/apache/aurora/gen/api.thrift 480b8f472bcfbe547a91c41638250350a0110334
src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 62fc8045f6a5fda234df73452685bd04e3142aaf
src/test/python/apache/aurora/client/api/BUILD c5326752c9841a3f3cb8c9cc57f29f675e345788
src/test/python/apache/aurora/client/api/test_quota_check.py PRE-CREATION
src/test/python/apache/aurora/client/api/test_updater.py 5843bb4d3698abc5c4a48ee6eaf2be85e9415a06
src/test/python/apache/aurora/client/commands/test_update.py ccc3ee9d97de4373b05ad2de8d88939510c2f052
Diff: https://reviews.apache.org/r/16444/diff/
Testing
-------
./pants src/test/python/twitter/aurora/client:all
Thanks,
Maxim Khutornenko
Re: Review Request 16444: Client side changes for the client quota check.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16444/
-----------------------------------------------------------
(Updated Jan. 3, 2014, 8:38 p.m.)
Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
Changes
-------
Dropping wfarner from review per request.
Repository: aurora
Description
-------
Quota checks should be done outside of populateJobConfig(). This is needed to properly account for the released resources to prevent failures in job updates approaching their quota limits.
Part 1: schema and client related changes.
Diffs
-----
src/main/python/apache/aurora/client/api/BUILD 9af74e9746500e7b388fc0b7fe7186fca0daefd3
src/main/python/apache/aurora/client/api/quota_check.py PRE-CREATION
src/main/python/apache/aurora/client/api/updater.py e364f26005d5194f2c8fea8c267fe2708c59d50b
src/main/python/apache/aurora/client/commands/BUILD 556d00aab633106a98cbfb4b1ee2d8e755627c04
src/main/python/apache/aurora/client/commands/core.py 45204b58ed14fdb83d31afe0fa4598f1b539062b
src/main/thrift/org/apache/aurora/gen/api.thrift 480b8f472bcfbe547a91c41638250350a0110334
src/test/python/apache/aurora/client/api/BUILD c5326752c9841a3f3cb8c9cc57f29f675e345788
src/test/python/apache/aurora/client/api/test_quota_check.py PRE-CREATION
src/test/python/apache/aurora/client/api/test_updater.py 5843bb4d3698abc5c4a48ee6eaf2be85e9415a06
src/test/python/apache/aurora/client/commands/test_update.py ccc3ee9d97de4373b05ad2de8d88939510c2f052
Diff: https://reviews.apache.org/r/16444/diff/
Testing
-------
./pants src/test/python/twitter/aurora/client:all
Thanks,
Maxim Khutornenko
Re: Review Request 16444: Client side changes for the client quota check.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16444/
-----------------------------------------------------------
(Updated Jan. 3, 2014, 8:35 p.m.)
Review request for Aurora, Mark Chu-Carroll, Bill Farner, and Brian Wickman.
Changes
-------
CR comments addressed.
Repository: aurora
Description
-------
Quota checks should be done outside of populateJobConfig(). This is needed to properly account for the released resources to prevent failures in job updates approaching their quota limits.
Part 1: schema and client related changes.
Diffs (updated)
-----
src/main/python/apache/aurora/client/api/BUILD 9af74e9746500e7b388fc0b7fe7186fca0daefd3
src/main/python/apache/aurora/client/api/quota_check.py PRE-CREATION
src/main/python/apache/aurora/client/api/updater.py e364f26005d5194f2c8fea8c267fe2708c59d50b
src/main/python/apache/aurora/client/commands/BUILD 556d00aab633106a98cbfb4b1ee2d8e755627c04
src/main/python/apache/aurora/client/commands/core.py 45204b58ed14fdb83d31afe0fa4598f1b539062b
src/main/thrift/org/apache/aurora/gen/api.thrift 480b8f472bcfbe547a91c41638250350a0110334
src/test/python/apache/aurora/client/api/BUILD c5326752c9841a3f3cb8c9cc57f29f675e345788
src/test/python/apache/aurora/client/api/test_quota_check.py PRE-CREATION
src/test/python/apache/aurora/client/api/test_updater.py 5843bb4d3698abc5c4a48ee6eaf2be85e9415a06
src/test/python/apache/aurora/client/commands/test_update.py ccc3ee9d97de4373b05ad2de8d88939510c2f052
Diff: https://reviews.apache.org/r/16444/diff/
Testing
-------
./pants src/test/python/twitter/aurora/client:all
Thanks,
Maxim Khutornenko
Re: Review Request 16444: Client side changes for the client quota check.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16444/#review31142
-----------------------------------------------------------
src/main/python/apache/aurora/client/api/quota_check.py
<https://reviews.apache.org/r/16444/#comment59498>
Done.
src/main/python/apache/aurora/client/api/quota_check.py
<https://reviews.apache.org/r/16444/#comment59496>
You are right - it does not. This was rather a placeholder to communicate the intent to use client quota checks in both create and update. There are no tests yet and it's not wired into create sequence. Dropping it for now to address create path in a separate changelist.
src/main/python/apache/aurora/client/api/quota_check.py
<https://reviews.apache.org/r/16444/#comment59499>
Done.
src/main/python/apache/aurora/client/api/updater.py
<https://reviews.apache.org/r/16444/#comment59500>
I was split on creating a wrapper as it did not seem to add much benefit apart from hiding +/- ops. Going with your suggestion now as a better long term approach (though a bit more involved to account for sum() and mocking specifics).
src/test/python/apache/aurora/client/api/test_quota_check.py
<https://reviews.apache.org/r/16444/#comment59501>
Done.
- Maxim Khutornenko
On Jan. 3, 2014, 1:23 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16444/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2014, 1:23 a.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll, Bill Farner, and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Quota checks should be done outside of populateJobConfig(). This is needed to properly account for the released resources to prevent failures in job updates approaching their quota limits.
>
> Part 1: schema and client related changes.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/BUILD 9af74e9746500e7b388fc0b7fe7186fca0daefd3
> src/main/python/apache/aurora/client/api/quota_check.py PRE-CREATION
> src/main/python/apache/aurora/client/api/updater.py e364f26005d5194f2c8fea8c267fe2708c59d50b
> src/main/python/apache/aurora/client/commands/BUILD 556d00aab633106a98cbfb4b1ee2d8e755627c04
> src/main/python/apache/aurora/client/commands/core.py 45204b58ed14fdb83d31afe0fa4598f1b539062b
> src/main/thrift/org/apache/aurora/gen/api.thrift 480b8f472bcfbe547a91c41638250350a0110334
> src/test/python/apache/aurora/client/api/BUILD c5326752c9841a3f3cb8c9cc57f29f675e345788
> src/test/python/apache/aurora/client/api/test_quota_check.py PRE-CREATION
> src/test/python/apache/aurora/client/api/test_updater.py 5843bb4d3698abc5c4a48ee6eaf2be85e9415a06
> src/test/python/apache/aurora/client/commands/test_update.py ccc3ee9d97de4373b05ad2de8d88939510c2f052
>
> Diff: https://reviews.apache.org/r/16444/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/twitter/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 16444: Client side changes for the client quota check.
Posted by Brian Wickman <wi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16444/#review31136
-----------------------------------------------------------
src/main/python/apache/aurora/client/api/quota_check.py
<https://reviews.apache.org/r/16444/#comment59487>
add newline
(2 newlines between toplevel statements)
src/main/python/apache/aurora/client/api/quota_check.py
<https://reviews.apache.org/r/16444/#comment59489>
i'm confused -- how does this work with released=None, since you do requested = self._subtract(acquired, released) and afaict there is no released = released or Quota(0,0,0)
src/main/python/apache/aurora/client/api/quota_check.py
<https://reviews.apache.org/r/16444/#comment59488>
2sp
src/main/python/apache/aurora/client/api/updater.py
<https://reviews.apache.org/r/16444/#comment59490>
'list' should never be used as a variable name since it's a builtin.
instead it should probably be s/list/instances/ and s/item/instance/
also consider creating a Quota wrapper class that implements __add__ and __sub__, since you've already implemented it in quota_checker. this way you can just do
def _aggregate_quota(instances, config_map):
return sum(config_map[instance] for instance in instances)
and in quota_check instead of self._subtract and self._add, you can just do the fluent + and -.
src/test/python/apache/aurora/client/api/test_quota_check.py
<https://reviews.apache.org/r/16444/#comment59491>
2nl
- Brian Wickman
On Jan. 3, 2014, 1:23 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16444/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2014, 1:23 a.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll, Bill Farner, and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Quota checks should be done outside of populateJobConfig(). This is needed to properly account for the released resources to prevent failures in job updates approaching their quota limits.
>
> Part 1: schema and client related changes.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/BUILD 9af74e9746500e7b388fc0b7fe7186fca0daefd3
> src/main/python/apache/aurora/client/api/quota_check.py PRE-CREATION
> src/main/python/apache/aurora/client/api/updater.py e364f26005d5194f2c8fea8c267fe2708c59d50b
> src/main/python/apache/aurora/client/commands/BUILD 556d00aab633106a98cbfb4b1ee2d8e755627c04
> src/main/python/apache/aurora/client/commands/core.py 45204b58ed14fdb83d31afe0fa4598f1b539062b
> src/main/thrift/org/apache/aurora/gen/api.thrift 480b8f472bcfbe547a91c41638250350a0110334
> src/test/python/apache/aurora/client/api/BUILD c5326752c9841a3f3cb8c9cc57f29f675e345788
> src/test/python/apache/aurora/client/api/test_quota_check.py PRE-CREATION
> src/test/python/apache/aurora/client/api/test_updater.py 5843bb4d3698abc5c4a48ee6eaf2be85e9415a06
> src/test/python/apache/aurora/client/commands/test_update.py ccc3ee9d97de4373b05ad2de8d88939510c2f052
>
> Diff: https://reviews.apache.org/r/16444/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/twitter/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 16444: Client side changes for the client quota check.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16444/
-----------------------------------------------------------
(Updated Jan. 3, 2014, 1:23 a.m.)
Review request for Aurora, Mark Chu-Carroll, Bill Farner, and Brian Wickman.
Changes
-------
Merged with master. Converted quota_check tests from mox to mock.
Repository: aurora
Description
-------
Quota checks should be done outside of populateJobConfig(). This is needed to properly account for the released resources to prevent failures in job updates approaching their quota limits.
Part 1: schema and client related changes.
Diffs (updated)
-----
src/main/python/apache/aurora/client/api/BUILD 9af74e9746500e7b388fc0b7fe7186fca0daefd3
src/main/python/apache/aurora/client/api/quota_check.py PRE-CREATION
src/main/python/apache/aurora/client/api/updater.py e364f26005d5194f2c8fea8c267fe2708c59d50b
src/main/python/apache/aurora/client/commands/BUILD 556d00aab633106a98cbfb4b1ee2d8e755627c04
src/main/python/apache/aurora/client/commands/core.py 45204b58ed14fdb83d31afe0fa4598f1b539062b
src/main/thrift/org/apache/aurora/gen/api.thrift 480b8f472bcfbe547a91c41638250350a0110334
src/test/python/apache/aurora/client/api/BUILD c5326752c9841a3f3cb8c9cc57f29f675e345788
src/test/python/apache/aurora/client/api/test_quota_check.py PRE-CREATION
src/test/python/apache/aurora/client/api/test_updater.py 5843bb4d3698abc5c4a48ee6eaf2be85e9415a06
src/test/python/apache/aurora/client/commands/test_update.py ccc3ee9d97de4373b05ad2de8d88939510c2f052
Diff: https://reviews.apache.org/r/16444/diff/
Testing
-------
./pants src/test/python/twitter/aurora/client:all
Thanks,
Maxim Khutornenko
Re: Review Request 16444: Client side changes for the client quota check.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16444/#review31082
-----------------------------------------------------------
Unless anyone prefers otherwise, i'll step out of this review.
- Bill Farner
On Dec. 24, 2013, 5:57 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16444/
> -----------------------------------------------------------
>
> (Updated Dec. 24, 2013, 5:57 a.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll, Bill Farner, and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Quota checks should be done outside of populateJobConfig(). This is needed to properly account for the released resources to prevent failures in job updates approaching their quota limits.
>
> Part 1: schema and client related changes.
>
>
> Diffs
> -----
>
> src/main/python/twitter/aurora/client/api/BUILD 4935b8a5da5621a581a825ea9da913279521ee7b
> src/main/python/twitter/aurora/client/api/quota_check.py PRE-CREATION
> src/main/python/twitter/aurora/client/api/updater.py 6e8e8a9fdfcf63a8fe0558d5ac36298601fd0552
> src/main/python/twitter/aurora/client/commands/core.py 712c7ea04de61f1d66cab1db5c527c407393ec30
> src/main/thrift/com/twitter/aurora/gen/api.thrift 9b3680b7f219e2805bdc37f2e7560cdca41a5913
> src/test/python/twitter/aurora/client/api/BUILD 2850ece7280010d86348b2bc001e86258ef217a8
> src/test/python/twitter/aurora/client/api/test_quota_check.py PRE-CREATION
> src/test/python/twitter/aurora/client/api/test_updater.py ae4de2b57be16f8daf030210abda1ba05f9f30a0
> src/test/python/twitter/aurora/client/commands/test_update.py 9aa2ba721d5c63803ca09bb3df24dc64f4583f74
>
> Diff: https://reviews.apache.org/r/16444/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/twitter/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>