Skip to content

Handle Hygraph circuit breaker fallbacks#451

Merged
shayancoin merged 2 commits intomainfrom
codex/generate-codex-task-blocks-for-backlog-0ti7wf
Oct 20, 2025
Merged

Handle Hygraph circuit breaker fallbacks#451
shayancoin merged 2 commits intomainfrom
codex/generate-codex-task-blocks-for-backlog-0ti7wf

Conversation

@shayancoin
Copy link
Owner

@shayancoin shayancoin commented Oct 20, 2025

Summary

  • return cached Hygraph responses by reading CircuitOpenError.fallback_result and remove the duplicate handler
  • surface the service-unavailable status code via FastAPI constants when no fallback is available
  • expand sync route tests with Supabase auth fixtures and circuit breaker coverage for cached and uncached paths

Testing

  • pytest backend/tests/test_sync_routes_metrics.py -q

https://chatgpt.com/codex/tasks/task_e_68f5c02182b08330b908f6c62b14dc10

Summary by CodeRabbit

Release Notes

  • New Features

    • Legacy price preview requests now automatically redirect to the current API version.
  • Improvements

    • Enhanced circuit breaker resilience with improved fallback handling for service reliability.
    • Strengthened observability metrics for better service monitoring and health visibility.
    • Improved error handling and caching mechanisms throughout the system.

@vercel
Copy link

vercel bot commented Oct 20, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
paform Ready Ready Preview Comment Oct 20, 2025 9:09am

💡 Enable Vercel Agent with $100 free credit for automated AI reviews

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 20, 2025

Caution

Review failed

Failed to post review comments

Walkthrough

Refactors backend observability and services across multiple layers: introduces OpenTelemetry and Prometheus metrics integration with safe updates, restructures HygraphService for lazy initialization and dependency injection, adds legacy price endpoint redirects, replaces Material/Module models with ProjectQuote/SyncEvent, and updates supporting infrastructure (cache factory, security helpers).

Changes

