Skip to content

Commit

Permalink
CC-5840: Add Media -> Endless retries if there is a validation error
Browse files Browse the repository at this point in the history 
Fixed by not adding message to retry queue if request returns a
validation specific error code (422)
  • Loading branch information
@drigato
drigato committed May 8, 2014
1 parent c291439 commit 0040965
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 4 deletions.
2 changes: 1 addition & 1 deletion airtime_mvc/application/modules/rest/controllers/MediaController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ private function fileNotFoundResponse()
private function invalidDataResponse()
{
$resp = $this->getResponse();
$resp->setHttpResponseCode(400);
$resp->setHttpResponseCode(422);
$resp->appendBody("ERROR: Invalid data");
}

Expand Down
10 changes: 7 additions & 3 deletions python_apps/airtime_analyzer/airtime_analyzer/status_reporter.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,13 @@ def send_http_request(picklable_request, retry_queue):
r.raise_for_status() # Raise an exception if there was an http error code returned
logging.info("HTTP request sent successfully.")
except requests.exceptions.RequestException as e:

This comment has been minimized.

Sign in to view

Copy link
@asantoni

asantoni Jun 3, 2014

Contributor

Also, I made a mistake here with this general RequestException handler. Only HTTPErrors give us a status code. ConnectionErrors won't, so I've handled HTTPErrors explicitly now.
# If the web server is having problems, retry the request later:
logging.error("HTTP request failed. Retrying later! Exception was: %s" % str(e))
retry_queue.append(picklable_request)
if r.status_code != 422:

This comment has been minimized.

Sign in to view

Copy link
@asantoni

asantoni Jun 3, 2014

Contributor

r is not defined in this context if the exception happens before the s.send() function call returns. We can't depend on variables created by the code above it to exist at the time the exception handler is invoked. (eg. A connection error won't fill in "r" because an exception gets thrown from inside that s.send() call.)

You should dig the necessary variables out of the exception. I've changed the code to be like this:
102 except requests.exceptions.HTTPError as e:
103 if e.response.status_code == 422:
# If the web server is having problems, retry the request later:
logging.error("HTTP request failed. Retrying later! Exception was: %s" % str(e))
retry_queue.append(picklable_request)
else:
# Do no retry the request if there was a metadata validation error
logging.error("HTTP request failed. Exception was: %s" % str(e))
except Exception as e:
logging.error("HTTP request failed with unhandled exception. %s" % str(e))
# Don't put the request into the retry queue, just give up on this one.
Expand Down

0 comments on commit 0040965

Please sign in to comment.