From f05016196dbf15e1244964d3d905b0a8cfd7053a Mon Sep 17 00:00:00 2001 From: MatthewFlamm <39341281+MatthewFlamm@users.noreply.github.com> Date: Wed, 8 May 2024 05:56:13 -0400 Subject: [PATCH] Retry with no data (#195) * retry-no-data and use in update_observation * fix retry_no_data propagation * enable for other updates, but make NotImplemented * fix typing * remove .vscode/settings.json * better docstrings * tests for NwsNoDataError and match statements * add match statements to NotImplementedError in tests * fix typo * remove unneeded settings * add test for raising stale forecasts --- src/pynws/__init__.py | 3 +- src/pynws/nws.py | 4 + src/pynws/simple_nws.py | 119 +++++++++++++--- .../gridpoints_forecast_hourly_empty.json | 92 +++++++++++++ tests/test_simple_nws.py | 128 +++++++++++++++--- 5 files changed, 310 insertions(+), 36 deletions(-) create mode 100644 tests/fixtures/gridpoints_forecast_hourly_empty.json diff --git a/src/pynws/__init__.py b/src/pynws/__init__.py index f8832c3..d498c0c 100644 --- a/src/pynws/__init__.py +++ b/src/pynws/__init__.py @@ -4,13 +4,14 @@ """ from .forecast import DetailedForecast -from .nws import Nws, NwsError +from .nws import Nws, NwsError, NwsNoDataError from .simple_nws import SimpleNWS, call_with_retry __all__ = [ "DetailedForecast", "Nws", "NwsError", + "NwsNoDataError", "SimpleNWS", "call_with_retry", ] diff --git a/src/pynws/nws.py b/src/pynws/nws.py index c0a2fe1..aabf0e1 100644 --- a/src/pynws/nws.py +++ b/src/pynws/nws.py @@ -28,6 +28,10 @@ def __init__(self: NwsError, message: str): self.message = message +class NwsNoDataError(NwsError): + """No data was returned.""" + + class Nws: """Class to more easily get data for one location.""" diff --git a/src/pynws/simple_nws.py b/src/pynws/simple_nws.py index a372619..4750a7f 100644 --- a/src/pynws/simple_nws.py +++ b/src/pynws/simple_nws.py @@ -26,7 +26,7 @@ from .const import ALERT_ID, API_WEATHER_CODE, Final from .forecast import DetailedForecast -from .nws import Nws, NwsError +from .nws import Nws, NwsError, NwsNoDataError from .units import convert_unit WIND_DIRECTIONS: Final = [ @@ -52,23 +52,48 @@ WIND: Final = {name: idx * 360 / 16 for idx, name in enumerate(WIND_DIRECTIONS)} -def _is_500_error(error: BaseException) -> bool: - """Return True if error is ClientResponseError and has a 5xx status.""" - return isinstance(error, ClientResponseError) and error.status >= 500 +def _nws_retry_func(retry_no_data: bool): + """ + Return function used for tenacity.retry. + + Retry if: + - if error is ClientResponseError and has a 5xx status. + - if error is NwsNoDataError, the behavior is determined by retry_no_data + + Parameters + ---------- + retry_no_data : bool + Whether to retry when `NwsNoDataError` is raised. + + """ + + def _retry(error: BaseException) -> bool: + """Whether to retry based on execptions.""" + if isinstance(error, ClientResponseError) and error.status >= 500: + return True + if retry_no_data and isinstance(error, NwsNoDataError): + return True + return False + + return _retry def _setup_retry_func( func: Callable[[Any, Any], Awaitable[Any]], interval: Union[float, timedelta], stop: Union[float, timedelta], -) -> Callable[[Any, Any], Awaitable[Any]]: + *, + retry_no_data=False, +) -> Callable[..., Awaitable[Any]]: from tenacity import retry, retry_if_exception, stop_after_delay, wait_fixed + retry_func = _nws_retry_func(retry_no_data=retry_no_data) + return retry( reraise=True, wait=wait_fixed(interval), stop=stop_after_delay(stop), - retry=retry_if_exception(_is_500_error), + retry=retry_if_exception(retry_func), )(func) @@ -78,6 +103,7 @@ async def call_with_retry( stop: Union[float, timedelta], /, *args, + retry_no_data=False, **kwargs, ) -> Callable[[Any, Any], Awaitable[Any]]: """Call an update function with retries. @@ -90,13 +116,15 @@ async def call_with_retry( Time interval for retry. stop : float, datetime.datetime.timedelta Time interval to stop retrying. + retry_no_data : bool + Whether to retry when no data is returned. args : Any Positional args to pass to func. kwargs : Any Keyword args to pass to func. """ - retried_func = _setup_retry_func(func, interval, stop) - return await retried_func(*args, **kwargs) + retried_func = _setup_retry_func(func, interval, stop, retry_no_data=retry_no_data) + return await retried_func(*args, raise_no_data=retry_no_data, **kwargs) class MetarParam(NamedTuple): @@ -219,24 +247,47 @@ def extract_metar(obs: Dict[str, Any]) -> Optional[Metar.Metar]: return metar_obs async def update_observation( - self: SimpleNWS, limit: int = 0, start_time: Optional[datetime] = None + self: SimpleNWS, + limit: int = 0, + start_time: Optional[datetime] = None, + *, + raise_no_data: bool = False, ) -> None: """Update observation.""" obs = await self.get_stations_observations(limit, start_time=start_time) if obs: self._observation = obs self._metar_obs = [self.extract_metar(iobs) for iobs in self._observation] + elif raise_no_data: + raise NwsNoDataError("Observation received with no data.") - async def update_forecast(self: SimpleNWS) -> None: + async def update_forecast(self: SimpleNWS, *, raise_no_data: bool = False) -> None: """Update forecast.""" self._forecast = await self.get_gridpoints_forecast() + if not self.forecast and raise_no_data: + raise NwsNoDataError("Forecast received with no data.") - async def update_forecast_hourly(self: SimpleNWS) -> None: + async def update_forecast_hourly( + self: SimpleNWS, *, raise_no_data: bool = False + ) -> None: """Update forecast hourly.""" self._forecast_hourly = await self.get_gridpoints_forecast_hourly() + if not self.forecast_hourly and raise_no_data: + raise NwsNoDataError("Forecast hourly received with no data.") + + async def update_detailed_forecast( + self: SimpleNWS, *, raise_no_data: bool = False + ) -> None: + """Update forecast. + + Note: + `raise_no_data`currently can only be set to `False`. + """ + if raise_no_data: + raise NotImplementedError( + "raise_no_data=True not implemented for update_detailed_forecast" + ) - async def update_detailed_forecast(self: SimpleNWS) -> None: - """Update forecast.""" self._detailed_forecast = await self.get_detailed_forecast() @staticmethod @@ -253,22 +304,52 @@ def _new_alerts( current_alert_ids = self._unique_alert_ids(current_alerts) return [alert for alert in alerts if alert[ALERT_ID] not in current_alert_ids] - async def update_alerts_forecast_zone(self: SimpleNWS) -> List[Dict[str, Any]]: - """Update alerts zone.""" + async def update_alerts_forecast_zone( + self: SimpleNWS, *, raise_no_data: bool = False + ) -> List[Dict[str, Any]]: + """Update alerts zone. + + Note: + `raise_no_data`currently can only be set to `False`. + """ + if raise_no_data: + raise NotImplementedError( + "raise_no_data=True not implemented for update_alerts_forecast_zone" + ) alerts = await self.get_alerts_forecast_zone() new_alerts = self._new_alerts(alerts, self._alerts_forecast_zone) self._alerts_forecast_zone = alerts return new_alerts - async def update_alerts_county_zone(self: SimpleNWS) -> List[Dict[str, Any]]: - """Update alerts zone.""" + async def update_alerts_county_zone( + self: SimpleNWS, *, raise_no_data: bool = False + ) -> List[Dict[str, Any]]: + """Update alerts zone. + + Note: + `raise_no_data`currently can only be set to `False`. + """ + if raise_no_data: + raise NotImplementedError( + "raise_no_data=True not implemented for update_alerts_county_zone" + ) alerts = await self.get_alerts_county_zone() new_alerts = self._new_alerts(alerts, self._alerts_county_zone) self._alerts_county_zone = alerts return new_alerts - async def update_alerts_fire_weather_zone(self: SimpleNWS) -> List[Dict[str, Any]]: - """Update alerts zone.""" + async def update_alerts_fire_weather_zone( + self: SimpleNWS, *, raise_no_data: bool = False + ) -> List[Dict[str, Any]]: + """Update alerts zone. + + Note: + `raise_no_data`currently can only be set to `False`. + """ + if raise_no_data: + raise NotImplementedError( + "raise_no_data=True not implemented for update_alerts_fire_weather_zone" + ) alerts = await self.get_alerts_fire_weather_zone() new_alerts = self._new_alerts(alerts, self._alerts_fire_weather_zone) self._alerts_fire_weather_zone = alerts diff --git a/tests/fixtures/gridpoints_forecast_hourly_empty.json b/tests/fixtures/gridpoints_forecast_hourly_empty.json new file mode 100644 index 0000000..d80af21 --- /dev/null +++ b/tests/fixtures/gridpoints_forecast_hourly_empty.json @@ -0,0 +1,92 @@ +{ + "@context": [ + "https://raw.githubusercontent.com/geojson/geojson-ld/master/contexts/geojson-base.jsonld", + { + "wx": "https://api.weather.gov/ontology#", + "geo": "http://www.opengis.net/ont/geosparql#", + "unit": "http://codes.wmo.int/common/unit/", + "@vocab": "https://api.weather.gov/ontology#" + } + ], + "type": "Feature", + "geometry": { + "type": "GeometryCollection", + "geometries": [ + { + "type": "Point", + "coordinates": [ + -84.956046999999998, + 29.995017300000001 + ] + }, + { + "type": "Polygon", + "coordinates": [ + [ + [ + -84.968174099999999, + 30.007206 + ], + [ + -84.970116500000003, + 29.984511300000001 + ], + [ + -84.943922400000005, + 29.982827500000003 + ], + [ + -84.941974999999999, + 30.005522000000003 + ], + [ + -84.968174099999999, + 30.007206 + ] + ] + ] + } + ] + }, + "properties": { + "updated": "2019-10-14T23:16:24+00:00", + "units": "us", + "forecastGenerator": "HourlyForecastGenerator", + "generatedAt": "2019-10-15T00:12:54+00:00", + "updateTime": "2019-10-14T23:16:24+00:00", + "validTimes": "2019-10-14T17:00:00+00:00/P7DT20H", + "elevation": { + "value": 7.9248000000000003, + "unitCode": "unit:m" + }, + "periods": [ + { + "number": null, + "name": null, + "startTime": null, + "endTime": null, + "isDaytime": null, + "temperature": null, + "temperatureUnit": null, + "temperatureTrend": null, + "probabilityOfPrecipitation": { + "unitCode": null, + "value": null + }, + "dewpoint": { + "unitCode": null, + "value": null + }, + "relativeHumidity": { + "unitCode": null, + "value": null + }, + "windSpeed": null, + "windDirection": null, + "icon": null, + "shortForecast": null, + "detailedForecast": null + } + ] + } +} \ No newline at end of file diff --git a/tests/test_simple_nws.py b/tests/test_simple_nws.py index 057781b..4c740f1 100644 --- a/tests/test_simple_nws.py +++ b/tests/test_simple_nws.py @@ -5,7 +5,7 @@ from freezegun import freeze_time import pytest -from pynws import NwsError, SimpleNWS, call_with_retry +from pynws import NwsError, NwsNoDataError, SimpleNWS, call_with_retry from tests.helpers import setup_app LATLON = (0, 0) @@ -173,6 +173,24 @@ async def test_nws_observation_noprop(aiohttp_client, mock_urls): assert observation is None + with pytest.raises(NwsNoDataError, match="Observation received with no data"): + await nws.update_observation(raise_no_data=True) + + +async def test_nws_observation_noprop_w_retry(aiohttp_client, mock_urls): + app = setup_app( + stations_observations=[ + "stations_observations_noprop.json", + "stations_observations.json", + ] + ) + client = await aiohttp_client(app) + nws = SimpleNWS(*LATLON, USERID, client) + await nws.set_station(STATION) + + await call_with_retry(nws.update_observation, 0, 5, retry_no_data=True) + assert nws.observation is not None + async def test_nws_observation_missing_value(aiohttp_client, mock_urls): app = setup_app(stations_observations="stations_observations_missing_value.json") @@ -218,20 +236,32 @@ async def test_nws_forecast(aiohttp_client, mock_urls): assert forecast[1]["probabilityOfPrecipitation"] == 0 -@freeze_time("2019-10-14T21:30:00-04:00") async def test_nws_forecast_discard_stale(aiohttp_client, mock_urls): - app = setup_app() - client = await aiohttp_client(app) - nws = SimpleNWS(*LATLON, USERID, client, filter_forecast=True) - await nws.update_forecast_hourly() - forecast = nws.forecast_hourly - assert forecast[0]["temperature"] == 77 + with freeze_time("2019-10-14T21:30:00-04:00"): + app = setup_app() + client = await aiohttp_client(app) + nws = SimpleNWS(*LATLON, USERID, client, filter_forecast=True) + await nws.update_forecast_hourly() + forecast = nws.forecast_hourly + assert forecast[0]["temperature"] == 77 - nws = SimpleNWS(*LATLON, USERID, client, filter_forecast=False) - await nws.update_forecast_hourly() - forecast = nws.forecast_hourly + nws = SimpleNWS(*LATLON, USERID, client, filter_forecast=False) + await nws.update_forecast_hourly() + forecast = nws.forecast_hourly - assert forecast[0]["temperature"] == 78 + assert forecast[0]["temperature"] == 78 + + with freeze_time("2019-10-15T21:30:00-04:00"): + nws = SimpleNWS(*LATLON, USERID, client, filter_forecast=True) + await nws.update_forecast_hourly() + forecast = nws.forecast_hourly + assert forecast == [] + + nws = SimpleNWS(*LATLON, USERID, client, filter_forecast=True) + with pytest.raises( + NwsNoDataError, match="Forecast hourly received with no data" + ): + await nws.update_forecast_hourly(raise_no_data=True) @freeze_time("2019-10-14T20:30:00-04:00") @@ -275,6 +305,64 @@ async def test_nws_forecast_empty(aiohttp_client, mock_urls): assert forecast == [] +@freeze_time("2019-10-13T14:30:00-04:00") +async def test_nws_forecast_empty_raise(aiohttp_client, mock_urls): + app = setup_app(gridpoints_forecast="gridpoints_forecast_empty.json") + client = await aiohttp_client(app) + nws = SimpleNWS(*LATLON, USERID, client) + with pytest.raises(NwsNoDataError, match="Forecast received with no data"): + await nws.update_forecast(raise_no_data=True) + + +@freeze_time("2019-10-14T20:30:00-04:00") +async def test_nws_forecast_hourly_empty(aiohttp_client, mock_urls): + app = setup_app(gridpoints_forecast_hourly="gridpoints_forecast_hourly_empty.json") + client = await aiohttp_client(app) + nws = SimpleNWS(*LATLON, USERID, client) + await nws.update_forecast_hourly() + forecast_hourly = nws.forecast_hourly + + assert forecast_hourly == [] + + +@freeze_time("2019-10-14T20:30:00-04:00") +async def test_nws_forecast_hourly_empty_raise(aiohttp_client, mock_urls): + app = setup_app(gridpoints_forecast_hourly="gridpoints_forecast_hourly_empty.json") + client = await aiohttp_client(app) + nws = SimpleNWS(*LATLON, USERID, client) + with pytest.raises(NwsNoDataError, match="Forecast hourly received with no data"): + await nws.update_forecast_hourly(raise_no_data=True) + + +async def test_nws_unimplemented_retry_no_data(aiohttp_client, mock_urls): + app = setup_app(gridpoints_forecast="gridpoints_forecast_empty.json") + client = await aiohttp_client(app) + nws = SimpleNWS(*LATLON, USERID, client) + with pytest.raises( + NotImplementedError, + match="raise_no_data=True not implemented for update_detailed_forecast", + ): + await nws.update_detailed_forecast(raise_no_data=True) + + with pytest.raises( + NotImplementedError, + match="raise_no_data=True not implemented for update_alerts_forecast_zone", + ): + await nws.update_alerts_forecast_zone(raise_no_data=True) + + with pytest.raises( + NotImplementedError, + match="raise_no_data=True not implemented for update_alerts_county_zone", + ): + await nws.update_alerts_county_zone(raise_no_data=True) + + with pytest.raises( + NotImplementedError, + match="raise_no_data=True not implemented for update_alerts_fire_weather_zone", + ): + await nws.update_alerts_fire_weather_zone(raise_no_data=True) + + async def test_nws_alerts_forecast_zone(aiohttp_client, mock_urls): app = setup_app( alerts_active_zone=["alerts_active_zone.json", "alerts_active_zone_second.json"] @@ -356,10 +444,13 @@ async def test_nws_alerts_all_zones_second_alert(aiohttp_client, mock_urls): assert len(alerts) == 2 -async def test_retry_5xx(aiohttp_client, mock_urls): - with patch("pynws.simple_nws._is_500_error") as err_mock: +async def test_retry(aiohttp_client, mock_urls): + with patch("pynws.simple_nws._nws_retry_func") as err_mock: # retry all exceptions - err_mock.return_value = True + def _return_true(error): + return True + + err_mock.return_value = _return_true app = setup_app() client = await aiohttp_client(app) @@ -392,8 +483,13 @@ async def mock_wrap(*args, **kwargs): return await mock_update(*args, **kwargs) await call_with_retry(mock_wrap, 0, 5, "", test=None) + # raise_no_data is always included when called with retry + mock_update.assert_called_once_with("", raise_no_data=False, test=None) - mock_update.assert_called_once_with("", test=None) + mock_update.reset_mock() + await call_with_retry(mock_wrap, 0, 5, "", test=None, retry_no_data=True) + # retry_no_data will change raise_no_data + mock_update.assert_called_once_with("", raise_no_data=True, test=None) async def test_retry_invalid_args():