fixup! Minor review feedback.

merino/providers/weather/backends/accuweather.py

@@ -83,9 +83,7 @@ def cache_inputs_for_weather_report(self, geolocation: Location) -> Optional[byt
         if geolocation.country is None or geolocation.postal_code is None:
             return None
 
-        return geolocation.country.encode("utf-8") + geolocation.postal_code.encode(
-            "utf-8"
-        )
+        return (geolocation.country + geolocation.postal_code).encode("utf-8")
 
     async def get_weather_report(
         self, geolocation: Location

merino/providers/weather/provider.py

@@ -76,7 +76,9 @@ def hidden(self) -> bool:  # noqa: D102
 
     def cache_key_for_weather_report(self, geolocation: Location) -> Optional[str]:
         """Compute a Redis key used to look up and store cached weather reports for a location."""
-        cache_inputs = self.backend.cache_inputs_for_weather_report(geolocation)
+        cache_inputs: Optional[bytes] = self.backend.cache_inputs_for_weather_report(
+            geolocation
+        )
         if not cache_inputs:
             return None
 
@@ -105,6 +107,8 @@ async def fetch_cached_weather_report(
                 return None
             return WeatherReport.parse_obj(weather_report_dict)
         except ValueError as exc:
+            # `ValueError` is the common superclass of `json.JSONDecodeError` and
+            # `pydantic.ValidationError`.
             raise BadCacheEntry("Failed to parse cache entry") from exc
 
     async def store_cached_weather_report(
@@ -115,7 +119,11 @@ async def store_cached_weather_report(
         if not cache_key:
             return
 
-        cache_value = json.dumps(weather_report.dict() if weather_report else {})
+        # If the request to the backend succeeded, but didn't return a report, we want to cache an
+        # empty value, so that subsequent requests for that location won't make additional backend
+        # calls every time. This case is separate from a transient backend error, which isn't
+        # negatively cached.
+        cache_value = weather_report.json() if weather_report else "{}"
         await self.redis.set(cache_key, cache_value, ex=self.cached_report_ttl_sec)
 
     async def query(self, srequest: SuggestionRequest) -> list[BaseSuggestion]:
@@ -131,11 +139,8 @@ async def query(self, srequest: SuggestionRequest) -> list[BaseSuggestion]:
             try:
                 weather_report = await self.backend.get_weather_report(geolocation)
                 try:
-                    # The call to `get_weather_report` can succeed, but return `None`. In that
-                    # case, we want to cache the absence of the report, so that subsequent requests
-                    # for that location aren't forwarded to the backend.
                     await self.store_cached_weather_report(geolocation, weather_report)
-                except Exception as exc:
+                except RedisError as exc:
                     logger.warning(f"Failed to store cached weather report: {exc}")
             except BackendError as backend_error:
                 logger.warning(backend_error)