diff --git a/tools/repo_pull/gerrit.py b/tools/repo_pull/gerrit.py index a4cd7dc36..054e436eb 100755 --- a/tools/repo_pull/gerrit.py +++ b/tools/repo_pull/gerrit.py @@ -112,7 +112,15 @@ def create_url_opener_from_args(args): def _decode_xssi_json(data): - """Trim XSSI protector and decode JSON objects.""" + """Trim XSSI protector and decode JSON objects. + + Returns: + An object returned by json.loads(). + + Raises: + ValueError: If data doesn't start with a XSSI token. + json.JSONDecodeError: If data failed to decode. + """ # Decode UTF-8 data = data.decode('utf-8') @@ -144,6 +152,14 @@ def query_change_lists(url_opener, gerrit, query_string, limits): def _make_json_post_request(url_opener, url, data, method='POST'): + """Open an URL request and decode its response. + + Returns a 3-tuple of (code, body, json). + code: A numerical value, the HTTP status code of the response. + body: A bytes, the response body. + json: An object, the parsed JSON response. + """ + data = json.dumps(data).encode('utf-8') headers = { 'Content-Type': 'application/json; charset=UTF-8', @@ -159,11 +175,14 @@ def _make_json_post_request(url_opener, url, data, method='POST'): with response_file: res_code = response_file.getcode() - # Nothing to parse if response is '204 No Content' - if res_code == 204: - return (res_code, None) - res_json = _decode_xssi_json(response_file.read()) - return (res_code, res_json) + res_body = response_file.read() + try: + res_json = _decode_xssi_json(res_body) + except ValueError: + # The response isn't JSON if it doesn't start with a XSSI token. + # Possibly a plain text error message or empty body. + res_json = None + return (res_code, res_body, res_json) def set_review(url_opener, gerrit_url, change_id, labels, message): @@ -205,13 +224,8 @@ def delete_topic(url_opener, gerrit_url, change_id): """Delete the topic name.""" url = '{}/a/changes/{}/topic'.format(gerrit_url, change_id) - request = Request(url) - request.get_method = lambda: 'DELETE' - response_file = url_opener.open(request) - try: - return (response_file.getcode(), response_file.read()) - finally: - response_file.close() + + return _make_json_post_request(url_opener, url, {}, method='DELETE') def set_hashtags(url_opener, gerrit_url, change_id, add_tags=None, @@ -229,19 +243,6 @@ def set_hashtags(url_opener, gerrit_url, change_id, add_tags=None, return _make_json_post_request(url_opener, url, data) -def get_patch(url_opener, gerrit_url, change_id, revision_id='current'): - """Download the patch file.""" - - url = '{}/a/changes/{}/revisions/{}/patch'.format( - gerrit_url, change_id, revision_id) - - response_file = url_opener.open(url) - try: - return base64.b64decode(response_file.read()) - finally: - response_file.close() - - def add_reviewers(url_opener, gerrit_url, change_id, reviewers): """Add reviewers.""" @@ -264,6 +265,19 @@ def delete_reviewer(url_opener, gerrit_url, change_id, name): return _make_json_post_request(url_opener, url, {}) +def get_patch(url_opener, gerrit_url, change_id, revision_id='current'): + """Download the patch file.""" + + url = '{}/a/changes/{}/revisions/{}/patch'.format( + gerrit_url, change_id, revision_id) + + response_file = url_opener.open(url) + try: + return base64.b64decode(response_file.read()) + finally: + response_file.close() + + def _parse_args(): """Parse command line options.""" parser = argparse.ArgumentParser() diff --git a/tools/repo_pull/repo_review.py b/tools/repo_pull/repo_review.py index 743505b04..bf97093a7 100755 --- a/tools/repo_pull/repo_review.py +++ b/tools/repo_pull/repo_review.py @@ -139,7 +139,7 @@ _SEP_SPLIT = '=' * 79 _SEP = '-' * 79 -def _print_error(change, res_code, res_json): +def _print_error(change, res_code, res_body, res_json): """Print the error message""" change_id = change['change_id'] @@ -158,19 +158,19 @@ def _print_error(change, res_code, res_json): json.dump(res_json, sys.stderr, indent=4, separators=(', ', ': ')) print(file=sys.stderr) + elif res_body: + print(_SEP, file=sys.stderr) + print(res_body.decode('utf-8'), file=sys.stderr) print(_SEP_SPLIT, file=sys.stderr) def _do_task(change, func, *args, **kwargs): """Process a task and report errors when necessary.""" - try: - res_code, res_json = func(*args) - except HTTPError as error: - res_code = error.code - res_json = None + + res_code, res_body, res_json = func(*args) if res_code != kwargs.get('expected_http_code', 200): - _print_error(change, res_code, res_json) + _print_error(change, res_code, res_body, res_json) errors = kwargs.get('errors') if errors is not None: