So there's a somewhat related problem here; a Retry object is not safe 
to use in multiple calls to request() because the state of the object 
gets modified when an error condition is hit. Fixing this is somewhat 
messy; you can't just clone it off every time you call urlopen() because 
it calls itself recursively. You could maybe get clever with some kind 
of "_initialized" variable on the retry object, or calling a clean() 
function or something before urlopen eventually returns or raises; but 
this makes it easy to introduce future errors. You could check for the 
'retries' object in request(), and clone it there, but there may be 
other places that call urlopen() that are missed, and it's kind of a 
violation of responsibility.

I ended up solving this by making Retry objects immutable and creating 
new ones every time retry.increment() is called. In some ways this 
simplifies the design; the Retry object doesn't need to store 
_total_error_count, _read_error_count and other counts. Further, Retry 
objects are never modified once they are created, which makes them 
easier to reason about in some respects. This solves the reuse problem 
by creating a new Retry object any time its state is changed.

     In some ways however it makes it more verbose; certainly more memory is
     required to return a Retry object every time we increment it.

Curious what you thought or if you had other ideas about how to get 
around this.

On 1/29/14, 9:33, Andrey Petrov wrote:
>
> In urllib3/util/retry.py:
>
> > +        """
> > +
> > +        def _should_apply_total():
> > +            # _total_specified here checks whether we should be applying the
> > +            # limit at all.
> > +            return self._total_specified
> > +
> > +        return (
> > +            (_should_apply_total() and self._total_error_count > self.total)
> > +            or self._connect_error_count > max(self.connect, self.total)
> > +            or self._redirect_count > max(self.redirects, self.total)
> > +            or self._read_error_count > max(self.read, self.total))
> > +
> > +
> > +    @classmethod
> > +    def from_int(self, raw_retry):
>
> I don't think we need this. |Retry(x)| is the same as |Retry.from_int(x)|.
>
> —
> Reply to this email directly or view it on GitHub 
> <https://github.com/shazow/urllib3/pull/326/files#r9258994>.
>