feat: get plan bulk with cache#10
feat: get plan bulk with cache#10tomerqodo wants to merge 4 commits intoaugment_full_base_feat_get_plan_bulk_with_cache_pr10from
Conversation
🤖 Augment PR SummarySummary: Adds a Redis-backed cache layer for bulk subscription plan retrieval to reduce load on the billing API. Changes:
Technical Notes: Cache entries are stored as JSON blobs in Redis and validated via Pydantic 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| plan: str | ||
| expiration_date: int | ||
|
|
||
| def __str__(self) -> str: |
There was a problem hiding this comment.
TypedDict definitions don’t support instance methods; adding __str__ here can raise a TypeError at import/class-creation time and prevent the module from loading.
🤖 Was this useful? React with 👍 or 👎
| logger.exception( | ||
| "get_plan_bulk: failed to validate subscription plan for tenant(%s)", tenant_id | ||
| ) | ||
| results[tenant_id] = None |
There was a problem hiding this comment.
get_plan_bulk() is annotated/documented as returning SubscriptionPlan values, but this stores None on validation failure; downstream code/tests expecting missing tenants (or dict-like plans) will break.
🤖 Was this useful? React with 👍 or 👎
| plans_to_cache: dict[str, SubscriptionPlan] = {} | ||
|
|
||
| for tenant_id, subscription_plan in bulk_plans.items(): | ||
| tenant_plans[tenant_id] = subscription_plan |
There was a problem hiding this comment.
get_plan_bulk_with_cache() copies subscription_plan values from get_plan_bulk() into tenant_plans (and then Redis); if any value is None, it will be serialized (e.g., as null) and later fail schema validation, causing repeated cache misses and potential None values in the returned mapping.
Other Locations
api/services/billing_service.py:376
🤖 Was this useful? React with 👍 or 👎
| """Clean up Redis cache before and after each test.""" | ||
| with flask_app_with_containers.app_context(): | ||
| # Clean up before test | ||
| yield |
There was a problem hiding this comment.
This fixture docstring says it cleans Redis “before and after”, but currently the cleanup happens only after yield; if any prior test left tenant_plan:* keys around, these tests can become order-dependent/flaky.
🤖 Was this useful? React with 👍 or 👎
3 participants
Benchmark PR from agentic-review-benchmarks#10