Cohort / File(s) Summary
API Routing
backend/api/main.py
Adds RedirectResponse import; introduces optional loading guard for sync_router with None check; adds new public endpoint /api/price/{remaining:path} that redirects (308) to /api/v1/price/* via new redirect_legacy_price_paths() handler.
Metrics Refactoring
backend/api/metrics.py
Blends OpenTelemetry with Prometheus metrics; introduces _PROMETHEUS_HTTP_ENABLED flag and suppress_prometheus_http_metrics() context manager; adds public API ingest_observability_metric(), observe_http_request(), observe_lcp(); adds safe metric updaters _safe_add_counter(), _safe_record_histogram(); introduces record_hygraph_cb_trip() and record_hygraph_cb_fallback() for circuit-break events.
Database Models
backend/api/models.py
Removes Material and Module models; introduces ProjectQuote (with project_payload, currency, subtotal, tax, total, pdf_url, timestamps) and SyncEvent (with event_id, source, body_sha256, received_at, unique constraints); updates __all__ exports accordingly.
Hygraph Service
backend/services/hygraph_service.py
Introduces lazy singleton-like client/cache initialization (_CLIENT, _CACHE, _build_client, _get_client, _get_cache); refactors HygraphService with dependency injection (client, cache, cache_ttl_seconds parameters); converts _fetch_collection from classmethod to instance method; converts pull_materials/pull_modules/pull_systems to use default instance; adds _default_instance class variable; adjusts hygraph_primary_expr method signature.
Sync Routes
backend/api/routes_sync.py
Introduces public get_hygraph_service() factory with caching via _HYGRAPH_SERVICE_CACHE; replaces direct HygraphService usage with factory calls; updates circuit-breaker metrics to use record_hygraph_cb_trip() and record_hygraph_cb_fallback(); standardizes HTTP 503 status handling; adds service initialization before async tasks.
Cache Services
backend/services/cache.py
Adds backward-compatible CacheBackend alias for RedisCache; introduces public create_cache() factory function with url/namespace/use_memory_fallback parameters and intelligent fallback to default Redis URL.
Security
backend/api/security.py
Removes _reset_cached_write_token(); introduces _reset_write_token_cache(); adjusts _load_expected_write_token() to normalize API_WRITE_TOKEN when present.
Pricing Models
backend/models/pricing.py
Adds optional margin_floor_unit_price: Mapped[float | None] column to ModuleCatalog model.
Testing
backend/tests/test_routes_quote_cnc.py, backend/tests/test_sync_routes_metrics.py, backend/tests/test_security.py
test_routes_quote_cnc: adds redirect test for /api/price/preview/api/v1/price/preview (308); test_sync_routes_metrics: adds circuit-breaker tests, updates auth expectations (401→403), adds OTEL_SDK_DISABLED env var; test_security: updates imports to use renamed _reset_write_token_cache.
Dependencies
requirements.txt
Adds SQLAlchemy>=2.0,<3.0; prometheus-fastapi-instrumentator>=7.0,<8.0; opentelemetry-api>=1.28,<2.0; opentelemetry-sdk>=1.28,<2.0; opentelemetry-exporter-otlp>=1.28,<2.0; opentelemetry-instrumentation-fastapi>=0.49b0,<1.0; sentry-sdk[fastapi,httpx,opentelemetry]==2.42.0.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant API as /api/price/{path}
    participant Handler as redirect_legacy_price_paths()
    participant Response as RedirectResponse

    Client->>API: POST /api/price/preview
    API->>Handler: route to handler (remaining="preview")
    Handler->>Handler: compute target: /api/v1/price/preview
    Handler->>Handler: build full URL from request
    Handler->>Response: create RedirectResponse(status_code=308)
    Response->>Client: 308 Location: /api/v1/price/preview
    Client->>API: follow redirect to /api/v1/price/preview
Loading
sequenceDiagram
    participant Sync as routes_sync.py
    participant Factory as get_hygraph_service()
    participant Cache as _HYGRAPH_SERVICE_CACHE
    participant HG as HygraphService
    participant CB as CircuitBreaker

    Sync->>Factory: get_hygraph_service()
    Factory->>Cache: check cache key (webhook_secret)
    alt Cache hit
        Cache-->>Factory: return cached HygraphService
    else Cache miss
        Factory->>HG: HygraphService(client, cache, ttl)
        HG->>HG: lazy init _default instance
        Factory->>Cache: store service
    end
    Factory-->>Sync: return HygraphService instance
    
    Sync->>HG: pull_materials()
    HG->>CB: execute with circuit breaker
    alt Success
        CB-->>HG: return result
        Sync->>Sync: record_http_request()
    else CircuitOpen
        CB-->>HG: raise CircuitOpenError(fallback_result)
        Sync->>Sync: record_hygraph_cb_trip()
        Sync->>Sync: record_hygraph_cb_fallback()
        Sync->>Sync: emit observability metrics
    end
Loading
sequenceDiagram
    participant Metrics as observe_http_request()
    participant OTEL as OTEL HTTP Histogram
    participant Prom as Prometheus HTTP Histogram
    participant Safe as _safe_record_histogram()

    Metrics->>Safe: call with OTEL histogram
    Safe->>OTEL: histogram.record(value, attributes)
    alt Success
        OTEL-->>Safe: recorded
    else Error
        Safe->>Safe: log warning (skip)
    end
    
    Metrics->>Safe: call with Prometheus histogram
    Safe->>Prom: histogram.record(value, attributes)
    alt Success
        Prom-->>Safe: recorded
    else Error
        Safe->>Safe: log warning (skip)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Rationale: This PR introduces interconnected changes across critical subsystems—HygraphService undergoes a major architectural shift to dependency injection and singleton patterns; metrics layer integrates OpenTelemetry with Prometheus and adds safe-update helpers; models are replaced with new schemas; new routing/redirect logic and factory patterns are added. While individual changes are understandable, the coordination between Hygraph service refactoring, metrics callbacks, test assertions, and model dependencies requires careful cross-file reasoning to verify correctness and catch subtle initialization or caching edge cases.

Possibly related PRs

Poem

🐰 Hops through the metrics, bright and true,
With observables cached and services new,
Legacy paths now redirect with grace,
While circuits and models find their place. 🚀✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description provides relevant content covering what was changed and how to test it; however, it does not follow the required template structure. The template specifies three required sections: PR Type, Short Description, and Tests Added. The author's description is missing the PR Type section entirely and uses non-template section headings ("Summary" and "Testing" instead of "Short Description" and test details). While the content itself describes the changes and provides a test command, the structural deviation from the template means critical categorization information is absent. Add the PR Type section at the beginning (specifying whether this is a Feature, Fix, Documentation, or Other). Update the section headings to match the template: use "Short Description" instead of "Summary" for the main change description, and expand the "Tests Added" section to explicitly list or describe the new test functions added (e.g., test_pull_circuit_breaker_returns_cached_payload and test_pull_circuit_breaker_without_cache_returns_503).
Docstring Coverage ⚠️ Warning Docstring coverage is 23.81% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "Handle Hygraph circuit breaker fallbacks" directly aligns with the main objectives described in the PR. The changeset's primary focus is on reading CircuitOpenError.fallback_result to return cached Hygraph responses and handling service-unavailable scenarios, which the title accurately summarizes. The title is concise, specific, and clearly communicates the purpose without unnecessary noise. A developer reviewing commit history would immediately understand this change concerns Hygraph circuit breaker fallback handling.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/generate-codex-task-blocks-for-backlog-0ti7wf

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 20, 2025

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #453

coderabbitai bot added a commit that referenced this pull request Oct 20, 2025
…7wf`

Docstrings generation was requested by @shayancoin.

* #451 (comment)

The following files were modified:

* `backend/api/main.py`
* `backend/api/metrics.py`
* `backend/api/routes_sync.py`
* `backend/api/security.py`
* `backend/services/cache.py`
* `backend/services/hygraph_service.py`
* `backend/tests/test_routes_quote_cnc.py`
* `backend/tests/test_security.py`
* `backend/tests/test_sync_routes_metrics.py`
@shayancoin shayancoin merged commit 9fc8ccb into main Oct 20, 2025
4 of 10 checks passed
shayancoin added a commit that referenced this pull request Oct 20, 2025
…7wf` (#453)

Docstrings generation was requested by @shayancoin.

* #451 (comment)

The following files were modified:

* `backend/api/main.py`
* `backend/api/metrics.py`
* `backend/api/routes_sync.py`
* `backend/api/security.py`
* `backend/services/cache.py`
* `backend/services/hygraph_service.py`
* `backend/tests/test_routes_quote_cnc.py`
* `backend/tests/test_security.py`
* `backend/tests/test_sync_routes_metrics.py`

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Shayan <shayan@coin.link>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant