From 7c22069c415b2240cf2d293fee1a04d1d7785aa9 Mon Sep 17 00:00:00 2001 From: Sean Gillies Date: Thu, 1 Sep 2022 14:51:07 -0600 Subject: [PATCH 1/4] Enable redirect following and avoid treating 302s as errors --- planet/http.py | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/planet/http.py b/planet/http.py index c9b2cf857..58a1d1aab 100644 --- a/planet/http.py +++ b/planet/http.py @@ -93,11 +93,13 @@ def _convert_and_raise(cls, error): @classmethod def _raise_for_status(cls, response): - try: - response.raise_for_status() - except httpx.HTTPStatusError as e: - e.response.read() - cls._convert_and_raise(e) + if response.status_code >= 400: + try: + response.raise_for_status() + except httpx.HTTPStatusError as e: + import pdb; pdb.set_trace() + e.response.read() + cls._convert_and_raise(e) return @@ -241,7 +243,7 @@ def __init__(self, auth: AuthType = None): LOGGER.info(f'Session read timeout set to {READ_TIMEOUT}.') timeout = httpx.Timeout(10.0, read=READ_TIMEOUT) - self._client = httpx.AsyncClient(auth=auth, timeout=timeout) + self._client = httpx.AsyncClient(auth=auth, timeout=timeout, follow_redirects=True) self._client.headers.update({'User-Agent': self._get_user_agent()}) self._client.headers.update({'X-Planet-App': 'python-sdk'}) @@ -265,11 +267,13 @@ async def alog_response(*args, **kwargs): @classmethod async def _raise_for_status(cls, response): - try: - response.raise_for_status() - except httpx.HTTPStatusError as e: - await e.response.aread() - cls._convert_and_raise(e) + if response.status_code >= 400: + try: + response.raise_for_status() + except httpx.HTTPStatusError as e: + import pdb; pdb.set_trace() + await e.response.aread() + cls._convert_and_raise(e) return From fa27daa5f4e048da3ba6b64d6de0401d4fa20155 Mon Sep 17 00:00:00 2001 From: Sean Gillies Date: Tue, 6 Sep 2022 17:48:04 -0600 Subject: [PATCH 2/4] Remove debugging, reformat --- planet/http.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/planet/http.py b/planet/http.py index 58a1d1aab..15ebecd60 100644 --- a/planet/http.py +++ b/planet/http.py @@ -97,7 +97,6 @@ def _raise_for_status(cls, response): try: response.raise_for_status() except httpx.HTTPStatusError as e: - import pdb; pdb.set_trace() e.response.read() cls._convert_and_raise(e) @@ -243,7 +242,9 @@ def __init__(self, auth: AuthType = None): LOGGER.info(f'Session read timeout set to {READ_TIMEOUT}.') timeout = httpx.Timeout(10.0, read=READ_TIMEOUT) - self._client = httpx.AsyncClient(auth=auth, timeout=timeout, follow_redirects=True) + self._client = httpx.AsyncClient(auth=auth, + timeout=timeout, + follow_redirects=True) self._client.headers.update({'User-Agent': self._get_user_agent()}) self._client.headers.update({'X-Planet-App': 'python-sdk'}) @@ -271,7 +272,6 @@ async def _raise_for_status(cls, response): try: response.raise_for_status() except httpx.HTTPStatusError as e: - import pdb; pdb.set_trace() await e.response.aread() cls._convert_and_raise(e) From a71c95179916a25222d89c325dc9c85b7329ba8a Mon Sep 17 00:00:00 2001 From: Sean Gillies Date: Tue, 6 Sep 2022 18:17:35 -0600 Subject: [PATCH 3/4] Change download test to fail if we don't follow redirects --- tests/integration/test_orders_api.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_orders_api.py b/tests/integration/test_orders_api.py index db4b19910..9acd4eec4 100644 --- a/tests/integration/test_orders_api.py +++ b/tests/integration/test_orders_api.py @@ -33,6 +33,7 @@ TEST_URL = 'http://www.MockNotRealURL.com/api/path' TEST_BULK_CANCEL_URL = f'{TEST_URL}/bulk/orders/v2/cancel' TEST_DOWNLOAD_URL = f'{TEST_URL}/download' +TEST_DOWNLOAD_ACTUAL_URL = f'{TEST_URL}/download_actual' TEST_ORDERS_URL = f'{TEST_URL}/orders/v2' TEST_STATS_URL = f'{TEST_URL}/stats/orders/v2' @@ -540,8 +541,16 @@ async def test_download_asset_md(tmpdir, session): 'Content-Type': 'application/json', 'Content-Disposition': 'attachment; filename="metadata.json"' } + + mock_redirect = httpx.Response(HTTPStatus.FOUND, + headers={ + 'Location': TEST_DOWNLOAD_ACTUAL_URL, + 'Content-Length': '0' + }) mock_resp = httpx.Response(HTTPStatus.OK, json=md_json, headers=md_headers) - respx.get(dl_url).return_value = mock_resp + + respx.get(dl_url).return_value = mock_redirect + respx.get(TEST_DOWNLOAD_ACTUAL_URL).return_value = mock_resp cl = OrdersClient(session, base_url=TEST_URL) filename = await cl.download_asset(dl_url, directory=str(tmpdir)) From eb79bd3891604b36a91d97545ca20b6310215c7c Mon Sep 17 00:00:00 2001 From: Sean Gillies Date: Tue, 6 Sep 2022 20:26:35 -0600 Subject: [PATCH 4/4] Use raise_for_status only if we have an error response This works around httpx's preference to raise in the redirect case. --- planet/http.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/planet/http.py b/planet/http.py index 15ebecd60..584c2714e 100644 --- a/planet/http.py +++ b/planet/http.py @@ -93,7 +93,7 @@ def _convert_and_raise(cls, error): @classmethod def _raise_for_status(cls, response): - if response.status_code >= 400: + if response.is_error: try: response.raise_for_status() except httpx.HTTPStatusError as e: @@ -268,7 +268,7 @@ async def alog_response(*args, **kwargs): @classmethod async def _raise_for_status(cls, response): - if response.status_code >= 400: + if response.is_error: try: response.raise_for_status() except httpx.HTTPStatusError as e: