From 480c885122642e220b34d6eba1383853f6584c26 Mon Sep 17 00:00:00 2001 From: Brigitta Sipocz Date: Wed, 25 Feb 2015 15:12:01 +0000 Subject: [PATCH 1/5] Checking response status, and raise exception if it retuns with a client or a server error --- astroquery/utils/commons.py | 7 +++++-- astroquery/utils/testing_tools.py | 5 +++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/astroquery/utils/commons.py b/astroquery/utils/commons.py index 2a91ad5eb2..3f175fffc4 100644 --- a/astroquery/utils/commons.py +++ b/astroquery/utils/commons.py @@ -84,13 +84,16 @@ def send_request(url, data, timeout, request_type='POST', headers={}, if request_type == 'GET': response = requests.get(url, params=data, timeout=timeout, headers=headers, **kwargs) - return response elif request_type == 'POST': response = requests.post(url, data=data, timeout=timeout, headers=headers, **kwargs) - return response else: raise ValueError("request_type must be either 'GET' or 'POST'.") + + if response.status_code >= 400: + raise response.raise_for_status() + else: + return response except requests.exceptions.Timeout: raise TimeoutError("Query timed out, time elapsed {time}s". format(time=timeout)) diff --git a/astroquery/utils/testing_tools.py b/astroquery/utils/testing_tools.py index 725e70fa6b..fcda184e45 100644 --- a/astroquery/utils/testing_tools.py +++ b/astroquery/utils/testing_tools.py @@ -33,15 +33,16 @@ class MockResponse(object): """ def __init__(self, content=None, url=None, headers={}, - content_type=None, stream=False, auth=None): + content_type=None, stream=False, auth=None, status_code=200): assert content is None or hasattr(content, 'decode') self.content = content self.raw = content self.headers = headers if content_type is not None: - self.headers.update({'Content-Type':content_type}) + self.headers.update({'Content-Type': content_type}) self.url = url self.auth = auth + self.status_code = status_code def iter_lines(self): c = self.content.split(b"\n") From 744d07ea409086484471c06f219293dd5a49abaa Mon Sep 17 00:00:00 2001 From: Brigitta Sipocz Date: Wed, 25 Feb 2015 15:40:13 +0000 Subject: [PATCH 2/5] Fix mock tests to have status_code attribute --- astroquery/utils/tests/test_utils.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/astroquery/utils/tests/test_utils.py b/astroquery/utils/tests/test_utils.py index 289635efa9..00fd28262a 100644 --- a/astroquery/utils/tests/test_utils.py +++ b/astroquery/utils/tests/test_utils.py @@ -85,14 +85,16 @@ def test_parse_radius_2(radius): def test_send_request_post(monkeypatch): - def mock_post(url, data, timeout, headers={}): + def mock_post(url, data, timeout, headers={}, status_code=200): class SpecialMockResponse(object): - def __init__(self, url, data, headers): + def __init__(self, url, data, headers, status_code): self.url = url self.data = data self.headers = headers - return SpecialMockResponse(url, data, headers=headers) + self.status_code = status_code + return SpecialMockResponse(url, data, headers=headers, + status_code=status_code) monkeypatch.setattr(requests, 'post', mock_post) response = commons.send_request('https://github.com/astropy/astroquery', @@ -103,8 +105,9 @@ def __init__(self, url, data, headers): def test_send_request_get(monkeypatch): - def mock_get(url, params, timeout, headers={}): + def mock_get(url, params, timeout, headers={}, status_code=200): req = requests.Request('GET', url, params=params, headers=headers).prepare() + req.status_code = status_code return req monkeypatch.setattr(requests, 'get', mock_get) response = commons.send_request('https://github.com/astropy/astroquery', @@ -113,8 +116,9 @@ def mock_get(url, params, timeout, headers={}): def test_quantity_timeout(monkeypatch): - def mock_get(url, params, timeout, headers={}): + def mock_get(url, params, timeout, headers={}, status_code=200): req = requests.Request('GET', url, params=params, headers=headers).prepare() + req.status_code = status_code return req monkeypatch.setattr(requests, 'get', mock_get) response = commons.send_request('https://github.com/astropy/astroquery', From b1b8fa9173bb0ab2e61f9e7c75c1179957f87f9c Mon Sep 17 00:00:00 2001 From: Brigitta Sipocz Date: Wed, 25 Feb 2015 18:22:31 +0000 Subject: [PATCH 3/5] removing unnecessary if statement --- astroquery/utils/commons.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/astroquery/utils/commons.py b/astroquery/utils/commons.py index 3f175fffc4..cb32e9f332 100644 --- a/astroquery/utils/commons.py +++ b/astroquery/utils/commons.py @@ -90,10 +90,10 @@ def send_request(url, data, timeout, request_type='POST', headers={}, else: raise ValueError("request_type must be either 'GET' or 'POST'.") - if response.status_code >= 400: - raise response.raise_for_status() - else: - return response + response.raise_for_status() + + return response + except requests.exceptions.Timeout: raise TimeoutError("Query timed out, time elapsed {time}s". format(time=timeout)) From de5dcb2a8aea6a45de689490be05cb09df345003 Mon Sep 17 00:00:00 2001 From: Brigitta Sipocz Date: Wed, 25 Feb 2015 19:23:38 +0000 Subject: [PATCH 4/5] Adding CHANGES entry --- CHANGES | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGES b/CHANGES index 289e62fc9a..f79cc4eaaf 100644 --- a/CHANGES +++ b/CHANGES @@ -1,14 +1,15 @@ 0.3 (unreleased) ---------------- -- None yet +- Bugfix for ``utils.commons.send_request()``: Raise exception if error status + is returned in the response. (#491) 0.2.3 (2014-09-30) ------------------ - AstroResponse has been removed, which means that all cached objects will have - new hashes. You should clear your cache: for most users, that means + new hashes. You should clear your cache: for most users, that means ``rm -r ~/.astropy/cache/astroquery/`` (#418) - In ESO and ALMA, default to *not* storing your password. New keyword ``store_password=False``. (#415) From eeaf4787b223031b536b17c28e1eb8a81f6d886d Mon Sep 17 00:00:00 2001 From: Brigitta Sipocz Date: Thu, 26 Feb 2015 12:36:29 +0000 Subject: [PATCH 5/5] Fix mock tests to have raise_for_status() method --- astroquery/utils/tests/test_utils.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/astroquery/utils/tests/test_utils.py b/astroquery/utils/tests/test_utils.py index 00fd28262a..35aa013251 100644 --- a/astroquery/utils/tests/test_utils.py +++ b/astroquery/utils/tests/test_utils.py @@ -93,6 +93,10 @@ def __init__(self, url, data, headers, status_code): self.data = data self.headers = headers self.status_code = status_code + + def raise_for_status(self): + pass + return SpecialMockResponse(url, data, headers=headers, status_code=status_code) monkeypatch.setattr(requests, 'post', mock_post) @@ -108,6 +112,7 @@ def test_send_request_get(monkeypatch): def mock_get(url, params, timeout, headers={}, status_code=200): req = requests.Request('GET', url, params=params, headers=headers).prepare() req.status_code = status_code + req.raise_for_status = lambda: None return req monkeypatch.setattr(requests, 'get', mock_get) response = commons.send_request('https://github.com/astropy/astroquery', @@ -119,6 +124,7 @@ def test_quantity_timeout(monkeypatch): def mock_get(url, params, timeout, headers={}, status_code=200): req = requests.Request('GET', url, params=params, headers=headers).prepare() req.status_code = status_code + req.raise_for_status = lambda: None return req monkeypatch.setattr(requests, 'get', mock_get) response = commons.send_request('https://github.com/astropy/astroquery',