Mailing-List: contact reviews-help@aurora.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@aurora.incubator.apache.org
Content-Type: multipart/alternative;
 boundary="===============9033204895230510392=="
MIME-Version: 1.0
Subject: Re: Review Request 26298: Use a less broad retry loop for RPCs.
From: "Bill Farner" <wfarner@apache.org>
To: "Maxim Khutornenko" <maxim@apache.org>,
 "Mark Chu-Carroll" <mchucarroll@twopensource.com>
Cc: "Bill Farner" <wfarner@apache.org>,
 "Aurora" <reviews@aurora.incubator.apache.org>
Date: Fri, 03 Oct 2014 17:16:03 -0000
Message-ID: <20141003171603.19041.32602@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Bill Farner" <noreply@reviews.apache.org>
References: <20141003002102.18846.17473@reviews.apache.org>
In-Reply-To: <20141003002102.18846.17473@reviews.apache.org>
Reply-To: "Bill Farner" <wfarner@apache.org>

--===============9033204895230510392==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> On Oct. 3, 2014, 12:21 a.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/api/scheduler_client.py, line 286
> > <https://reviews.apache.org/r/26298/diff/1/?file=713279#file713279line286>
> >
> >     Why not just add a break into the Exception catcher instead?
> 
> Maxim Khutornenko wrote:
>     Actually, we are throwing there already. Wait, how is the exception is trapped in that loop?
> 
> Bill Farner wrote:
>     I scratched my head at this for a while, and wound up with this review: https://reviews.apache.org/r/26308/
>     
>     AFAICT `threading.Event` does not override `__bool__`, so those branches are never entered, and we loop until the timer is up.
> 
> Maxim Khutornenko wrote:
>     I am assuming this patch will be revisited then?
> 
> Bill Farner wrote:
>     Yup.  I'm going to open a new review to introduce the sleep between transient error retries, though.

Scratch that - didn't notice the local raise->catch on TransientError.  Closing.


- Bill


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


On Oct. 3, 2014, 12:18 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26298/
> -----------------------------------------------------------
> 
> (Updated Oct. 3, 2014, 12:18 a.m.)
> 
> 
> Review request for Aurora, Mark Chu-Carroll and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> A few times when making changes, i've found myself confused at a stalled test and spiked CPU, only to find that my test should have failed, but an exception is trapped in this retry loop.  The key change here is that unknown exceptions will break the loop.
> 
> Making this change pointed out what should have been a test failure in `test_transient_error`, where an exception caused by an unexpected call to `getVersion` was swallowed.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py b400cb2dbdb35077fc2c4a6e161c2959a9217317 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 1cbfbf86e903d890baac7d34461109f9beaff442 
> 
> Diff: https://reviews.apache.org/r/26298/diff/
> 
> 
> Testing
> -------
> 
> ./pants src/test/python:all -vxs
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


--===============9033204895230510392==--