Skip to content

Commit

Permalink
Send HTTP error results to comments
Browse files Browse the repository at this point in the history
Try to detect errors while posting style/license check comments and, if
errors are raised, post a new comment with the error description. This
should work for errors like https status 413, Payload too big, and maybe
a few others. Since the error might be due to instability in the service,
there wouldn't be any reason to retry, but since we don't know if that is
the case, print it to console as well and retry the POST anyway.

Signed-off-by: Fabio Utzig <[email protected]>
  • Loading branch information
utzig committed Mar 9, 2020
1 parent 02733f2 commit d614af9
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 11 deletions.
8 changes: 6 additions & 2 deletions check_license.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,12 @@ def get_files_per_commits(commits):

if DEBUG:
print("Comment body: ", comment)
if not backend.new_comment(owner, repo, TRAVIS_PULL_REQUEST, comment):
exit(1)
try:
if not backend.new_comment(owner, repo, TRAVIS_PULL_REQUEST, comment):
exit(1)
except backend.HttpError as e:
comment = LICENSE_BOT_ID + "\n## License check fail: " + str(e)
backend.new_comment(owner, repo, TRAVIS_PULL_REQUEST, comment)

# FIXME: check category-x?
sha, state = None, None
Expand Down
8 changes: 6 additions & 2 deletions check_style.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,12 @@ def main():
if DEBUG:
print("Comment body: ", comment)
owner, repo = TRAVIS_REPO_SLUG.split("/")
if not backend.new_comment(owner, repo, TRAVIS_PULL_REQUEST, comment):
exit(1)
try:
if not backend.new_comment(owner, repo, TRAVIS_PULL_REQUEST, comment):
exit(1)
except backend.HttpError as e:
comment = STYLE_BOT_ID + "\n## Style check fail: " + str(e)
backend.new_comment(owner, repo, TRAVIS_PULL_REQUEST, comment)


if __name__ == '__main__':
Expand Down
27 changes: 20 additions & 7 deletions utils/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,25 @@
RETRY_WAIT_TIME = 30


class HttpError(Exception):
def __init__(self, message):
self.message = message


def do_post(url, data, headers):
retry = 1
while retry <= MAX_RETRIES:
r = requests.post(url, data=data, headers=headers)
# FIXME: should use the correct code for no respose here?
if r.status_code == 200:

# 413 means payload too large, it happens when we try to submit too
# much data; in this situation there's not point in retrying
if r.status_code == 200 or r.status_code == 413:
break
else:
# FIXME: when it fails lt we know what has gone wrong!
print("post http status: {}".format(r.status_code))
print("FAIL - http status: {}".format(r.status_code))
time.sleep(RETRY_WAIT_TIME)
retry += 1
return r.status_code == 200
return r.status_code


def new_comment(owner, repo, pr, comment_body):
Expand All @@ -56,7 +62,13 @@ def new_comment(owner, repo, pr, comment_body):
'number': pr,
'body': comment_body,
}).encode('utf-8'))
return do_post(GH_COMMENTER_URL, data=payload, headers=headers)
status = do_post(GH_COMMENTER_URL, data=payload, headers=headers)
if status == 413:
raise HttpError("Payload was too large")
elif status != 200:
# this is for debug, better handling can be added in the future
raise HttpError("HTTP status {}".format(status))
return True


def send_status(owner, repo, sha, state):
Expand All @@ -71,4 +83,5 @@ def send_status(owner, repo, sha, state):
'sha': sha,
'state': state,
}).encode('utf-8'))
return do_post(GH_STATUS_REPORTER_URL, data=payload, headers=headers)
status = do_post(GH_STATUS_REPORTER_URL, data=payload, headers=headers)
return status == 200

0 comments on commit d614af9

Please sign in to comment.