diff --git a/CHANGELOG.md b/CHANGELOG.md index 15ff609..7ac47e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,17 @@ ### Fixed +- Ordering of exception handling to correctly parse a broken response - Assert that the expected columns are also present if the result is empty +### Removed + +- unused attributes `response` and `parameters`/`params` from `OhsomeResponse` + +### Added + +- init variable `data` to `OhsomeResponse` + ## 0.3.2 ### Fixed diff --git a/ohsome/clients.py b/ohsome/clients.py index 4053e33..d64e461 100644 --- a/ohsome/clients.py +++ b/ohsome/clients.py @@ -13,9 +13,9 @@ import pandas as pd import requests import shapely -from requests import Session +from requests import Session, Response from requests.adapters import HTTPAdapter -from requests.exceptions import RetryError +from requests.exceptions import RetryError, JSONDecodeError from urllib3 import Retry from ohsome import OhsomeException, OhsomeResponse @@ -332,37 +332,36 @@ def _handle_request(self) -> OhsomeResponse: Handles request to ohsome API :return: """ - ohsome_exception = None - response = None try: - response = self._session().post(url=self._url, data=self._parameters) - response.raise_for_status() - response.json() + response = self._post_request() + self._check_response(response) + data = self._get_response_data(response) + except OhsomeException as ohsome_exception: + if self.log: + ohsome_exception.log(self.log_dir) + raise ohsome_exception - except requests.exceptions.HTTPError as e: - try: - error_message = e.response.json()["message"] - except json.decoder.JSONDecodeError: - error_message = f"Invalid URL: Is {self._url} valid?" + return OhsomeResponse(data=data, url=self._url) - ohsome_exception = OhsomeException( - message=error_message, + def _post_request(self) -> Response: + try: + response = self._session().post(url=self._url, data=self._parameters) + except KeyboardInterrupt: + raise OhsomeException( + message="Keyboard Interrupt: Query was interrupted by the user.", url=self._url, params=self._parameters, - error_code=e.response.status_code, - response=e.response, + error_code=440, ) - except requests.exceptions.ConnectionError as e: - ohsome_exception = OhsomeException( + raise OhsomeException( message="Connection Error: Query could not be sent. Make sure there are no network " f"problems and that the ohsome API URL {self._url} is valid.", url=self._url, params=self._parameters, response=e.response, ) - except requests.exceptions.RequestException as e: if isinstance(e, RetryError): # retry one last time without retries, this will raise the original error instead of a cryptic retry @@ -371,22 +370,35 @@ def _handle_request(self) -> OhsomeResponse: self._OhsomeBaseClient__retry = False self._handle_request() - ohsome_exception = OhsomeException( + raise OhsomeException( message=str(e), url=self._url, params=self._parameters, response=e.response, ) + return response - except KeyboardInterrupt: - ohsome_exception = OhsomeException( - message="Keyboard Interrupt: Query was interrupted by the user.", + def _check_response(self, response: Response) -> None: + try: + response.raise_for_status() + except requests.exceptions.HTTPError as e: + try: + error_message = e.response.json()["message"] + except json.decoder.JSONDecodeError: + error_message = f"Invalid URL: Is {self._url} valid?" + + raise OhsomeException( + message=error_message, url=self._url, params=self._parameters, - error_code=440, + error_code=e.response.status_code, + response=e.response, ) - except ValueError as e: + def _get_response_data(self, response: Response) -> dict: + try: + return response.json() + except (ValueError, JSONDecodeError) as e: if response: error_code, message = extract_error_message_from_invalid_json( response.text @@ -394,30 +406,21 @@ def _handle_request(self) -> OhsomeResponse: else: message = str(e) error_code = None - ohsome_exception = OhsomeException( + raise OhsomeException( message=message, url=self._url, error_code=error_code, params=self._parameters, response=response, ) - except AttributeError: - ohsome_exception = OhsomeException( + raise OhsomeException( message=f"Seems like {self._url} is not a valid endpoint.", url=self._url, error_code=404, params=self._parameters, ) - # If there has been an error and logging is enabled, write it to file - if ohsome_exception: - if self.log: - ohsome_exception.log(self.log_dir) - raise ohsome_exception - - return OhsomeResponse(response, url=self._url, params=self._parameters) - def _format_parameters(self, params): """ Check and format parameters of the query diff --git a/ohsome/helper.py b/ohsome/helper.py index a34e4f9..ec5bc60 100644 --- a/ohsome/helper.py +++ b/ohsome/helper.py @@ -6,7 +6,7 @@ import datetime import json import re -from typing import Tuple, Union, List +from typing import Tuple, Union, List, Optional import geopandas as gpd import numpy as np @@ -245,12 +245,12 @@ def format_list_parameters(parameters: dict) -> dict: return parameters -def find_groupby_names(url): +def find_groupby_names(url: Optional[str]) -> List[str]: """ Get the groupBy names :return: """ - return [name.strip("/") for name in url.split("groupBy")[1:]] + return [name.strip("/") for name in url.split("groupBy")[1:]] if url else [] def extract_error_message_from_invalid_json(responsetext: str) -> Tuple[int, str]: diff --git a/ohsome/response.py b/ohsome/response.py index 4301475..259929a 100644 --- a/ohsome/response.py +++ b/ohsome/response.py @@ -16,12 +16,10 @@ class OhsomeResponse: """Contains the response of the request to the ohsome API""" - def __init__(self, response=None, url=None, params=None): + def __init__(self, data: dict, url: str = None): """Initialize the OhsomeResponse class.""" - self.response = response + self.data = data self.url = url - self.parameters = params - self.data = response.json() def as_dataframe( self, multi_index: Optional[bool] = True, explode_tags: Optional[tuple] = () diff --git a/ohsome/test/conftest.py b/ohsome/test/conftest.py index 2b7ee16..bf19e53 100644 --- a/ohsome/test/conftest.py +++ b/ohsome/test/conftest.py @@ -116,4 +116,4 @@ def dummy_ohsome_response() -> OhsomeResponse: ) response = Response() response._content = test_gdf.to_json().encode() - return OhsomeResponse(response=response) + return OhsomeResponse(data=response.json()) diff --git a/ohsome/test/test_exceptions.py b/ohsome/test/test_exceptions.py index 786a05e..f840987 100644 --- a/ohsome/test/test_exceptions.py +++ b/ohsome/test/test_exceptions.py @@ -43,6 +43,31 @@ def test_timeout_error(base_client): ) +def test_broken_response_timeout_error(base_client): + """Test whether an OhsomeException is raised in case of a JSONDecodeError.""" + + bboxes = "8.67066,49.41423,8.68177,49.4204" + time = "2010-01-01/2011-01-01/P1Y" + fltr = "building=* and type:way" + timeout = 30 + + client = base_client + with pytest.raises(ohsome.OhsomeException) as e_info: + with responses.RequestsMock() as rsps: + rsps.post( + "https://api.ohsome.org/v1/elements/geometry", + body=b'{\n "attribution" : {\n "url" : "https://ohsome.org/copyrights",\n "text" : "\xc2\xa9 OpenStreetMap contributors"\n },\n "apiVersion" : "1.10.3",\n "type" : "FeatureCollection",\n "features" : [{\n "timestamp" : "2024-07-31T10:37:31.603661767",\n "status" : 413,\n "message" : "The given query is too large in respect to the given timeout. Please use a smaller region and/or coarser time period.",\n "requestUrl" : "https://api.ohsome.org/v1/elements/geometry"\n}', + ) + client.elements.geometry.post( + bboxes=bboxes, time=time, filter=fltr, timeout=timeout + ) + assert ( + "The given query is too large in respect to the given timeout. Please use a smaller region and/or coarser " + "time period." in e_info.value.message + ) + assert e_info.value.error_code == 413 + + @pytest.mark.vcr def test_invalid_url(): """ @@ -194,7 +219,7 @@ def test_exception_connection_reset(base_client): """ with patch( - "requests.Response.raise_for_status", + "requests.sessions.Session.post", MagicMock( side_effect=RequestException( "This request was failed on purpose without response!" diff --git a/ohsome/test/test_response.py b/ohsome/test/test_response.py index fa3c3c5..6d01e70 100644 --- a/ohsome/test/test_response.py +++ b/ohsome/test/test_response.py @@ -561,7 +561,7 @@ def test_explode_tags_present_on_empty_result(): '{"attribution":{"url":"https://ohsome.org/copyrights","text":"© OpenStreetMap contributors"},' '"apiVersion":"1.10.1","type":"FeatureCollection","features":[]}' ).encode() - computed_df = OhsomeResponse(response=response).as_dataframe( + computed_df = OhsomeResponse(data=response.json()).as_dataframe( explode_tags=("some_key", "some_other_key") ) diff --git a/poetry.lock b/poetry.lock index 700ca98..07f389a 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1598,13 +1598,13 @@ files = [ [[package]] name = "notebook" -version = "7.2.1" +version = "7.2.2" description = "Jupyter Notebook - A web-based notebook environment for interactive computing" optional = false python-versions = ">=3.8" files = [ - {file = "notebook-7.2.1-py3-none-any.whl", hash = "sha256:f45489a3995746f2195a137e0773e2130960b51c9ac3ce257dbc2705aab3a6ca"}, - {file = "notebook-7.2.1.tar.gz", hash = "sha256:4287b6da59740b32173d01d641f763d292f49c30e7a51b89c46ba8473126341e"}, + {file = "notebook-7.2.2-py3-none-any.whl", hash = "sha256:c89264081f671bc02eec0ed470a627ed791b9156cad9285226b31611d3e9fe1c"}, + {file = "notebook-7.2.2.tar.gz", hash = "sha256:2ef07d4220421623ad3fe88118d687bc0450055570cdd160814a59cf3a1c516e"}, ] [package.dependencies]