Skip to content
This repository was archived by the owner on Oct 29, 2024. It is now read-only.

retry on HTTPError exceptions need raise_for_status()? #832

Open
loopway opened this issue Jun 16, 2020 · 3 comments
Open

retry on HTTPError exceptions need raise_for_status()? #832

loopway opened this issue Jun 16, 2020 · 3 comments
Assignees
Labels

Comments

@loopway
Copy link

loopway commented Jun 16, 2020

We occasionally receive a http status 500 code (time out) from our influxdb instance, probably because of temporary peak load. Looking at the retry code in InfluxDBClient class I can see that you try to catch HTTPErrors (line 345):

# Try to send the request more than once by default (see #103)
retry = True
_try = 0
while retry:
try:
response = self._session.request(
method=method,
url=url,
auth=(self._username, self._password),
params=params,
data=data,
stream=stream,
headers=headers,
proxies=self._proxies,
verify=self._verify_ssl,
timeout=self._timeout
)
break
except (requests.exceptions.ConnectionError,
requests.exceptions.HTTPError,
requests.exceptions.Timeout):
_try += 1
if self._retries != 0:
retry = _try < self._retries
if not retry:
raise
if method == "POST":
time.sleep((2 ** _try) * random.random() / 100.0)

According to the requests library documentation a raise_for_status() is needed for the exception to be caught:

https://requests.readthedocs.io/en/master/user/quickstart/#errors-and-exceptions

Or am I missing something?

@russorat
Copy link
Contributor

@loopway thanks for the issue. I'm not sure that's needed tbh. this code prints exception using python 3.7.7 on my mac:

# import requests module
import requests

try:
  # ping an incorrect url
  response = requests.get('https://not a real url/')

  print(response)
except (requests.exceptions.ConnectionError,
             requests.exceptions.HTTPError,
             requests.exceptions.Timeout):
  print("exception")

@loopway
Copy link
Author

loopway commented Jun 27, 2020

@russorat in your example it gets printed because it's a 'ConnectionError', and not a 'HTTPError'. Please try this code which will connect to httpstat.us to return a http error code 500:

# import requests module
import requests

try:
  # get a http status code 500
  response = requests.get('https://httpstat.us/500')

  print(response)

  # comment this and exception will not be raised
  response.raise_for_status()
except (requests.exceptions.ConnectionError,
             requests.exceptions.HTTPError,
             requests.exceptions.Timeout):
  # will only be printed if raise_for_status() is called
  print("exception")

@russorat
Copy link
Contributor

russorat commented Jul 6, 2020

@loopway thanks for clarifying! We'd be happy to review a PR if you're interested!

loopway added a commit to loopway/influxdb-python that referenced this issue Jul 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants