diff --git a/src/sample_python_app/app/lifecycle.py b/src/sample_python_app/app/lifecycle.py index 309c643..24ee248 100644 --- a/src/sample_python_app/app/lifecycle.py +++ b/src/sample_python_app/app/lifecycle.py @@ -15,7 +15,7 @@ def start_metrics_server(port: int) -> None: """Start the Prometheus metrics server on the specified port.""" if _port_in_use(port): - logger.error("Port %s already in use; metrics disabled", port) + logger.error(f"Port {port} already in use; metrics disabled") return logger.info(f"Starting Prometheus metrics on 0.0.0.0:{port}") diff --git a/src/sample_python_app/app/runner.py b/src/sample_python_app/app/runner.py index cf58275..2a71db3 100644 --- a/src/sample_python_app/app/runner.py +++ b/src/sample_python_app/app/runner.py @@ -3,17 +3,12 @@ Handles fetching, validation, and display of astronomical data. """ -import json import time from datetime import date -import httpx -from pydantic import ValidationError - from sample_python_app.core import ( FETCH_COUNTER, FETCH_DURATION, - FETCH_ERRORS, setup_logger, weather_settings, ) @@ -41,30 +36,31 @@ def __init__(self, client: CustomHTTPClient) -> None: self.client = client def fetch(self, *, exit_on_error: bool = True) -> None: - """Fetch astronomical data and display if not already displayed today.""" + """Fetch astronomical data and display if not already shown today.""" lat = weather_settings.LOCATION.latitude lon = weather_settings.LOCATION.longitude - logger.info(f"Using latitude={lat} longitude={lon}") + + logger.info("Using latitude=%s longitude=%s", lat, lon) + start = time.time() + try: astro = fetch_astronomical_data_from_api(lat, lon, client=self.client) forecast = fetch_hourly_forecast_from_api(lat, lon, client=self.client) FETCH_COUNTER.inc() - except ( - httpx.HTTPStatusError, - httpx.RequestError, - ValidationError, - json.JSONDecodeError, - AppError, - ) as exc: - self._handle_fetch_error(exc, exit_on_error) + except AppError as exc: + logger.error("Weather fetch failed: %s", exc) + if exit_on_error: + raise SystemExit(1) from None return finally: FETCH_DURATION.observe(time.time() - start) - today_str = date.today().isoformat() - if self._last_displayed_day != today_str: + + today = date.today().isoformat() + + if self._last_displayed_day != today: display_astronomical_data(astro, forecast) - self._last_displayed_day = today_str + self._last_displayed_day = today def reset_display(self): """Reset the last displayed day so display will occur again.""" @@ -82,22 +78,6 @@ def close(self) -> None: except Exception: logger.exception("Error closing HTTP client in AstroFetcher") - def _handle_fetch_error(self, exc: Exception, exit_on_error: bool) -> None: - FETCH_ERRORS.inc() - if isinstance(exc, httpx.HTTPStatusError): - logger.error("HTTP status error: %s", exc) - elif isinstance(exc, httpx.RequestError): - logger.error("Network error: %s", exc) - elif isinstance(exc, ValidationError): - logger.error("Validation error: %s", exc) - elif isinstance(exc, json.JSONDecodeError): - logger.error("JSON decode error: %s", exc) - else: - logger.exception("Unexpected error") - if exit_on_error: - raise SystemExit(1) from exc - raise AppError(str(exc)) from exc - runner_client = CustomHTTPClient( headers=weather_settings.WEATHER_HEADERS, diff --git a/src/sample_python_app/core/__init__.py b/src/sample_python_app/core/__init__.py index 8ffe538..957cd6a 100644 --- a/src/sample_python_app/core/__init__.py +++ b/src/sample_python_app/core/__init__.py @@ -6,6 +6,9 @@ FETCH_COUNTER, FETCH_DURATION, FETCH_ERRORS, + HTTP_REQUEST_DURATION, + HTTP_REQUEST_EXCEPTIONS, + HTTP_REQUESTS, ) __all__ = [ @@ -16,4 +19,7 @@ "FETCH_COUNTER", "FETCH_DURATION", "FETCH_ERRORS", + "HTTP_REQUESTS", + "HTTP_REQUEST_EXCEPTIONS", + "HTTP_REQUEST_DURATION", ] diff --git a/src/sample_python_app/core/metrics.py b/src/sample_python_app/core/metrics.py index 625fda0..f695aa3 100644 --- a/src/sample_python_app/core/metrics.py +++ b/src/sample_python_app/core/metrics.py @@ -14,10 +14,27 @@ "fetch_duration_seconds", "Duration of astronomical data fetches in seconds", ) - +HTTP_REQUESTS = Counter( + "http_requests_total", + "Total HTTP requests made", + ["method", "host", "path", "status_code"], +) +HTTP_REQUEST_EXCEPTIONS = Counter( + "http_request_exceptions_total", + "Total HTTP request exceptions", + ["method", "host", "path", "exception_type"], +) +HTTP_REQUEST_DURATION = Histogram( + "http_request_duration_seconds", + "HTTP request latency in seconds", + ["method", "host", "path"], +) __all__ = [ "FETCH_COUNTER", "FETCH_ERRORS", "FETCH_DURATION", + "HTTP_REQUESTS", + "HTTP_REQUEST_EXCEPTIONS", + "HTTP_REQUEST_DURATION", "start_http_server", ] diff --git a/src/sample_python_app/exceptions/__init__.py b/src/sample_python_app/exceptions/__init__.py index 345200e..5157985 100644 --- a/src/sample_python_app/exceptions/__init__.py +++ b/src/sample_python_app/exceptions/__init__.py @@ -2,6 +2,9 @@ from sample_python_app.exceptions.custom import ( AppError, + HTTPTimeoutError, + NetworkError, + ServiceError, ) -__all__ = ["AppError"] +__all__ = ["AppError", "HTTPTimeoutError", "NetworkError", "ServiceError"] diff --git a/src/sample_python_app/exceptions/custom.py b/src/sample_python_app/exceptions/custom.py index f5f1bff..2c59043 100644 --- a/src/sample_python_app/exceptions/custom.py +++ b/src/sample_python_app/exceptions/custom.py @@ -1,6 +1,38 @@ """Custom exceptions for the sample_python_app project.""" +class HTTPTimeoutError(Exception): + """Raised when an HTTP request times out.""" + + def __init__(self, message: str) -> None: + """Initialize the HTTPTimeoutError with a message.""" + super().__init__(message) + + +class NetworkError(Exception): + """Raised when a network-level error occurs. + + (connection refused, DNS failure, etc.). + """ + + def __init__(self, message: str) -> None: + """Initialize the NetworkError with a message.""" + super().__init__(message) + + +class ServiceError(Exception): + """Raised when the HTTP response returns an error status (4xx or 5xx).""" + + def __init__(self, status_code: int, body: str | None = None) -> None: + """Initialize the ServiceError with status code and optional response body.""" + self.status_code = status_code + self.body = body + msg = f"Service returned status {status_code}" + if body: + msg += f": {body}" + super().__init__(msg) + + class AppError(Exception): """Base exception for the application.""" diff --git a/src/sample_python_app/services/data_loader.py b/src/sample_python_app/services/data_loader.py index fe9b6cc..7438cfe 100644 --- a/src/sample_python_app/services/data_loader.py +++ b/src/sample_python_app/services/data_loader.py @@ -1,10 +1,12 @@ -"""Handles loading and validating weather.gov astronomical data from file and API.""" +"""Handles loading and validating weather.gov astronomical data.""" from __future__ import annotations from pydantic import ValidationError from sample_python_app.core import setup_logger, weather_settings +from sample_python_app.exceptions import NetworkError, ServiceError +from sample_python_app.exceptions.custom import AppError from sample_python_app.models import ( AstronomicalData, ForecastFeature, @@ -12,138 +14,89 @@ ) from sample_python_app.services.http_client import CustomHTTPClient +logger = setup_logger(mode="silent") + weather_client = CustomHTTPClient( - headers=weather_settings.WEATHER_HEADERS, base_url=weather_settings.WEATHER_API_BASE + headers=weather_settings.WEATHER_HEADERS, + base_url=weather_settings.WEATHER_API_BASE, ) def resolve_point_metadata( - lat: float, lon: float, client: CustomHTTPClient + lat: float, lon: float, client: CustomHTTPClient | None = None ) -> WeatherGovFeature: - """Resolve and return the WeatherGovFeature for the given lat/lon. - - This calls the `/points/{lat},{lon}` endpoint and returns the - validated `WeatherGovFeature` model. Other functions can call this - to discover grid coordinates or forecast URLs. - """ - logger = setup_logger(mode="silent") - api_client = client or weather_client - points_path = f"/points/{lat},{lon}" - logger.info( - "Resolving point metadata for coordinates %s,%s: %s", lat, lon, points_path - ) - points_data = api_client.get_json(points_path) - point_model = WeatherGovFeature.model_validate(points_data) - logger.info( - "Resolved point metadata: grid=%s x=%s y=%s", - point_model.properties.grid_id, - point_model.properties.grid_x, - point_model.properties.grid_y, - ) - return point_model - + """Resolve grid metadata for coordinates.""" + client = client or weather_client + path = f"/points/{lat},{lon}" -def fetch_astronomical_data_from_api( - lat: float, lon: float, client: CustomHTTPClient -) -> AstronomicalData: - """Fetch and validate astronomical data from weather.gov API for given coordinates. + try: + data = client.get_json(path) + return WeatherGovFeature.model_validate(data) - Args: - lat (float): Latitude of the location. - lon (float): Longitude of the location. - client (HTTPClient, optional): HTTP client to use for requests. If - not provided the module-level `weather_client` will be used. + except (NetworkError, ServiceError) as exc: + raise AppError("Failed to resolve point metadata") from exc - Returns: - AstronomicalData: Validated astronomical data from API response. + except ValidationError as exc: + raise AppError("Invalid metadata returned from weather service") from exc - Raises: - ValidationError: If the API response fails validation. - httpx.HTTPError: If the API request fails. - """ - logger = setup_logger(mode="silent") - points_path = f"/points/{lat},{lon}" +def fetch_astronomical_data_from_api( + lat: float, + lon: float, + client: CustomHTTPClient | None = None, +) -> AstronomicalData: + """Fetch astronomical data for coordinates.""" client = client or weather_client - logger.info("Fetching astronomical data from: %s", points_path) + path = f"/points/{lat},{lon}" + try: - data = client.get_json(points_path) + data = client.get_json(path) model = WeatherGovFeature.model_validate(data) - astro = model.properties.astronomical_data - logger.info("AstronomicalData fetched and validated from API.") - return astro - except ValidationError as e: - logger.error("Data validation error: %s", e) - raise - - -def fetch_hourly_forecast_from_api( - lat: float, lon: float, client: CustomHTTPClient -) -> ForecastFeature: - """Fetch hourly forecast Feature for the given coordinates. - - This function first queries the `/points/{lat},{lon}` endpoint to resolve - the appropriate grid URL, then requests the hourly forecast and validates - it against `ForecastFeature`. + return model.properties.astronomical_data - Args: - lat (float): Latitude of the location. - lon (float): Longitude of the location. - client (HTTPClient, optional): HTTP client to use for requests. If - not provided the module-level `weather_client` will be used. + except (NetworkError, ServiceError) as exc: + raise AppError("Weather API request failed") from exc - Returns: - ForecastFeature: Parsed and validated hourly forecast feature. + except ValidationError as exc: + raise AppError( + "Invalid astronomical data returned from weather service" + ) from exc - Raises: - httpx.HTTPError: If an HTTP request fails. - ValidationError: If the response fails pydantic validation. - """ - logger = setup_logger(mode="silent") +def fetch_hourly_forecast_from_api( + lat: float, + lon: float, + client: CustomHTTPClient | None = None, +) -> ForecastFeature: + """Fetch hourly forecast for coordinates.""" client = client or weather_client - # Resolve point metadata to find the hourly forecast URL - point_model = resolve_point_metadata(lat, lon, client=client) - forecast_url = point_model.properties.forecast_hourly - logger.info("Fetching hourly forecast from: %s", forecast_url) - - # forecast_url is often an absolute URL returned by the API; httpx - # client with a base_url accepts absolute URLs as well, so pass through. - forecast_data = client.get_json(forecast_url) - - forecast_model = ForecastFeature.model_validate(forecast_data) - logger.info("Hourly forecast fetched and validated.") - return forecast_model + try: + point = resolve_point_metadata(lat, lon, client) + forecast_url = point.properties.forecast_hourly + data = client.get_json(forecast_url) + return ForecastFeature.model_validate(data) + except (NetworkError, ServiceError) as exc: + raise AppError("Weather API request failed") from exc + except ValidationError as exc: + raise AppError("Invalid forecast returned from weather service") from exc def fetch_hourly_forecast_by_grid( - grid_id: str, grid_x: int, grid_y: int, client: CustomHTTPClient + grid_id: str, + grid_x: int, + grid_y: int, + client: CustomHTTPClient | None = None, ) -> ForecastFeature: - """Fetch hourly forecast Feature directly from grid coordinates. - - Builds the gridpoints URL (for example `HGX/59,98`) and fetches the - hourly forecast Feature at - `/gridpoints/{grid_id}/{grid_x},{grid_y}/forecast/hourly`. - - Args: - grid_id: Grid identifier (e.g. "HGX"). - grid_x: Grid X coordinate (e.g. 59). - grid_y: Grid Y coordinate (e.g. 98). - client (HTTPClient, optional): HTTP client to use for requests. If - not provided the module-level `weather_client` will be used. - - Returns: - ForecastFeature: Parsed and validated hourly forecast feature. - - """ - logger = setup_logger(mode="silent") + """Fetch hourly forecast using grid coordinates.""" client = client or weather_client path = f"/gridpoints/{grid_id}/{grid_x},{grid_y}/forecast/hourly" - logger.info("Fetching hourly forecast by grid from: %s", path) - data = client.get_json(path) - model = ForecastFeature.model_validate(data) - logger.info("Hourly forecast (grid) fetched and validated.") - return model + try: + data = client.get_json(path) + return ForecastFeature.model_validate(data) + except (NetworkError, ServiceError) as exc: + raise AppError("Weather API request failed") from exc + except ValidationError as exc: + raise AppError("Invalid forecast returned from weather service") from exc diff --git a/src/sample_python_app/services/http_client.py b/src/sample_python_app/services/http_client.py index 8246a4e..9d805bc 100644 --- a/src/sample_python_app/services/http_client.py +++ b/src/sample_python_app/services/http_client.py @@ -1,29 +1,25 @@ -"""Shared HTTP client wrapper for service modules. - -Provides a thin wrapper around `httpx.Client` so callers can share -common headers (like `User-Agent`) at instantiation and use a single -session for multiple requests. -""" +"""HTTP client abstraction for making requests with metrics and error handling.""" from __future__ import annotations +import time from typing import Any import httpx -from sample_python_app.core import setup_logger +from sample_python_app.core import ( + HTTP_REQUEST_DURATION, + HTTP_REQUEST_EXCEPTIONS, + HTTP_REQUESTS, +) +from sample_python_app.core.logging import logger +from sample_python_app.exceptions import HTTPTimeoutError, NetworkError, ServiceError -logger = setup_logger(mode="silent") +JSONType = dict[str, object] | list[dict[str, object]] class CustomHTTPClient: - """Simple HTTP client wrapper around httpx.Client. - - Args: - headers: Optional default headers to include on each request. - timeout: Request timeout in seconds. - - """ + """HTTP client wrapper around httpx.Client.""" def __init__( self, @@ -31,49 +27,100 @@ def __init__( timeout: float = 10.0, base_url: str | None = None, ) -> None: - """Initialize the HTTP client wrapper. + """Initialize the CustomHTTPClient with optional headers, timeout, and base URL. Args: - headers: Optional default headers to include on each request. - timeout: Request timeout in seconds. - base_url: Optional base URL for relative request paths. + headers (dict, optional): HTTP headers to include in requests. + timeout (float, optional): Request timeout in seconds. Defaults to 10.0. + base_url (str, optional): Base URL for all requests. Defaults to None. """ - self.headers = headers or {"User-Agent": "(milsman2, milsman2@gmail.com)"} - client_kwargs: dict = {"headers": self.headers, "timeout": timeout} - if base_url is not None: + client_kwargs: dict[str, Any] = { + "headers": headers or {"User-Agent": "sample-python-app"}, + "timeout": timeout, + } + + if base_url: client_kwargs["base_url"] = base_url + self._client = httpx.Client(**client_kwargs) + self._host_label = "weather_api" + self._last_response: httpx.Response | None = None - def get(self, url: str, **kwargs: Any) -> httpx.Response: - """Perform a GET request and return the Response. + def request_json(self, method: str, url: str, **kwargs: Any) -> JSONType: + """Perform HTTP request and return parsed JSON.""" + path = url if url.startswith("/") else "/" + url + start_time = time.time() - Accepts either absolute URLs or relative paths when a `base_url` is set. - """ - logger.info("HTTP GET %s", url) - return self._client.get(url, **kwargs) + try: + response = self._client.request(method, url, **kwargs) + self._last_response = response + except httpx.TimeoutException as exc: + HTTP_REQUEST_EXCEPTIONS.labels( + method=method, + host=self._host_label, + path=path, + exception_type="TimeoutException", + ).inc() + raise HTTPTimeoutError(str(exc)) from exc + except httpx.RequestError as exc: + HTTP_REQUEST_EXCEPTIONS.labels( + method=method, + host=self._host_label, + path=path, + exception_type=type(exc).__name__, + ).inc() + raise NetworkError(str(exc)) from exc + + duration = time.time() - start_time + + HTTP_REQUEST_DURATION.labels( + method=method, + host=self._host_label, + path=path, + ).observe(duration) + + HTTP_REQUESTS.labels( + method=method, + host=self._host_label, + path=path, + status_code=str(response.status_code), + ).inc() + + full_url = str(response.request.url) if hasattr(response, "request") else url + logger.info( + f"HTTP {method} {full_url} responded with status code " + f"{response.status_code}" + ) + + if not response.is_success: + logger.error(f"HTTP error status code: {response.status_code}") + logger.debug(f"HTTP error response body: {response.text}") + raise ServiceError( + status_code=response.status_code, + body=response.text, + ) - def get_json(self, url: str, **kwargs: Any) -> Any: - """GET the given URL and return the parsed JSON body. + try: + return response.json() + except ValueError as exc: + raise ServiceError( + status_code=response.status_code, + body="Invalid JSON response", + ) from exc - Raises an HTTPStatusError on non-2xx responses. - """ - # Accept either absolute URLs or relative paths when base_url is set. - resp = self.get(url, **kwargs) - resp.raise_for_status() - return resp.json() + def get_json(self, url: str, **kwargs: Any) -> JSONType: + """Return parsed JSON from a GET request.""" + return self.request_json("GET", url, **kwargs) def close(self) -> None: - """Close the underlying httpx client.""" - try: - self._client.close() - except Exception: - logger.exception("Error closing HTTP client") + """Close the underlying HTTP client.""" + self._client.close() def __enter__(self) -> CustomHTTPClient: - """Context manager enter returns the client instance.""" + """Support context manager entry.""" return self - def __exit__(self, exc_type, exc, tb) -> None: # type: ignore[override] - """Context manager exit closes the underlying client.""" + def __exit__(self, *args: Any) -> None: + """Support context manager exit by closing the client.""" self.close()