From 51da6f3c3a15c5c08f87f85efc44d7eee45455c1 Mon Sep 17 00:00:00 2001 From: Daniel McKnight <34697904+NeonDaniel@users.noreply.github.com> Date: Tue, 7 May 2024 09:45:00 -0700 Subject: [PATCH] Update W|A and OWM wrappers to match upstream APIs (#96) * Update Wolfram|Alpha to use "imperial" instead of "nonmetric" to match API docs Update OWM to use onecall 3.0 as the latest * Add more error logging to troubleshoot test failure * Update OWM wrapper to add backwards-compat `error` keys * Update to validate updated API params * Update to raise backwards-compat errors for invalid lat/lon * Handle both `unit` and `units` kwargs to troubleshoot errors Prevent logging configured keys Prevent logging test API missing config * Undo 'unit' kwarg handling --------- Co-authored-by: Daniel McKnight --- neon_api_proxy/client/open_weather_map.py | 3 +++ neon_api_proxy/controller.py | 5 +++-- neon_api_proxy/services/owm_api.py | 18 +++++++++++++----- neon_api_proxy/services/wolfram_api.py | 3 ++- tests/test_wolfram_api.py | 10 +++++----- 5 files changed, 26 insertions(+), 13 deletions(-) diff --git a/neon_api_proxy/client/open_weather_map.py b/neon_api_proxy/client/open_weather_map.py index 0cfb3cf..3454839 100644 --- a/neon_api_proxy/client/open_weather_map.py +++ b/neon_api_proxy/client/open_weather_map.py @@ -104,6 +104,9 @@ def _make_api_call(lat: Union[str, float], lng: Union[str, float], except JSONDecodeError: data = {"error": "Error decoding response", "response": resp} + if data.get('cod') == "400": + # Backwards-compat. Put error response under `error` key + data["error"] = data.get("message") if data.get('cod'): data['cod'] = str(data['cod']) # TODO: Handle failures diff --git a/neon_api_proxy/controller.py b/neon_api_proxy/controller.py index 7fb6688..172e4cd 100644 --- a/neon_api_proxy/controller.py +++ b/neon_api_proxy/controller.py @@ -89,8 +89,9 @@ def init_service_instances(self, service_class_mapping: dict) -> dict: api_key = self.config.get(item, {}).get("api_key") if self.config \ else None try: - if api_key is None: - LOG.warning(f"No API key for {item} in {self.config}") + if api_key is None and item != 'api_test_endpoint': + LOG.warning(f"No API key for {item} in " + f"{list(self.config.keys())}") service_mapping[item] = \ service_class_mapping[item](api_key=api_key) except Exception as e: diff --git a/neon_api_proxy/services/owm_api.py b/neon_api_proxy/services/owm_api.py index d13603f..f25fa30 100644 --- a/neon_api_proxy/services/owm_api.py +++ b/neon_api_proxy/services/owm_api.py @@ -32,7 +32,7 @@ from requests import Response from neon_api_proxy.cached_api import CachedAPI -from ovos_utils.log import LOG +from ovos_utils.log import LOG, log_deprecation from neon_utils.authentication_utils import find_neon_owm_key @@ -74,15 +74,23 @@ def handle_query(self, **kwargs) -> dict: "content": repr(e), "encoding": None} if not resp.ok: - LOG.error(f"Bad response code: {resp.status_code}") + LOG.error(f"Bad response code: {resp.status_code}: " + f"content={resp.content}") return {"status_code": resp.status_code, "content": resp.content, "encoding": resp.encoding} def _get_api_response(self, lat: str, lng: str, units: str, api: str = "onecall", lang: str = "en") -> Response: - str(float(lat)) - str(float(lng)) + try: + assert isinstance(float(lat), float), f"Invalid latitude: {lat}" + assert isinstance(float(lng), float), f"Invalid longitude: {lng}" + except AssertionError as e: + raise ValueError(e) + if api != "onecall": + log_deprecation(f"{api} was requested but only `onecall` " + f"is supported", "1.0.0") + api = "onecall" assert units in ("metric", "imperial", "standard") query_params = {"lat": lat, "lon": lng, @@ -90,7 +98,7 @@ def _get_api_response(self, lat: str, lng: str, units: str, "units": units, "lang": lang} query_str = urllib.parse.urlencode(query_params) - base_url = "http://api.openweathermap.org/data/2.5" + base_url = "http://api.openweathermap.org/data/3.0" resp = self.get_with_cache_timeout(f"{base_url}/{api}?{query_str}", self.cache_timeout) return resp diff --git a/neon_api_proxy/services/wolfram_api.py b/neon_api_proxy/services/wolfram_api.py index 39250d3..175c536 100644 --- a/neon_api_proxy/services/wolfram_api.py +++ b/neon_api_proxy/services/wolfram_api.py @@ -92,7 +92,7 @@ def _build_query_string(**kwargs) -> str: query_params = dict() query_params['i'] = kwargs.get("query") query_params['units'] = kwargs.get("units") if \ - kwargs.get("units") == "metric" else "nonmetric" + kwargs.get("units") == "metric" else "imperial" lat = kwargs.get("lat") lng = kwargs.get("lng") if kwargs.get("latlong"): @@ -156,6 +156,7 @@ def _query_api(self, query: str) -> dict: :return: dict response containing: `status_code`, `content`, and `encoding` """ + LOG.debug(f"query={query}") result = self.get_with_cache_timeout(query, timeout=self.cache_time) if not result.ok: # 501 = Wolfram couldn't understand diff --git a/tests/test_wolfram_api.py b/tests/test_wolfram_api.py index b64db69..b36e26e 100644 --- a/tests/test_wolfram_api.py +++ b/tests/test_wolfram_api.py @@ -39,12 +39,12 @@ "ip": "50.47.129.133"} VALID_QUERY_LAT_LON = {"query": "how far away is new york?", - "units": "nonmetric", + "units": "imperial", "lat": "47.4797", "lng": "122.2079"} VALID_QUERY_LAT_LON_IP = {"query": "how far away is Bellevue?", - "units": "nonmetric", + "units": "nonmetric", # Test backwards-compat. "lat": "47.4797", "lng": "122.2079", "ip": "50.47.129.133"} @@ -86,11 +86,11 @@ def test_build_query_url_invalid_param_types(self): def test_build_query_string_valid_minimal(self): query_str = self.api._build_query_string(**VALID_QUERY_MINIMAL) - self.assertEqual(query_str, f"i=how+far+away+is+Miami%3F&units=nonmetric") + self.assertEqual(query_str, f"i=how+far+away+is+Miami%3F&units=imperial") def test_build_query_string_valid_lat_lng(self): query_str = self.api._build_query_string(**VALID_QUERY_LAT_LON) - self.assertEqual(query_str, f"i=how+far+away+is+new+york%3F&units=nonmetric&latlong=47.4797%2C122.2079") + self.assertEqual(query_str, f"i=how+far+away+is+new+york%3F&units=imperial&latlong=47.4797%2C122.2079") def test_build_query_string_valid_ip(self): query_str = self.api._build_query_string(**VALID_QUERY_IP) @@ -98,7 +98,7 @@ def test_build_query_string_valid_ip(self): def test_build_query_string_valid_lat_lng_ip(self): query_str = self.api._build_query_string(**VALID_QUERY_LAT_LON_IP) - self.assertEqual(query_str, f"i=how+far+away+is+Bellevue%3F&units=nonmetric&latlong=47.4797%2C122.2079") + self.assertEqual(query_str, f"i=how+far+away+is+Bellevue%3F&units=imperial&latlong=47.4797%2C122.2079") def test_build_query_invalid_query(self): with self.assertRaises(ValueError):