-
Notifications
You must be signed in to change notification settings - Fork 759
integration: add tripo3d provider #1296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well-structured Tripo3D provider implementation with comprehensive API coverage. Found one type safety issue in getTask.ts that could cause runtime errors.
src/providers/tripo3d/getTask.ts
Outdated
| 'unknown' | ||
| 'banned' | ||
| 'expired'; | ||
input: any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐛 Bug Fix
Issue: The input
field is typed as any
, which eliminates type safety and could lead to runtime errors when accessing properties
Fix: Define a proper interface for the input structure based on the task types
Impact: Prevents potential runtime crashes and improves code maintainability
input: any; | |
input: Record<string, unknown>; |
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use Matter AICommand List
|
@dd-eg-user as discussed, would you be able to add |
Happy to take a look! Could you point me to an example where this is done today? https://github.com/search?q=repo%3APortkey-AI%2Fgateway+credits_used&type=code yields no results. Tripo3D doesn't really have a notion of
because 1) you probably want cost per task and there could be multiple tasks running simultaneously affecting balance 2) multiple requests per API key in between T0 and T2 would also alter balance making it incorrect. We could try to embed the pricing table into this provider, but it can get out of date quickly and we also have to put in logic as to what is meant by things like https://platform.tripo3d.ai/docs/schema and https://platform.tripo3d.ai/docs/billing |
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use Matter AICommand List
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @dd-eg-user
since we do not have a unified signature for these routes,
I'd suggest only keeping the api.ts file and removing the transforms in src/providers/tripo3d
this would allow gateway to act as auth layer and allow passthrough proxy requests
so you index file would simply become
import { ProviderConfigs } from '../types';
import Tripo3DAPIConfig from './api';
const Tripo3DConfig: ProviderConfigs = {
api: Tripo3DAPIConfig,
};
export default Tripo3DConfig;
you can remove the changes in src/providers/types.ts
as well
so finally the only required files are
src/providers/tripo3d/api.ts
, src/providers/tripo3d/index.ts
, globals.ts
Thanks for the review. How does this comment and #1296 (comment) work together? The pricing is only relevant on one endpoint. If we remove those endpoints and acts as a passthrough, not sure how we'd add pricing information to the response. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clean refactor to use gatewayRequestURL for better path extraction. The implementation correctly handles the /v1 prefix removal.
src/providers/tripo3d/api.ts
Outdated
getEndpoint: ({ gatewayRequestURL }) => { | ||
// For passthrough proxy, extract path after /v1 | ||
return gatewayRequestURL.split('/v1')[1] || ''; | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Code Refactor
Issue: The split operation could fail if gatewayRequestURL doesn't contain '/v1', potentially causing runtime errors
Fix: Add validation to ensure the URL contains '/v1' before splitting
Impact: Prevents potential crashes when processing malformed URLs
getEndpoint: ({ gatewayRequestURL }) => { | |
// For passthrough proxy, extract path after /v1 | |
return gatewayRequestURL.split('/v1')[1] || ''; | |
}, | |
getEndpoint: ({ gatewayRequestURL }) => { | |
// For passthrough proxy, extract path after /v1 | |
const parts = gatewayRequestURL.split('/v1'); | |
return parts.length > 1 ? parts[1] : ''; | |
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Resolved: Original URL splitting logic completely removed and replaced with empty string return
Simple refactor replacing complex URL parsing logic with empty string return. Change appears intentional for simplified endpoint handling. |
Add comprehensive Tripo3D provider integration with support for: - Task creation (text_to_model, image_to_model, etc.) - Task status polling - File uploads and STS token management - Account balance checking Implements all Tripo3D API endpoints from their OpenAPI schema.
- Changes input field type from 'any' to 'Record<string, any>' - Adds documentation explaining why this type is used - Maintains flexibility while providing better type safety - Addresses feedback about eliminating 'any' usage
- Remove custom transforms and endpoint configs as requested in review - Keep only api.ts for authentication layer functionality - Revert endpoint string additions to types.ts - Delete unnecessary transform files (createTask, getTask, upload, balance, pricing) - Provider now acts as pure passthrough proxy like Sagemaker
- Change getEndpoint to use providerPath instead of hardcoded fn cases - Fixes TypeScript errors after removing endpoint strings from types
- Replace providerPath with gatewayRequestURL.split('/v1')[1] - Matches pattern used by other passthrough providers like Sagemaker - Fixes TypeScript error in getEndpoint function
- Change from gatewayRequestURL.split('/v1')[1] to empty string return - Matches pattern used by other simple passthrough providers like Replicate - Addresses review comment about incorrect getEndpoint implementation
d1a3373
to
51405a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simple endpoint configuration change looks good. The hardcoded empty string approach is valid for this provider's architecture.
headers: ({ providerOptions }) => { | ||
return { Authorization: `Bearer ${providerOptions.apiKey}` }; | ||
}, | ||
getEndpoint: ({ fn }) => '', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Code Refactor
Issue: The getEndpoint
function now returns a hardcoded empty string, which may not be intuitive for future maintainers
Fix: Add a comment explaining why an empty string is returned for this provider
Impact: Improves code maintainability and reduces confusion for developers
getEndpoint: ({ fn }) => '', | |
getEndpoint: ({ fn }) => '', // Tripo3D uses base URL without additional path |
Description
Adds comprehensive Tripo3D provider integration to the Portkey AI Gateway, enabling users to route 3D model generation requests through Portkey to Tripo3D's API. This implementation provides full support for all Tripo3D endpoints
including task creation, status polling, file uploads, and account management.
Motivation
Tripo3D is a leading 3D generation platform that converts text prompts and images into high-quality 3D models. Adding Tripo3D as a provider expands Portkey's capabilities into the 3D generation space, allowing users to leverage Portkey's
reliability features (retries, fallbacks, load balancing) with Tripo3D's powerful 3D AI models.
Type of Change
How Has This Been Tested?
Screenshots (if applicable)
Checklist
Related Issues