aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Steve Niemitz" <st...@tellapart.com>
Subject Re: Review Request 31423: Stop the announcer and status checkers before starting to kill the runners
Date Thu, 26 Feb 2015 03:40:58 GMT


> On Feb. 26, 2015, 3:38 a.m., Joshua Cohen wrote:
> > src/test/python/apache/aurora/executor/test_thermos_executor.py, lines 384-386
> > <https://reviews.apache.org/r/31423/diff/2/?file=877156#file877156line384>
> >
> >     You should be able to use contextlib.nested for this:
> >     
> >         with contextlib.nested(
> >             temporary_dir(),
> >             mock.patch.object(...),
> >             mock.patch.object(...)) as (
> >                 checkpoint_root, 
> >                 status_check_stop, 
> >                 runner_stop):
> >                 
> >           # use all the things
> >           
> >     Alternately, I think you can just wrap the with in parens as well, since we're
pinned to python2.7 now (though a quick scan of the code doesn't reveal any place that we
do that, but does show lots of places where we use contextlib.nested).

I tried parens and they don't work with "with" for whatever reason (syntax error).  I tried
to avoid contextlib.nested because it's deprecated as of 2.7, but I can use that if you guys
think its ok.


> On Feb. 26, 2015, 3:38 a.m., Joshua Cohen wrote:
> > src/test/python/apache/aurora/executor/test_thermos_executor.py, line 398
> > <https://reviews.apache.org/r/31423/diff/2/?file=877156#file877156line398>
> >
> >     Is it worth being explicit and passing any_order=False (even though it's the
default), just to make it clear that the order is important?
> >     
> >     I had to look it up to see what the behavior was, maybe for people more familiar
with python/mock would know right away, but it's not abundantly clear.

:thumbsup:


- Steve


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


On Feb. 26, 2015, 2:14 a.m., Steve Niemitz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31423/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2015, 2:14 a.m.)
> 
> 
> Review request for Aurora, Brian Wickman and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Stop the announcer and status checkers before starting to kill the runners.
> 
> This allows the task to be removed from the ZK ensemble before it begins getting killed.
 The delay can be significant if the task takes some time to shutdown, and during the time
it stops responding to requests.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 9c0282392dbb9cca308baf47adc1750c1f5cacc6

>   src/test/python/apache/aurora/executor/BUILD 2ee9b1233e9db47455ddccccffbc48691d379222

>   src/test/python/apache/aurora/executor/test_thermos_executor.py 8dbfb1db5eb7a6548820ff7cf82a9c7092f61d28

> 
> Diff: https://reviews.apache.org/r/31423/diff/
> 
> 
> Testing
> -------
> 
> We're now running this in our production environments.  Watching ZK, I can confirm that
the nodes are removed before process shutdown begins.  Watching the executor log also confirms
this.
> 
> I couldn't observe any other side effects either.
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>


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