-
Notifications
You must be signed in to change notification settings - Fork 2
feat: implement VLM Run Python SDK client #21
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
Co-Authored-By: Sudeep Pillai <[email protected]>
- Add Files class with list/upload/retrieve/delete methods - Add Models class with list method - Add FineTuning class with create/list/retrieve/cancel/events methods - Mirror together-python implementation structure Co-Authored-By: Sudeep Pillai <[email protected]>
- Add APIRequestor class with tenacity-based retry mechanism - Implement exponential backoff for failed requests - Add comprehensive error handling - Set appropriate timeout values and constants Co-Authored-By: Sudeep Pillai <[email protected]>
- Add Files, Models, and FineTuning resource initialization - Mark old methods as deprecated with migration guidance - Add proper dot notation access (client.files.list(), etc.) - Add base_url and timeout configuration Co-Authored-By: Sudeep Pillai <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
⚙️ Control Options:
|
Co-Authored-By: Sudeep Pillai <[email protected]>
vlmrun/client/base_requestor.py
Outdated
| def __init__( | ||
| self, | ||
| message: str, | ||
| http_status: Optional[int] = None, |
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.
Use int | None instead of Optional
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.
Use | None annotations for all keyword arguments.
Co-Authored-By: Sudeep Pillai <[email protected]>
vlmrun/client/base_requestor.py
Outdated
| def __init__( | ||
| self, | ||
| message: str, | ||
| http_status: Optional[int] = None, |
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.
Use | None annotations for all keyword arguments.
vlmrun/client/files.py
Outdated
| class Files: | ||
| """Files resource for VLM Run API.""" | ||
|
|
||
| def __init__(self, client) -> None: |
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.
annotate the client client: Client
vlmrun/client/types.py
Outdated
| id: str | ||
| name: str | ||
| domain: str | ||
| created_at: Optional[datetime] = None |
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.
Use | None instead of Optional
Co-Authored-By: Sudeep Pillai <[email protected]>
spillai
left a comment
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 up the docstrings (remove simple ones as they're obvious), and improve the overall devex for the client API to be extremely high-quality.
Add stubs for a test_client.py that takes in different variants (with and without API key, base url) to make sure that the variables are either loaded from env. var or used as provided by the user.
vlmrun/client/client.py
Outdated
|
|
||
| api_key: Optional[str] = None | ||
|
|
||
| base_url: str = "https://api.vlm.run/v1" |
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.
Set this to None by default, and initialize it in __post_init__ by getting VLMRUN_BASE_URL just like VLMRUN_API_KEY. Default to https://api.vlm.run/v1 if it's not set.
vlmrun/client/client.py
Outdated
| # TODO: Implement API call | ||
| return [] | ||
|
|
||
| """List all files (deprecated). |
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.
Remove these simple docstrings.
- Add base_url environment variable fallback - Update docstrings to be more concise and helpful - Add comprehensive test coverage for client initialization - Remove deprecated method docstrings Co-Authored-By: Sudeep Pillai <[email protected]>
- Set default base_url to https://api.vlm.run/v1 - Only use environment value when default is unchanged - Clean up test environment in client tests Co-Authored-By: Sudeep Pillai <[email protected]>
- Make base_url default to None and use environment fallback - Clean up test environment in all client tests - Fix test_client_missing_api_key environment handling Co-Authored-By: Sudeep Pillai <[email protected]>
|
Also the CI is populate with a |
- Check for both None and empty string in API key validation - Only use environment base URL if default hasn't been overridden - Improve docstring clarity around base URL precedence Co-Authored-By: Sudeep Pillai <[email protected]>
- Update test assertions to clarify /v1 suffix handling - Improve test comments for URL handling behavior - Ensure consistent base URL behavior across tests Co-Authored-By: Sudeep Pillai <[email protected]>
VLM Run Python SDK Client Implementation
This PR implements the VLM Run Python SDK client following the together-python client structure.
Changes
Testing
Link to Devin run: https://app.devin.ai/sessions/ce09cb448f6244a69cb687dd7b8e76fd