-
Notifications
You must be signed in to change notification settings - Fork 178
Feature/s3 #1375
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
Feature/s3 #1375
Conversation
Removed boto3/botocore dependencies and the CosClient, replacing direct COS upload logic with a new API-based approach using presigned URLs. Moved experiment state update logic to a dedicated API module and updated all usages. Refactored Client to remove COS logic, simplified session handling, and updated uploader and callback code to use the new APIs. Updated imports and tests to reflect file moves and API changes.
Added buffer.seek(0) before uploading to COS to ensure the buffer pointer is at the start. Also removed unused COS upload tests and related imports from test_client.py for code cleanup.
Summary of ChangesHello @SAKURA-CAT, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在增强SwanLab SDK的对象存储兼容性,通过将对象存储签名过程转移到后端,实现对多种私有对象存储解决方案的支持。这一改动涉及核心客户端的重构、引入新的API层来处理实验状态和文件上传,并清理了不再需要的客户端对象存储依赖和相关代码,从而提升了系统的灵活性和可扩展性。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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 Review
本次PR的核心改动是将对象存储的签名和上传逻辑从SDK端转移到了后端。现在SDK会从后端获取预签名URL,然后直接上传文件。这大大简化了SDK的逻辑,移除了对boto3等库的依赖,是一个很好的重构。
代码结构更加清晰,将API调用相关的函数移动到了新的swanlab/core_python/api目录中,例如update_experiment_state和upload_to_cos。
我发现了一个在文件上传重试逻辑中的潜在bug,可能会导致重试失败或上传不完整的文件。除此之外,整体改动看起来很不错。
Relocated the buffer.seek(0) call from upload_to_cos to the _upload function to ensure the buffer is reset before uploading. This centralizes buffer handling and prevents redundant seeks.
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.
Pull request overview
This PR refactors the object storage upload mechanism to use backend-signed URLs instead of client-side signing with AWS credentials. This architectural change removes the dependency on boto3/botocore libraries and simplifies the client by delegating signing responsibilities to the backend server, which is important for supporting multiple object storage providers in private deployments.
Key Changes
- Replaced client-side AWS credential-based signing (boto3) with backend pre-signed URL generation
- Refactored client architecture by extracting session management and API endpoints into separate modules
- Updated experiment state management to use a dedicated API function instead of client methods
Reviewed changes
Copilot reviewed 11 out of 13 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
swanlab/core_python/api/service.py |
New service API implementing backend-signed COS uploads with presigned URLs |
swanlab/core_python/api/experiment.py |
New API module for experiment state updates |
swanlab/core_python/client/session.py |
Extracted session creation logic from client module |
swanlab/core_python/client/__init__.py |
Removed COS client logic, cleaned up API methods, simplified client structure |
swanlab/core_python/client/cos.py |
Deleted boto3-based COS client implementation |
swanlab/core_python/uploader/upload.py |
Updated to use new upload_to_cos function and renamed decorator |
swanlab/data/porter/__init__.py |
Updated to use new experiment state API |
swanlab/data/callbacker/cloud.py |
Updated to use new experiment state API |
swanlab/core_python/auth/providers/api_key.py |
Updated import path for session module |
swanlab/core_python/__init__.py |
Removed create_session export (now in client submodule) |
test/unit/core_python/test_session.py |
Updated import path and removed trailing whitespace |
test/unit/core_python/test_client.py |
Removed COS tests and updated exception type assertion |
requirements.txt |
Removed boto3 and botocore dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Enhanced the file upload function with retry logic and exponential backoff for robustness against network errors. Added comprehensive unit tests to verify upload success, retry behavior, exception handling, and server error scenarios.
Enhanced the upload_to_cos function to better handle exceptions during concurrent uploads. Failed uploads are now logged with warnings and retried, with errors on retry attempts also logged for improved reliability and debugging.
|
/gemini review |
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 Review
这个 Pull Request 主要重构了对象存储的实现,将签名过程移至后端,客户端通过获取预签名 URL 的方式上传文件。这使得客户端不再需要依赖 boto3,简化了依赖管理和客户端逻辑。相关的代码也进行了优化和拆分,例如将文件上传和实验状态更新的逻辑分别封装到了 swanlab/core_python/api/service.py 和 swanlab/core_python/api/experiment.py 中,使得 Client 类的职责更单一,代码结构更清晰。整体来看,这是一个很好的重构,提高了代码的可维护性和扩展性。我在代码中发现了一些可以改进的地方,主要涉及重试策略的实现和代码注释的清晰度,具体请看我的评论。
Changed the retry delay in upload_file to use exponential backoff (2^(attempt-1) seconds) instead of linear. Also removed a redundant comment in upload_to_cos for clarity.
Deleted the import of get_host_api and related code that handled URL prefixing for private environments, as it is no longer needed.
为了实现私有化后端对接多种对象存储,现将对象存储签名改为后端签名
同时优化了API部分的代码,为 #1369 做准备