Skip to content

Conversation

@hezhefly
Copy link

新增功能:为 sandbox 模块的添加了S3上传功能
代码参考:参考oss_storage.py;
依赖说明:新增boto3(AWS S3 官方 SDK)作为基础依赖

 - 在Settings类中添加了S3配置选项
- 更新了FILE_SYSTEM字面量
  - 添加了S3相关的配置字段
  - 更新了FILE_SYSTEM字面量以包含"s3"选项
  - 添加了验证逻辑确保S3配置正确
update SandboxManagerEnvConfig params
add `S3Storage` class
@rayrayraykk rayrayraykk self-requested a review October 13, 2025 04:35
Copy link
Member

@rayrayraykk rayrayraykk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see the inline comments, thanks!

@rayrayraykk rayrayraykk requested a review from Copilot October 16, 2025 08:56
Copy link
Contributor

Copilot AI left a 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 adds S3 support to the sandbox module's file storage system, expanding storage options beyond the existing local and OSS systems.

Key changes:

  • Added S3 as a new file system option alongside existing local and OSS storage
  • Implemented complete S3Storage class with upload/download functionality and MD5-based file change detection
  • Extended configuration models to include S3-specific settings (endpoint, credentials, bucket, region)

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/agentscope_runtime/sandbox/model/manager_config.py Added S3 configuration fields and validation logic
src/agentscope_runtime/sandbox/manager/storage/s3_storage.py New S3Storage implementation with boto3 integration
src/agentscope_runtime/sandbox/manager/storage/init.py Added S3Storage to module exports
src/agentscope_runtime/sandbox/manager/server/config.py Added S3 settings to server configuration
src/agentscope_runtime/sandbox/manager/server/app.py Integrated S3 configuration parameters
src/agentscope_runtime/sandbox/manager/sandbox_manager.py Added S3 storage initialization logic
Comments suppressed due to low confidence (1)

src/agentscope_runtime/sandbox/manager/storage/s3_storage.py:1

  • The validation logic doesn't include s3_endpoint_url in the required fields check, but it's marked as required in the field description. Consider adding it to the validation or updating the field description to clarify when it's optional.
# -*- coding: utf-8 -*-

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@rayrayraykk
Copy link
Member

@hezhefly
Copy link
Author

@rayrayraykk 您好!已完成所有评审建议的修复:

  1. 移除了 s3_storage.py 中不必要的 sys.path 修改;
  2. 重构了 S3 配置字段的验证逻辑,提升可读性;
  3. 优化了 ETag 格式判断,避免 multipart 上传场景下的文件对比错误;
  4. 在 pyproject.toml 中添加了 boto3 依赖;
  5. 修复了 Linter 报错,通过 pre-commit 检查;
  6. 解决了与 main 分支的冲突(pyproject.toml、app.py)。
    请帮忙再次评审,谢谢!

Copy link
Member

@rayrayraykk rayrayraykk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please update the tutorial in Cookbook accordingly, thanks!

@rayrayraykk
Copy link
Member

And the pre-commit check still failed: https://github.com/agentscope-ai/agentscope-runtime/actions/runs/18590217929/job/53231232759?pr=113 . Please follow the instruction to format your code: https://runtime.agentscope.io/zh/contribute.html#id7 (e.g., pre-commit run --all-files).

hezhefly and others added 4 commits October 28, 2025 21:43
在高级沙箱配置文档中新增S3存储支持的配置说明和环境变量
统一字符串引号使用双引号并优化代码格式
修复S3存储实现中的格式问题和注释换行
优化文件上传下载逻辑的代码可读性
@hezhefly
Copy link
Author

Could you please update the tutorial in Cookbook accordingly, thanks!

done!

@hezhefly
Copy link
Author

And the pre-commit check still failed: https://github.com/agentscope-ai/agentscope-runtime/actions/runs/18590217929/job/53231232759?pr=113 . Please follow the instruction to format your code: https://runtime.agentscope.io/zh/contribute.html#id7 (e.g., pre-commit run --all-files).

done!

@cla-assistant
Copy link

cla-assistant bot commented Dec 2, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.

❌ Weicheng WC4 He
❌ hezhefly


Weicheng WC4 He seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants