aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Maxim Khutornenko" <ma...@apache.org>
Subject Re: Review Request 24126: Adding a wait_for_batch_completion option into parallel updater.
Date Thu, 31 Jul 2014 04:09:13 GMT


> On July 31, 2014, 12:19 a.m., Bill Farner wrote:
> > src/main/python/apache/aurora/client/api/updater.py, line 242
> > <https://reviews.apache.org/r/24126/diff/1/?file=646402#file646402line242>
> >
> >     Nit - this flag is only gated on "if not".  Consider inverting the meaning of
the flag and don't negate.

The default (False) behavior is to not wait for completion. The current name feels right to
me as the feature is not ON by default.


> On July 31, 2014, 12:19 a.m., Bill Farner wrote:
> > src/main/python/apache/aurora/client/api/updater.py, line 245
> > <https://reviews.apache.org/r/24126/diff/1/?file=646402#file646402line245>
> >
> >     Ditto - gate is always negative.  Consider inverting the meaning and return
value of the function.

Renamed.


> On July 31, 2014, 12:19 a.m., Bill Farner wrote:
> > src/main/python/apache/aurora/client/api/updater.py, line 247
> > <https://reviews.apache.org/r/24126/diff/1/?file=646402#file646402line247>
> >
> >     s/Wait/Waiting/

done.


> On July 31, 2014, 12:19 a.m., Bill Farner wrote:
> > src/test/python/apache/aurora/client/api/test_updater.py, line 821
> > <https://reviews.apache.org/r/24126/diff/1/?file=646405#file646405line821>
> >
> >     Can you add two more test cases with batch_size > 1:
> >     
> >      - instances % batch_size == 0
> >      - instances % batch_size != 0

Unfortunately, any batch_size greater than 1 would require complete refactoring of the updater
unit tests and switching to python mock library. Mox does not handle out of order verification
properly. I have originally implemented both tests but had to back out at the last moment
as tests ran clean only about 80% of the time with other 20% failing stub verification.  


- Maxim


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


On July 30, 2014, 11:59 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24126/
> -----------------------------------------------------------
> 
> (Updated July 30, 2014, 11:59 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman.
> 
> 
> Bugs: AURORA-626
>     https://issues.apache.org/jira/browse/AURORA-626
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding a wait_for_batch_completion option into parallel updater.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/api/updater.py 05b4c0c76ac2a8551f3aa370ab487f9f0802c3dc

>   src/main/python/apache/aurora/client/api/updater_util.py c5f8f23912701568e1ee6b69186a533fdd29a5d7

>   src/main/python/apache/aurora/config/schema/base.py 3b90ccb33e6ffef9e14befd42d95b8b8a94e949b

>   src/test/python/apache/aurora/client/api/test_updater.py 7020712c9f0b33ec29646482517768ccb13e881f

> 
> Diff: https://reviews.apache.org/r/24126/diff/
> 
> 
> Testing
> -------
> 
> ./pants src/test/python:all
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message