Add lightweight /v1/approved-apps/marketplace endpoint#4288
Add lightweight /v1/approved-apps/marketplace endpoint#4288smian1 wants to merge 1 commit intoBasedHardware:mainfrom
Conversation
Add a new endpoint that returns only the 15 fields needed by the web frontend, excluding large fields like external_integration (~3MB) to keep response size under Next.js 2MB cache limit. This reduces response size from ~2.5MB to ~500KB, fixing cache warnings and 429 rate limiting during Next.js builds.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new lightweight endpoint, /v1/approved-apps/marketplace, to provide a smaller data payload for the web frontend, which is a great optimization to stay within Next.js's cache limits. The implementation is straightforward and aligns with existing patterns in the router. My review includes one high-severity suggestion to improve the maintainability and robustness of the new endpoint by leveraging Pydantic's serialization capabilities instead of manually constructing a dictionary.
|
|
||
| return [ | ||
| normalize_app_numeric_fields({ | ||
| 'id': app.id, | ||
| 'name': app.name, | ||
| 'description': app.description, | ||
| 'author': app.author, | ||
| 'image': app.image, | ||
| 'category': app.category, | ||
| 'capabilities': list(app.capabilities), | ||
| 'installs': app.installs, | ||
| 'rating_avg': app.rating_avg, | ||
| 'rating_count': app.rating_count, | ||
| 'created_at': app.created_at, | ||
| 'is_paid': app.is_paid, | ||
| 'price': app.price, | ||
| 'payment_plan': app.payment_plan, | ||
| 'is_popular': app.is_popular, | ||
| }) | ||
| for app in filtered_apps | ||
| ] |
There was a problem hiding this comment.
Manually creating a dictionary of app fields is brittle and hard to maintain. If the App model is updated in the future, this endpoint could easily become outdated or miss new fields, leading to potential bugs.
To make this more robust and maintainable, it's better to leverage Pydantic's serialization capabilities. Using model_dump with an include set ensures that you're explicitly selecting the fields you need, making the code cleaner and less prone to errors.
fields_to_include = {
'id', 'name', 'description', 'author', 'image', 'category',
'capabilities', 'installs', 'rating_avg', 'rating_count', 'created_at',
'is_paid', 'price', 'payment_plan', 'is_popular',
}
return [
normalize_app_numeric_fields(
app.model_dump(mode='json', include=fields_to_include)
)
for app in filtered_apps
]There was a problem hiding this comment.
Pull request overview
This PR adds a new lightweight /v1/approved-apps/marketplace endpoint specifically optimized for the web marketplace. The endpoint returns only 15 essential fields needed by the frontend, explicitly excluding the large external_integration field (~3MB) that causes the original /v1/approved-apps endpoint to exceed Next.js's 2MB cache limit.
Changes:
- New GET endpoint
/v1/approved-apps/marketplacethat returns a filtered subset of app fields - Reduces response payload from ~2.5MB to ~500KB by excluding unnecessary fields like
external_integration - Maintains backward compatibility by keeping the original
/v1/approved-appsendpoint unchanged
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @router.get('/v1/approved-apps/marketplace', tags=['v1']) | ||
| def get_marketplace_apps(): |
There was a problem hiding this comment.
The endpoint lacks a response_model parameter in the decorator. While the existing /v1/approved-apps endpoint uses response_model=List[AppBaseModel], this new endpoint returns a custom dict structure. Consider either: 1) creating a dedicated Pydantic model for the marketplace response (e.g., MarketplaceApp) and using it as response_model, or 2) documenting why response_model is intentionally omitted if the custom dict structure is preferred.
|
@beastoin Here's what was tested: Endpoint Verification:
Response Size:
Pattern Compliance:
Pending:
|
|
@mdmohsin7 can you review. |
|
Hey @smian1 👋 Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request. After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:
Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out. Thank you for being part of the Omi community! 💜 |
Summary
/v1/approved-apps/marketplaceendpoint that returns only 15 fields needed by web frontendexternal_integrationfield (~3MB) to keep response under Next.js 2MB cache limitWhy
The current
/v1/approved-appsendpoint returns ~2.5MB of data, exceeding Next.js's 2MB cache limit. This causes cache warnings and 429 rate limiting during builds.Test plan