Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions src/accessiweather/notifications/notification_event_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,15 +225,28 @@ def _check_discussion_update(
"""
if not issuance_time:
# No issuance time available (non-US location or API issue)
logger.debug(
"_check_discussion_update: no issuance_time (non-US location or fetch failed) "
"— skipping"
)
return None

# First time seeing discussion - store but don't notify
if self.state.last_discussion_issuance_time is None:
self.state.last_discussion_issuance_time = issuance_time
self.state.last_discussion_text = discussion_text
logger.debug("First discussion issuance time stored: %s", issuance_time)
logger.debug(
"_check_discussion_update: first-run — stored issuance_time=%s, no notification",
issuance_time,
)
return None

logger.debug(
"_check_discussion_update: last=%s current=%s",
self.state.last_discussion_issuance_time,
issuance_time,
)

# Check if issuance time is newer (discussion was updated)
if issuance_time > self.state.last_discussion_issuance_time:
logger.info(
Expand All @@ -249,7 +262,7 @@ def _check_discussion_update(
self.state.last_discussion_text = discussion_text

issued_label = (
issuance_time.strftime("%-I:%M %p")
issuance_time.strftime("%I:%M %p").lstrip("0")
if hasattr(issuance_time, "strftime")
else str(issuance_time)
)
Expand All @@ -265,6 +278,10 @@ def _check_discussion_update(
)

self.state.last_discussion_text = discussion_text
logger.debug(
"_check_discussion_update: issuance_time unchanged (%s) — no notification",
issuance_time,
)
return None

def _check_severe_risk_change(
Expand Down
22 changes: 18 additions & 4 deletions src/accessiweather/weather_client_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -424,12 +424,18 @@ async def get_notification_event_data(self, location: Location) -> WeatherData:

try:
if self._is_us_location(location):
discussion_task = asyncio.create_task(
self._get_nws_forecast_and_discussion(location)
)
# Use discussion-only fetch so a forecast API failure can never
# silently suppress AFD update notifications.
discussion_task = asyncio.create_task(self._get_nws_discussion_only(location))
alerts_task = asyncio.create_task(self._get_nws_alerts(location))
forecast, discussion, discussion_issuance_time = await discussion_task
discussion, discussion_issuance_time = await discussion_task
alerts = await alerts_task
logger.debug(
"get_notification_event_data: discussion=%s issuance=%s alerts=%s",
"ok" if discussion else "None",
discussion_issuance_time,
len(alerts.alerts) if alerts and alerts.alerts else 0,
)
weather_data.discussion = discussion
weather_data.discussion_issuance_time = discussion_issuance_time
weather_data.alerts = alerts or WeatherAlerts(alerts=[])
Expand Down Expand Up @@ -1053,6 +1059,14 @@ async def _get_nws_forecast_and_discussion(
location, self.nws_base_url, self.user_agent, self.timeout, self._get_http_client()
)

async def _get_nws_discussion_only(
self, location: Location
) -> tuple[str | None, datetime | None]:
"""Fetch only the NWS AFD discussion (no forecast). Used by the notification path."""
return await nws_client.get_nws_discussion_only(
location, self.nws_base_url, self.user_agent, self.timeout, self._get_http_client()
)

async def _get_nws_alerts(self, location: Location) -> WeatherAlerts | None:
"""Delegate to the NWS client module."""
alert_radius_type = getattr(self.settings, "alert_radius_type", "county")
Expand Down
117 changes: 106 additions & 11 deletions src/accessiweather/weather_client_nws.py
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,9 @@ async def get_nws_forecast_and_discussion(
"""
Fetch forecast and discussion from the NWS API for the given location.

Forecast and discussion fetches are independent: if the forecast fetch fails,
the discussion is still returned (and vice versa).

Returns:
Tuple of (forecast, discussion_text, discussion_issuance_time)

Expand All @@ -591,40 +594,67 @@ async def get_nws_forecast_and_discussion(

# Use provided client or create a new one
if client is not None:
# Fetch grid data if not provided
# Fetch grid data if not provided (needed by both forecast and discussion)
if grid_data is None:
grid_url = f"{nws_base_url}/points/{location.latitude},{location.longitude}"
response = await _client_get(client, grid_url, headers=headers)
response.raise_for_status()
grid_data = response.json()

forecast_url = grid_data["properties"]["forecast"]
response = await _client_get(client, forecast_url, headers=feature_headers)
response.raise_for_status()
forecast_data = response.json()
# Fetch forecast independently so a failure doesn't kill the discussion
parsed_forecast: Forecast | None = None
try:
forecast_url = grid_data["properties"]["forecast"]
response = await _client_get(client, forecast_url, headers=feature_headers)
response.raise_for_status()
parsed_forecast = parse_nws_forecast(response.json())
except Exception as forecast_exc: # noqa: BLE001
logger.warning(
"Forecast fetch failed (discussion will still be returned): %s", forecast_exc
)

discussion, discussion_issuance_time = await get_nws_discussion(
client, headers, grid_data, nws_base_url
)
logger.debug(
"get_nws_forecast_and_discussion: forecast=%s discussion_len=%s issuance=%s",
"ok" if parsed_forecast else "None",
len(discussion) if discussion else 0,
discussion_issuance_time,
)

return parsed_forecast, discussion, discussion_issuance_time

return parse_nws_forecast(forecast_data), discussion, discussion_issuance_time
grid_url = f"{nws_base_url}/points/{location.latitude},{location.longitude}"

async with httpx.AsyncClient(timeout=timeout, follow_redirects=True) as new_client:
response = await new_client.get(grid_url, headers=headers)
response.raise_for_status()
grid_data = response.json()

forecast_url = grid_data["properties"]["forecast"]
response = await new_client.get(forecast_url, headers=feature_headers)
response.raise_for_status()
forecast_data = response.json()
# Fetch forecast independently so a failure doesn't kill the discussion
parsed_forecast = None
try:
forecast_url = grid_data["properties"]["forecast"]
response = await new_client.get(forecast_url, headers=feature_headers)
response.raise_for_status()
parsed_forecast = parse_nws_forecast(response.json())
except Exception as forecast_exc: # noqa: BLE001
logger.warning(
"Forecast fetch failed (discussion will still be returned): %s", forecast_exc
)

discussion, discussion_issuance_time = await get_nws_discussion(
new_client, headers, grid_data, nws_base_url
)
logger.debug(
"get_nws_forecast_and_discussion: forecast=%s discussion_len=%s issuance=%s",
"ok" if parsed_forecast else "None",
len(discussion) if discussion else 0,
discussion_issuance_time,
)

return parse_nws_forecast(forecast_data), discussion, discussion_issuance_time
return parsed_forecast, discussion, discussion_issuance_time

except Exception as exc: # noqa: BLE001
logger.error(f"Failed to get NWS forecast and discussion: {exc}")
Expand All @@ -633,6 +663,71 @@ async def get_nws_forecast_and_discussion(
return None, None, None


@async_retry_with_backoff(max_attempts=3, base_delay=1.0, timeout=20.0)
async def get_nws_discussion_only(
location: Location,
nws_base_url: str,
user_agent: str,
timeout: float,
client: httpx.AsyncClient | None = None,
) -> tuple[str | None, datetime | None]:
"""
Fetch only the NWS Area Forecast Discussion for a location.

Lighter-weight than get_nws_forecast_and_discussion — skips the forecast
fetch entirely. Used by the notification event path so that a transient
forecast API error never silently suppresses AFD update notifications.

Returns:
Tuple of (discussion_text, discussion_issuance_time).
Returns (None, None) on unrecoverable error.

"""
try:
headers = {"User-Agent": user_agent}
logger.debug(
"get_nws_discussion_only: fetching grid data for %s,%s",
location.latitude,
location.longitude,
)

if client is not None:
grid_url = f"{nws_base_url}/points/{location.latitude},{location.longitude}"
response = await _client_get(client, grid_url, headers=headers)
response.raise_for_status()
grid_data = response.json()
discussion, issuance_time = await get_nws_discussion(
client, headers, grid_data, nws_base_url
)
logger.debug(
"get_nws_discussion_only: discussion_len=%s issuance=%s",
len(discussion) if discussion else 0,
issuance_time,
)
return discussion, issuance_time

grid_url = f"{nws_base_url}/points/{location.latitude},{location.longitude}"
async with httpx.AsyncClient(timeout=timeout, follow_redirects=True) as new_client:
response = await new_client.get(grid_url, headers=headers)
response.raise_for_status()
grid_data = response.json()
discussion, issuance_time = await get_nws_discussion(
new_client, headers, grid_data, nws_base_url
)
logger.debug(
"get_nws_discussion_only: discussion_len=%s issuance=%s",
len(discussion) if discussion else 0,
issuance_time,
)
return discussion, issuance_time

except Exception as exc: # noqa: BLE001
logger.error("Failed to fetch NWS discussion only: %s", exc)
if isinstance(exc, RETRYABLE_EXCEPTIONS) or is_retryable_http_error(exc):
raise
return None, None


async def get_nws_discussion(
client: httpx.AsyncClient,
headers: dict[str, str],
Expand Down
Loading
Loading