Skip to content

Commit

Permalink
[DISCO-3069] add region code to weather suggestion response [load tes…
Browse files Browse the repository at this point in the history
…t: (skip)] (#708)
  • Loading branch information
misaniwere authored Nov 14, 2024
1 parent ec494d4 commit fc4aaf9
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 11 deletions.
10 changes: 8 additions & 2 deletions merino/providers/weather/backends/accuweather/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@ class AccuweatherLocation(BaseModel):
# Default is US English (en-us).
localized_name: str

# Unique Administrative Area ID for the Location.
administrative_area_id: str


class WeatherData(NamedTuple):
"""The quartet for weather data used internally."""
Expand Down Expand Up @@ -633,13 +636,15 @@ async def as_awaitable(val: Any) -> Any:
if location_key and location is None:
# request was made with location key rather than geolocation
# so location info is not in the cache
location = AccuweatherLocation(localized_name="N/A", key=location_key)

location = AccuweatherLocation(
localized_name="N/A", key=location_key, administrative_area_id="N/A"
)
# if all the other three values are present, ttl here would be a valid ttl value
if location and current_conditions and forecast and ttl:
# Return the weather report with the values returned from the cache.
return WeatherReport(
city_name=location.localized_name,
region_code=location.administrative_area_id,
current_conditions=current_conditions,
forecast=forecast,
ttl=ttl,
Expand Down Expand Up @@ -690,6 +695,7 @@ async def as_awaitable(val: Any) -> Any:
return (
WeatherReport(
city_name=location.localized_name,
region_code=location.administrative_area_id,
current_conditions=current_conditions,
forecast=forecast,
ttl=weather_report_ttl,
Expand Down
4 changes: 4 additions & 0 deletions merino/providers/weather/backends/accuweather/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ def process_location_response_with_country_and_region(response: Any) -> dict[str
{
"Key": key,
"LocalizedName": localized_name,
"AdministrativeArea": {"ID": administrative_area_id},
},
*_,
]:
Expand All @@ -86,6 +87,7 @@ def process_location_response_with_country_and_region(response: Any) -> dict[str
return { # type: ignore
"key": key,
"localized_name": localized_name,
"administrative_area_id": administrative_area_id,
}
case _:
return None
Expand All @@ -102,6 +104,7 @@ def process_location_response_with_country(response: Any) -> dict[str, Any] | No
{
"Key": key,
"LocalizedName": localized_name,
"AdministrativeArea": {"ID": administrative_area_id},
},
]:
# `type: ignore` is necessary because mypy gets confused when
Expand All @@ -111,6 +114,7 @@ def process_location_response_with_country(response: Any) -> dict[str, Any] | No
return { # type: ignore
"key": key,
"localized_name": localized_name,
"administrative_area_id": administrative_area_id,
}
case _:
return None
Expand Down
1 change: 1 addition & 0 deletions merino/providers/weather/backends/protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class WeatherReport(BaseModel):
current_conditions: CurrentConditions
forecast: Forecast
ttl: int
region_code: str


class LocationCompletion(BaseModel):
Expand Down
2 changes: 2 additions & 0 deletions merino/providers/weather/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class Suggestion(BaseSuggestion):
"""Model for weather suggestions."""

city_name: str
region_code: str
current_conditions: CurrentConditions
forecast: Forecast

Expand Down Expand Up @@ -145,6 +146,7 @@ def build_suggestion(
score=self.score,
icon=None,
city_name=data.city_name,
region_code=data.region_code,
current_conditions=data.current_conditions,
forecast=data.forecast,
custom_details=CustomDetails(weather=WeatherDetails(weather_report_ttl=data.ttl)),
Expand Down
2 changes: 2 additions & 0 deletions tests/integration/api/v1/suggest/test_suggest_weather.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ def test_suggest_with_weather_report(client: TestClient, backend_mock: Any) -> N
"""
weather_report: WeatherReport = WeatherReport(
city_name="Milton",
region_code="WA",
current_conditions=CurrentConditions(
url=HttpUrl(
"http://www.accuweather.com/en/us/milton-wa/98354/current-weather/"
Expand Down Expand Up @@ -224,6 +225,7 @@ def test_suggest_with_weather_report(client: TestClient, backend_mock: Any) -> N
score=0.3,
icon=None,
city_name=weather_report.city_name,
region_code=weather_report.region_code,
current_conditions=weather_report.current_conditions,
forecast=weather_report.forecast,
)
Expand Down
11 changes: 3 additions & 8 deletions tests/integration/providers/weather/backends/test_accuweather.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ def fixture_expected_weather_report() -> WeatherReport:
"""Create an `AccuWeatherReport` for assertions"""
return WeatherReport(
city_name="San Francisco",
region_code="CA",
current_conditions=CurrentConditions(
url=HttpUrl(
"https://www.accuweather.com/en/us/san-francisco-ca/94103/"
Expand Down Expand Up @@ -108,6 +109,7 @@ def fixture_expected_weather_report_via_location_key() -> WeatherReport:
"""Create an `AccuWeatherReport` for assertions"""
return WeatherReport(
city_name="N/A",
region_code="N/A",
current_conditions=CurrentConditions(
url=HttpUrl(
"https://www.accuweather.com/en/us/san-francisco-ca/94103/"
Expand Down Expand Up @@ -184,6 +186,7 @@ def fixture_accuweather_cached_location_key() -> bytes:
location: dict[str, Any] = {
"key": "39376",
"localized_name": "San Francisco",
"administrative_area_id": "CA",
}
return json.dumps(location).encode("utf-8")

Expand Down Expand Up @@ -646,7 +649,6 @@ async def test_get_weather_report_with_location_key_from_cache(
statsd_mock: Any,
expected_weather_report_via_location_key: WeatherReport,
accuweather_parameters: dict[str, Any],
accuweather_cached_location_key: bytes,
accuweather_cached_current_conditions: bytes,
accuweather_cached_forecast_fahrenheit: bytes,
) -> None:
Expand Down Expand Up @@ -699,9 +701,7 @@ async def test_get_weather_report_via_location_key_with_both_current_conditions_
weather_context_with_location_key: WeatherContext,
expected_weather_report_via_location_key: WeatherReport,
accuweather_parameters: dict[str, Any],
accuweather_cached_location_key: bytes,
accuweather_current_conditions_response: bytes,
accuweather_cached_forecast_fahrenheit: bytes,
accuweather_forecast_response_fahrenheit: bytes,
) -> None:
"""Test that we can get weather report via location key with both current conditions and
Expand Down Expand Up @@ -756,7 +756,6 @@ async def test_get_weather_report_via_location_key_with_only_current_conditions_
weather_context_with_location_key: WeatherContext,
expected_weather_report_via_location_key: WeatherReport,
accuweather_parameters: dict[str, Any],
accuweather_cached_location_key: bytes,
accuweather_current_conditions_response: bytes,
accuweather_cached_forecast_fahrenheit: bytes,
) -> None:
Expand Down Expand Up @@ -876,11 +875,7 @@ async def test_get_weather_report_with_location_key_with_cache_error(
filter_caplog: FilterCaplogFixture,
weather_context_with_location_key: WeatherContext,
statsd_mock: Any,
expected_weather_report_via_location_key: WeatherReport,
accuweather_parameters: dict[str, Any],
accuweather_cached_location_key: bytes,
accuweather_cached_current_conditions: bytes,
accuweather_cached_forecast_fahrenheit: bytes,
mocker: MockerFixture,
) -> None:
"""Test that we catch the CacheAdapterError exception when running the script against the
Expand Down
5 changes: 4 additions & 1 deletion tests/unit/providers/weather/backends/test_accuweather.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ def fixture_expected_weather_report() -> WeatherReport:
"""Create an `AccuWeatherReport` for assertions"""
return WeatherReport(
city_name="San Francisco",
region_code="CA",
current_conditions=CurrentConditions(
url=HttpUrl(
"https://www.accuweather.com/en/us/san-francisco-ca/94103/"
Expand Down Expand Up @@ -353,6 +354,7 @@ def fixture_expected_weather_report_via_location_key() -> WeatherReport:
"""Create an `AccuWeatherReport` for assertions"""
return WeatherReport(
city_name="N/A",
region_code="N/A",
current_conditions=CurrentConditions(
url=HttpUrl(
"https://www.accuweather.com/en/us/san-francisco-ca/94103/"
Expand Down Expand Up @@ -588,6 +590,7 @@ def fixture_accuweather_cached_location_key() -> bytes:
location: dict[str, Any] = {
"key": "39376",
"localized_name": "San Francisco",
"administrative_area_id": "CA",
}
return orjson.dumps(location)

Expand Down Expand Up @@ -1605,7 +1608,7 @@ async def test_get_location_by_geolocation(
) -> None:
"""Test that the get_location method returns an AccuweatherLocation."""
expected_location: AccuweatherLocation = AccuweatherLocation(
key="39376", localized_name="San Francisco"
key="39376", localized_name="San Francisco", administrative_area_id="CA"
)
country: str = "US"
region: str = "CA"
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/providers/weather/test_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ def fixture_weather_report() -> WeatherReport:
"""Return a test WeatherReport."""
return WeatherReport(
city_name="San Francisco",
region_code="CA",
current_conditions=CurrentConditions(
url=HttpUrl(
"http://www.accuweather.com/en/us/san-francisco-ca/"
Expand Down Expand Up @@ -117,6 +118,7 @@ async def test_query_weather_report_returned(
score=settings.providers.accuweather.score,
icon=None,
city_name=weather_report.city_name,
region_code=weather_report.region_code,
current_conditions=weather_report.current_conditions,
forecast=weather_report.forecast,
custom_details=CustomDetails(
Expand Down

0 comments on commit fc4aaf9

Please sign in to comment.