Handle correctly http error raise in run_http_exc
Currently http errors are raised inside a try bloc and transformed in a more generic exception. This change raises http errors outside the try bloc. Change-Id: Iece270ac84925625e848f3049072f39e19e068cc
This commit is contained in:
parent
0a1c1b0a86
commit
0393c989ae
@ -321,6 +321,10 @@ or code. This exit status will also be returned if the patchset is already
|
|||||||
applied to the current branch.
|
applied to the current branch.
|
||||||
.It 70
|
.It 70
|
||||||
Cannot determine top level Git directory or .git subdirectory path.
|
Cannot determine top level Git directory or .git subdirectory path.
|
||||||
|
.It 101
|
||||||
|
Unauthorized (401) http request done by git-review.
|
||||||
|
.It 104
|
||||||
|
Not Found (404) http request done by git-review.
|
||||||
.El
|
.El
|
||||||
.Pp
|
.Pp
|
||||||
Exit status larger than 31 indicates problem with
|
Exit status larger than 31 indicates problem with
|
||||||
|
@ -154,6 +154,11 @@ def run_command_exc(klazz, *argv, **env):
|
|||||||
return output
|
return output
|
||||||
|
|
||||||
|
|
||||||
|
def http_code_2_return_code(code):
|
||||||
|
"""Tranform http status code to system return code."""
|
||||||
|
return (code - 301) % 255 + 1
|
||||||
|
|
||||||
|
|
||||||
def run_http_exc(klazz, url, **env):
|
def run_http_exc(klazz, url, **env):
|
||||||
"""Run http GET request url, on failure raise klazz
|
"""Run http GET request url, on failure raise klazz
|
||||||
|
|
||||||
@ -168,12 +173,12 @@ def run_http_exc(klazz, url, **env):
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
res = requests.get(url, **env)
|
res = requests.get(url, **env)
|
||||||
if not 200 <= res.status_code < 300:
|
|
||||||
code = (res.status_code - 301) % 255 + 1
|
|
||||||
raise klazz(code, res.text, ('GET', url), env)
|
|
||||||
return res
|
|
||||||
except Exception as err:
|
except Exception as err:
|
||||||
raise klazz(255, str(err), ('GET', url), env)
|
raise klazz(255, str(err), ('GET', url), env)
|
||||||
|
if not 200 <= res.status_code < 300:
|
||||||
|
raise klazz(http_code_2_return_code(res.status_code),
|
||||||
|
res.text, ('GET', url), env)
|
||||||
|
return res
|
||||||
|
|
||||||
|
|
||||||
def get_version():
|
def get_version():
|
||||||
|
@ -152,3 +152,41 @@ class GitReviewConsole(testtools.TestCase, fixtures.TestWithFixtures):
|
|||||||
self.assertTrue(cmd.check_use_color_output(),
|
self.assertTrue(cmd.check_use_color_output(),
|
||||||
"Failed to use fallback to color.ui when "
|
"Failed to use fallback to color.ui when "
|
||||||
"color.review not present")
|
"color.review not present")
|
||||||
|
|
||||||
|
|
||||||
|
class FakeResponse(object):
|
||||||
|
|
||||||
|
def __init__(self, code, text=""):
|
||||||
|
self.status_code = code
|
||||||
|
self.text = text
|
||||||
|
|
||||||
|
|
||||||
|
class FakeException(Exception):
|
||||||
|
|
||||||
|
def __init__(self, code, *args, **kwargs):
|
||||||
|
super(FakeException, self).__init__(*args, **kwargs)
|
||||||
|
self.code = code
|
||||||
|
|
||||||
|
|
||||||
|
class GitReviewUnitTest(testtools.TestCase):
|
||||||
|
"""Class for misc unit tests."""
|
||||||
|
|
||||||
|
@mock.patch('requests.get', return_value=FakeResponse(404))
|
||||||
|
def test_run_http_exc_raise_http_error(self, mock_get):
|
||||||
|
url = 'http://gerrit.example.com'
|
||||||
|
try:
|
||||||
|
cmd.run_http_exc(FakeException, url)
|
||||||
|
self.fails('Exception expected')
|
||||||
|
except FakeException as err:
|
||||||
|
self.assertEqual(cmd.http_code_2_return_code(404), err.code)
|
||||||
|
mock_get.assert_called_once_with(url)
|
||||||
|
|
||||||
|
@mock.patch('requests.get', side_effect=Exception())
|
||||||
|
def test_run_http_exc_raise_unknown_error(self, mock_get):
|
||||||
|
url = 'http://gerrit.example.com'
|
||||||
|
try:
|
||||||
|
cmd.run_http_exc(FakeException, url)
|
||||||
|
self.fails('Exception expected')
|
||||||
|
except FakeException as err:
|
||||||
|
self.assertEqual(255, err.code)
|
||||||
|
mock_get.assert_called_once_with(url)
|
||||||
|
Loading…
Reference in New Issue
Block a user