feat: get plan bulk with cache#10
feat: get plan bulk with cache#10tomerqodo wants to merge 4 commits intocodex_full_base_feat_get_plan_bulk_with_cache_pr10from
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70fe873c2a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| except Exception: | ||
| 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.
When TypeAdapter.validate_python fails, the code logs and still inserts None into results. That violates the dict[str, SubscriptionPlan] contract and will surface as runtime errors for callers that do plan["plan"] or similar. It also means get_plan_bulk_with_cache will cache null for those tenants, so subsequent cache hits still fail validation and trigger repeated API calls. This happens whenever the billing API returns malformed data (e.g., missing expiration_date). Consider omitting the tenant from results (as the new unit test expects) or filtering out None before returning/caching.
Useful? React with 👍 / 👎.
3 participants
Benchmark PR from agentic-review-benchmarks#10