Conversation
hittyt
requested review from
Generalwin,
Pangjiping,
jwx0925 and
ninan-nn
as code owners
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 908726f4b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def _validate_credentials(self, credentials: dict[str, Any]) -> None: | ||
| base_url = credentials.get("opensandbox_base_url", "") | ||
| api_key = credentials.get("opensandbox_api_key", "") | ||
| if not base_url or not api_key: | ||
| raise ToolProviderCredentialValidationError("Missing OpenSandbox base URL or API key.") |
There was a problem hiding this comment.
The tools support falling back to OPENSANDBOX_BASE_URL/OPENSANDBOX_API_KEY, but the provider validation rejects missing credentials before tools ever run. In Dify, provider validation happens when configuring the plugin, so a deployment that relies on env vars (e.g., secrets injected into the runtime container) will fail to register the provider even though build_connection_config would succeed at invocation time. If env-only configuration is intended, this should read env vars here or mark the credentials optional in the provider definition so the plugin can be used with env-based configuration.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Add env recognition support for the fallback path
|
@ninan-nn please review this about Python sdk usage |
| # uv pip compile pyproject.toml -o ./requirements.txt | ||
| dependencies = [ | ||
| "dify-plugin>=0.5.1", | ||
| "opensandbox", |
There was a problem hiding this comment.
Set the dependency version range to opensandbox>=1.0.0,<2.0.0 to avoid unexpected breaking changes from future major releases and ensure compatibility.
There was a problem hiding this comment.
Nice suggestion! Added opensandbox>=0.1.0,<0.2.0 to ensure compatibility.
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| [project] |
There was a problem hiding this comment.
Suggested: use Pyright and Ruff for type checking and linting.
There was a problem hiding this comment.
So far I had depended ruff already.
Since it's just a MVP dify plugin and is not stable, I believe we do NOT introduce Pyright now for simplicity.
| en_US: API key for OpenSandbox server authentication. Falls back to OPENSANDBOX_API_KEY env var. | ||
| zh_Hans: 用于 OpenSandbox 服务鉴权的 API Key。如未填写，将使用 OPENSANDBOX_API_KEY 环境变量。 | ||
| tools: | ||
| - tools/sandbox_create.yaml |
There was a problem hiding this comment.
why filesystem tool not provided, eg. file_read & file_write
hittyt
force-pushed
the
feat/dify-integration
branch
from
00b6eec to
05a5978
Compare
- Add OpenSandbox Dify plugin with sandbox_create, sandbox_run, sandbox_kill tools - Add E2E test framework for Dify plugin integration - Add GitHub Actions workflow for automated E2E testing - Support both local testing and CI testing - Use stable Dify release tag 1.11.4 - Add Docker image caching to speed up E2E tests - Add unit tests for plugin tools - Add Apache 2.0 license headers - Use OpenSandbox own icon
hittyt
force-pushed
the
feat/dify-integration
branch
from
67a9d99 to
a68b919
Compare
Align Dify plugin E2E image build with other E2E workflows by using the repository root as Docker build context. This avoids missing file errors for components referenced via COPY in components/execd/Dockerfile. Made-with: Cursor
4 participants
Summary
This PR introduces a Dify plugin that enables Dify workflows to use OpenSandbox as a code execution environment.
Features
Three sandbox tools for Dify workflows:
Flexible credential management:
Testing
Breaking Changes
Checklist