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 <[email protected]>
NeonGeckoCom · May 7, 2024 · 51da6f3 · 51da6f3
1 parent 031ff19
commit 51da6f3
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 13 deletions.
diff --git 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
@@ -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
@@ -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,23 +74,31 @@ 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,
                         "appid": self._api_key,
                         "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
@@ -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
@@ -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,19 +86,19 @@ 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)
         self.assertEqual(query_str, f"i=how+far+away+is+Moscow%3F&units=metric&ip=50.47.129.133")
 
     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):