Potential implementation of urllib3.util.Retry #269
Conversation
No tests, no docs, etc, just trying to push through this to see what the logic's like. Some things I don't like: - it's fairly repetitive, we call increment/exhausted every time there's an error, and once when we have a response. - checking the response twice, once in increment and once in is_retryable - calling _connect_error_count from outside the class - conflict between individual retry and total retry values causing tests to fail, need to nail down this behavior - setting default timeout for a specific test instead of all of them
| @@ -377,7 +377,7 @@ def is_same_host(self, url): | |||
|
|
|||
| return (scheme, host, port) == (self.scheme, self.host, self.port) | |||
|
|
|||
| def urlopen(self, method, url, body=None, headers=None, retries=3, | |||
| def urlopen(self, method, url, body=None, headers=None, retries=None, | |||
There was a problem hiding this comment.
Probably should be _Default to match timeout.
There was a problem hiding this comment.
The reason we needed _Default was because None has semantic value for a timeout. None doesn't mean anything for retries. We just need a sentinel, it's easy to change it.
There was a problem hiding this comment.
None could have the semantic value of not having a retry limit (rather than use sys.maxint). At least that's what I feel we should do within the Retry(...) object, so it makes sense to have a similar semantic here? (I could also be convinced to allow True to be used interchangeably, but I feel None is more sensible because it often means "not specified", thus no restriction.)
_Default would be whatever sensible default we would like to provide.
0 (or maybe False could be used interchangeably) would have the semantic meaning of not allowing retries.
|
I think this PR is obsoleted by #326. Let me know if I'm mistaken. |
Reference PR for #260 , as mentioned in the last commit
No tests, no docs, etc, just trying to push through this to see what the
logic's like. Some things I don't like:
error, and once when we have a response.
_connect_error_countfrom outside the classfail, need to nail down this behavior