From 2678d7746f8f827913b822310e861102efbe5bc9 Mon Sep 17 00:00:00 2001 From: Miles Kane Date: Tue, 3 Mar 2026 22:38:33 -0600 Subject: [PATCH 1/2] feat: Refactor HTTP client usage and remove auto-merge workflow --- .github/workflows/auto-merge-on-release.yml | 75 --------------- .github/workflows/ci-cd.yaml | 10 -- src/sample_python_app/app/runner.py | 46 +++++++-- src/sample_python_app/core/config.py | 2 + src/sample_python_app/services/data_loader.py | 96 ++++++++++++------- src/sample_python_app/services/http_client.py | 79 +++++++++++++++ src/sample_python_app/ui/synthwave.py | 6 -- 7 files changed, 183 insertions(+), 131 deletions(-) delete mode 100644 .github/workflows/auto-merge-on-release.yml create mode 100644 src/sample_python_app/services/http_client.py diff --git a/.github/workflows/auto-merge-on-release.yml b/.github/workflows/auto-merge-on-release.yml deleted file mode 100644 index a4113c4..0000000 --- a/.github/workflows/auto-merge-on-release.yml +++ /dev/null @@ -1,75 +0,0 @@ -name: Auto-merge reusable workflow - -on: - workflow_call: - inputs: - head_branch: - description: 'Branch name to find PR for' - required: false - type: string - -permissions: - contents: write - pull-requests: write - -jobs: - auto-merge: - runs-on: ubuntu-latest - steps: - - name: Auto-merge matching PR - uses: actions/github-script@v8 - with: - github-token: ${{ secrets.GITHUB_TOKEN }} - script: | - // Use the provided input; caller should supply `head_branch` when invoking. - const head_branch = "${{ inputs.head_branch }}"; - const owner = context.repo.owner; - const repo = context.repo.repo; - - if (!head_branch) { - core.info('No head_branch provided; nothing to do.'); - return; - } - - core.info(`Auto-merge triggered for branch: ${head_branch}`); - - const prs = await github.rest.pulls.list({ - owner, - repo, - head: `${owner}:${head_branch}`, - state: 'open', - }); - - if (!prs.data || prs.data.length === 0) { - core.info(`No open pull request found for branch ${head_branch}`); - return; - } - - const pr = prs.data[0]; - - if (pr.draft) { - core.info(`Pull request #${pr.number} is a draft; skipping merge.`); - return; - } - - try { - const res = await github.rest.pulls.merge({ - owner, - repo, - pull_number: pr.number, - merge_method: 'merge', - commit_title: head_branch, - commit_message: '', - }); - - if (res.data.merged) { - core.info(`Successfully merged PR #${pr.number} (branch ${head_branch}).`); - } else { - core.info(`Merge attempt returned: ${JSON.stringify(res.data)}`); - } - } catch (err) { - core.setFailed(`Failed to merge PR #${pr.number}: ${err}`); - } - - - name: Done - run: echo "auto-merge finished" diff --git a/.github/workflows/ci-cd.yaml b/.github/workflows/ci-cd.yaml index d498850..0e1aa24 100644 --- a/.github/workflows/ci-cd.yaml +++ b/.github/workflows/ci-cd.yaml @@ -42,13 +42,3 @@ jobs: permissions: contents: write secrets: inherit - - trigger-auto-merge: - needs: [release, docker-build-and-image-scan] - uses: milsman2/python-app-template/.github/workflows/auto-merge-on-release.yml@main - permissions: - contents: write - pull-requests: write - with: - head_branch: ${{ github.ref_name }} - secrets: inherit diff --git a/src/sample_python_app/app/runner.py b/src/sample_python_app/app/runner.py index d96807a..cf58275 100644 --- a/src/sample_python_app/app/runner.py +++ b/src/sample_python_app/app/runner.py @@ -22,17 +22,23 @@ fetch_astronomical_data_from_api, fetch_hourly_forecast_from_api, ) +from sample_python_app.services.http_client import CustomHTTPClient from sample_python_app.ui import display_astronomical_data logger = setup_logger("normal") class AstroFetcher: - """Fetches astronomical data and displays only once per day.""" + """Fetches astronomical data and displays only once per day. - def __init__(self) -> None: - """Initialize the AstroFetcher with no last displayed day.""" + Accepts an `HTTPClient` to use for all outbound requests so the + runner can own the client's lifecycle and tests can inject mocks. + """ + + def __init__(self, client: CustomHTTPClient) -> None: + """Initialize the AstroFetcher with an optional HTTP client.""" self._last_displayed_day: str | None = None + self.client = client def fetch(self, *, exit_on_error: bool = True) -> None: """Fetch astronomical data and display if not already displayed today.""" @@ -41,8 +47,8 @@ def fetch(self, *, exit_on_error: bool = True) -> None: logger.info(f"Using latitude={lat} longitude={lon}") start = time.time() try: - astro = fetch_astronomical_data_from_api(lat, lon) - forecast = fetch_hourly_forecast_from_api(lat, lon) + 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, @@ -64,6 +70,18 @@ def reset_display(self): """Reset the last displayed day so display will occur again.""" self._last_displayed_day = None + def close(self) -> None: + """Close the associated HTTP client if present. + + This allows the runner to delegate shutdown responsibility to the + fetcher when it owns the client's lifecycle. + """ + if hasattr(self, "client") and self.client is not None: + try: + self.client.close() + 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): @@ -81,4 +99,20 @@ def _handle_fetch_error(self, exc: Exception, exit_on_error: bool) -> None: raise AppError(str(exc)) from exc -fetcher = AstroFetcher() +runner_client = CustomHTTPClient( + headers=weather_settings.WEATHER_HEADERS, + base_url=weather_settings.WEATHER_API_BASE, +) +fetcher = AstroFetcher(client=runner_client) + + +def shutdown_runner() -> None: + """Shutdown helper to close long-lived resources owned by the runner. + + Call this from application shutdown hooks to ensure the HTTP client is + properly closed and connections are released. + """ + try: + fetcher.close() + except Exception: + logger.exception("Error during runner shutdown") diff --git a/src/sample_python_app/core/config.py b/src/sample_python_app/core/config.py index 2b0d7ca..1aea43f 100644 --- a/src/sample_python_app/core/config.py +++ b/src/sample_python_app/core/config.py @@ -11,6 +11,8 @@ class WeatherSettings(BaseSettings): """Weather-related settings.""" LOCATION: Coordinate = Coordinate(Latitude(29.8469), Longitude(-95.4689)) + WEATHER_API_BASE: str = "https://api.weather.gov" + WEATHER_HEADERS: dict = {"User-Agent": "(milsman2, milsman2@gmail.com)"} model_config = SettingsConfigDict( env_file=".env", diff --git a/src/sample_python_app/services/data_loader.py b/src/sample_python_app/services/data_loader.py index 5a6f897..fe9b6cc 100644 --- a/src/sample_python_app/services/data_loader.py +++ b/src/sample_python_app/services/data_loader.py @@ -1,22 +1,58 @@ """Handles loading and validating weather.gov astronomical data from file and API.""" -import httpx +from __future__ import annotations + from pydantic import ValidationError -from sample_python_app.core import setup_logger +from sample_python_app.core import setup_logger, weather_settings from sample_python_app.models import ( AstronomicalData, ForecastFeature, WeatherGovFeature, ) +from sample_python_app.services.http_client import CustomHTTPClient + +weather_client = CustomHTTPClient( + headers=weather_settings.WEATHER_HEADERS, base_url=weather_settings.WEATHER_API_BASE +) + +def resolve_point_metadata( + lat: float, lon: float, client: CustomHTTPClient +) -> WeatherGovFeature: + """Resolve and return the WeatherGovFeature for the given lat/lon. -def fetch_astronomical_data_from_api(lat: float, lon: float) -> AstronomicalData: + 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 + + +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. 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. Returns: AstronomicalData: Validated astronomical data from API response. @@ -27,24 +63,23 @@ def fetch_astronomical_data_from_api(lat: float, lon: float) -> AstronomicalData """ logger = setup_logger(mode="silent") - url = f"https://api.weather.gov/points/{lat},{lon}" - headers = {"User-Agent": "(milsman2, milsman2@gmail.com)"} - logger.info(f"Fetching astronomical data from URL: {url}") - logger.info(f"Request headers: {headers}") + points_path = f"/points/{lat},{lon}" + client = client or weather_client + logger.info("Fetching astronomical data from: %s", points_path) try: - response = httpx.get(url, headers=headers) - response.raise_for_status() - data = response.json() + data = client.get_json(points_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(f"Data validation error: {e}") + logger.error("Data validation error: %s", e) raise -def fetch_hourly_forecast_from_api(lat: float, lon: float) -> ForecastFeature: +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 @@ -54,6 +89,8 @@ def fetch_hourly_forecast_from_api(lat: float, lon: float) -> ForecastFeature: 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. Returns: ForecastFeature: Parsed and validated hourly forecast feature. @@ -64,22 +101,16 @@ def fetch_hourly_forecast_from_api(lat: float, lon: float) -> ForecastFeature: """ logger = setup_logger(mode="silent") - headers = {"User-Agent": "(milsman2, milsman2@gmail.com)"} + client = client or weather_client # Resolve point metadata to find the hourly forecast URL - points_url = f"https://api.weather.gov/points/{lat},{lon}" - logger.info(f"Resolving grid for coordinates {lat},{lon}: {points_url}") - resp = httpx.get(points_url, headers=headers) - resp.raise_for_status() - points_data = resp.json() - - point_model = WeatherGovFeature.model_validate(points_data) + point_model = resolve_point_metadata(lat, lon, client=client) forecast_url = point_model.properties.forecast_hourly - logger.info(f"Fetching hourly forecast from: {forecast_url}") + logger.info("Fetching hourly forecast from: %s", forecast_url) - resp2 = httpx.get(forecast_url, headers=headers) - resp2.raise_for_status() - forecast_data = resp2.json() + # 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.") @@ -87,7 +118,7 @@ def fetch_hourly_forecast_from_api(lat: float, lon: float) -> ForecastFeature: def fetch_hourly_forecast_by_grid( - grid_id: str, grid_x: int, grid_y: int + grid_id: str, grid_x: int, grid_y: int, client: CustomHTTPClient ) -> ForecastFeature: """Fetch hourly forecast Feature directly from grid coordinates. @@ -99,22 +130,19 @@ def fetch_hourly_forecast_by_grid( 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") - headers = {"User-Agent": "(milsman2, milsman2@gmail.com)"} + client = client or weather_client - url = ( - "https://api.weather.gov/gridpoints/ " - f"{grid_id}/{grid_x},{grid_y}/forecast/hourly" - ) - logger.info(f"Fetching hourly forecast by grid from: {url}") - resp = httpx.get(url, headers=headers) - resp.raise_for_status() - data = resp.json() + 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.") diff --git a/src/sample_python_app/services/http_client.py b/src/sample_python_app/services/http_client.py new file mode 100644 index 0000000..8246a4e --- /dev/null +++ b/src/sample_python_app/services/http_client.py @@ -0,0 +1,79 @@ +"""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. +""" + +from __future__ import annotations + +from typing import Any + +import httpx + +from sample_python_app.core import setup_logger + +logger = setup_logger(mode="silent") + + +class CustomHTTPClient: + """Simple HTTP client wrapper around httpx.Client. + + Args: + headers: Optional default headers to include on each request. + timeout: Request timeout in seconds. + + """ + + def __init__( + self, + headers: dict | None = None, + timeout: float = 10.0, + base_url: str | None = None, + ) -> None: + """Initialize the HTTP client wrapper. + + Args: + headers: Optional default headers to include on each request. + timeout: Request timeout in seconds. + base_url: Optional base URL for relative request paths. + + """ + 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["base_url"] = base_url + self._client = httpx.Client(**client_kwargs) + + def get(self, url: str, **kwargs: Any) -> httpx.Response: + """Perform a GET request and return the Response. + + 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) + + def get_json(self, url: str, **kwargs: Any) -> Any: + """GET the given URL and return the parsed JSON body. + + 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 close(self) -> None: + """Close the underlying httpx client.""" + try: + self._client.close() + except Exception: + logger.exception("Error closing HTTP client") + + def __enter__(self) -> CustomHTTPClient: + """Context manager enter returns the client instance.""" + return self + + def __exit__(self, exc_type, exc, tb) -> None: # type: ignore[override] + """Context manager exit closes the underlying client.""" + self.close() diff --git a/src/sample_python_app/ui/synthwave.py b/src/sample_python_app/ui/synthwave.py index 916ba98..598f757 100644 --- a/src/sample_python_app/ui/synthwave.py +++ b/src/sample_python_app/ui/synthwave.py @@ -58,9 +58,3 @@ def synthwave_dashboard( ) console.print(layout) - # Also print a plain-text summary of events so tests that capture - # stdout can assert on labels like "Sunrise" and "Astronomical Twilight Begin". - times = astro.formatted(settings.tz, "%I:%M %p %Z") - for name, val in times.items(): - label = name.replace("_", " ").title() - console.print(f"{label}: {val}") From 8480bb952537415b963bd6d251cdd8de3b0b60dc Mon Sep 17 00:00:00 2001 From: Miles Kane Date: Tue, 3 Mar 2026 22:44:19 -0600 Subject: [PATCH 2/2] feat: Enhance display function with logging and summary of astronomical events --- src/sample_python_app/ui/display.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/sample_python_app/ui/display.py b/src/sample_python_app/ui/display.py index 003a677..9e0f418 100644 --- a/src/sample_python_app/ui/display.py +++ b/src/sample_python_app/ui/display.py @@ -1,6 +1,6 @@ """Handles formatting and displaying astronomical data using rich and pyfiglet.""" -from sample_python_app.core.config import settings +from sample_python_app.core import settings, setup_logger from sample_python_app.models import AstronomicalData, ForecastFeature from .synthwave import synthwave_dashboard @@ -11,4 +11,20 @@ def display_astronomical_data(astro: AstronomicalData, forecast: ForecastFeature Combined synthwave dashboard will be shown. """ + logger = setup_logger("normal") + # Log plain-text summary for testability + tz = settings.tz + time_fmt = "%I:%M %p %Z" + events = astro.formatted(tz, time_fmt) + summary_keys = [ + "sunrise", + "sunset", + "astronomical_twilight_begin", + "astronomical_twilight_end", + ] + for key in summary_keys: + value = events.get(key) + if value: + label = key.replace("_", " ").title() + logger.info(f"{label}: {value}") synthwave_dashboard(astro, forecast, settings)