Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RETRIES condition applied even during first attempt #181

Open
jpfabris opened this issue Jan 8, 2021 · 1 comment
Open

RETRIES condition applied even during first attempt #181

jpfabris opened this issue Jan 8, 2021 · 1 comment

Comments

@jpfabris
Copy link

jpfabris commented Jan 8, 2021

I'm using the 0.18.1 version for python3, and I've identified a behavior that doesn't seem right (forgive me in case i'm mistaken). The _conn_request method from Http class inside __init__.py has the following lines right at the beginning of the method that includes a while condition to handle the maximum attempts of connection:

i = 0
seen_bad_status_line = False
while i < RETRIES:
    <starts connection attempt>

Shouldn't it be an less or equal condition? There's a semantic issue with this approach since the first connection isn't actually a retry attempt, but it would do the trick. I have an use case in which I can't let more than one connection attempt, and I expected it would work when I set RETRIES = 0, since the expected would be to consider this amount after the first attempt. However I ended up having the RETRIES amount considered even at the initial attempt, which made the while condition fail and an
"local variable 'response' referenced before assignment" occurred afterwards, since in case of fail it ends up going straight to a return (response, content) line, but the response doesn't exist since it is declared inside the while loop.

@temoto
Copy link
Member

temoto commented Jan 8, 2021

You are right, re in RETRIES suggests it only works after first failure. Even comment above that variable says

A request will be tried RETRIES times if it fails at the socket/connection level.

If we change code to semantically proper behavior for re-tries, existing code in the wild will break for no good reason.

If you change httplib2.RETRIES = 1 it should work as you expect.

Error at RETRIES = 0 could be considered as separate problem with solution along these lines:

     def _conn_request(self, conn, request_uri, method, body, headers):
+        if RETRIES < 1:
+            raise ValueError("httplib2.RETRIES must be >= 1")
+
         i = 0

if you are willing to send in a patch with that, there should be a unit test too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants