Repository navigation
feat: move the Code Interpreter template and e2b-charts into the monorepo - #1961
devin-ai-integration[bot] wants to merge 2 commits into
Conversation
…repo Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
🦋 Changeset detectedLatest commit: dda9349 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Package ArtifactsBuilt from f1c5194. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-dockerfile-utils-0.1.1-devin-1791475809-move-code-interpreter-template.0.tgz ./e2b-2.54.1-devin-1791475809-move-code-interpreter-template.0.tgzCLI ( npm install ./e2b-cli-2.21.2-devin-1791475809-move-code-interpreter-template.0.tgzCode Interpreter JS SDK ( npm install ./e2b-code-interpreter-2.8.3-devin-1791475809-move-code-interpreter-template.0.tgzDesktop JS SDK ( npm install ./e2b-desktop-2.4.1-devin-1791475809-move-code-interpreter-template.0.tgzPython SDK ( pip install ./e2b_dockerfile_utils-0.1.0+devin.1791475809.move.code.interpreter.template-py3-none-any.whl ./e2b-2.54.0+devin.1791475809.move.code.interpreter.template-py3-none-any.whlCode Interpreter Python SDK ( pip install ./e2b_code_interpreter-2.10.3+devin.1791475809.move.code.interpreter.template-py3-none-any.whlDesktop Python SDK ( pip install ./e2b_desktop-2.6.1+devin.1791475809.move.code.interpreter.template-py3-none-any.whl |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed against TASTE.md (e2b-dev/sdk-harness).
Scope: only the code this PR adds. Most of TASTE.md governs the public surface of packages/js-sdk and packages/python-sdk, which this PR doesn't touch. So the rules that could apply were the ones covering code the SDKs run or call into: command-string composition (T-43), env-var config (T-49/T-50), HTTP failure detection (T-61), named defaults and enums (T-15/T-47), errors (T-58/T-62/T-63), and use of the current SDK surface in the build scripts (T-3/T-66). The ported e2b_charts package (ChartType enum, pydantic models, flat __init__ exports) raised nothing worth flagging under T-15/T-16/T-54.
6 violations, all commented inline:
- T-43 ×2:
server/messaging.pyputs user-suppliedenvsandcwdinto kernel code without quoting. - T-50 ×2:
server/envs.py(E2B_LOCAL=falseturns local mode on) andbuild_debug.py(empty var gives an empty alias). - T-61 ×1:
server/envs.pyreturns envd's error body as the env-var map. - T-3/T-66 ×1: the build scripts call
Template.buildwith the deprecatedalias=kwarg.
Same issues on lines not commented separately:
- T-3/T-66:
build_ci.pyL7,build_debug.pyL17 andbuild_test.pyL9 also passalias=. Move the name to the second positional argument. - T-43:
_delete_env_var_snippetinserver/messaging.py(L171-180) needs the same quoting as the setter.
Minor, not tied to a specific line:
- T-15/T-47: the server dispatches on bare language literals (
"python","javascript","typescript", …) incontexts.pyandmessaging.py, and repeats"/home/user"and port49999(also hard-coded intemplate.pyandtests/conftest.py). ALanguageenum and named constants would put each of these in one place. - T-58/T-63:
contexts.create_contextraises a bareExceptionwhen session creation fails. On acwdfailure it also returns aPlainTextResponsefrom a function annotated-> Context, sopost_executethen fails oncontext.id. Raise a specific error and map it to the HTTP response inmain.py.
| if self.language == "python": | ||
| return f"import os; os.environ['{key}'] = '{value}'" | ||
| elif self.language in ["javascript", "typescript"]: | ||
| return f"process.env['{key}'] = '{value}'" | ||
| elif self.language == "r": | ||
| return f'Sys.setenv({key} = "{value}")' | ||
| elif self.language == "java": | ||
| return f'System.setProperty("{key}", "{value}");' | ||
| elif self.language == "bash": | ||
| return f"export {key}='{value}'" |
There was a problem hiding this comment.
T-43: escape anything you interpolate into a command string. key and value come from the user's envs and go into the code unquoted, so a value like it's or C:\path, or a newline, gives you a syntax error or a silently wrong value in every kernel. A JSON string literal is valid Python, JS, R and Java source, and shlex.quote handles bash (this needs import shlex):
| if self.language == "python": | |
| return f"import os; os.environ['{key}'] = '{value}'" | |
| elif self.language in ["javascript", "typescript"]: | |
| return f"process.env['{key}'] = '{value}'" | |
| elif self.language == "r": | |
| return f'Sys.setenv({key} = "{value}")' | |
| elif self.language == "java": | |
| return f'System.setProperty("{key}", "{value}");' | |
| elif self.language == "bash": | |
| return f"export {key}='{value}'" | |
| if self.language == "python": | |
| return f"import os; os.environ[{json.dumps(key)}] = {json.dumps(value)}" | |
| elif self.language in ["javascript", "typescript"]: | |
| return f"process.env[{json.dumps(key)}] = {json.dumps(value)}" | |
| elif self.language == "r": | |
| return f"Sys.setenv({json.dumps(key)} = {json.dumps(value)})" | |
| elif self.language == "java": | |
| return f"System.setProperty({json.dumps(key)}, {json.dumps(value)});" | |
| elif self.language == "bash": | |
| return f"export {key}={shlex.quote(value)}" |
The matching _delete_env_var_snippet (L171-180) needs the same fix. The bash key stays bare because a variable name can't be quoted, so check it against ^[A-Za-z_][A-Za-z0-9_]*$ first.
| if language == "python": | ||
| request = self._get_execute_request(message_id, f"%cd {path}", True) | ||
| elif language in ("javascript", "typescript"): | ||
| request = self._get_execute_request( | ||
| message_id, f"process.chdir('{path}')", True | ||
| ) | ||
| elif language == "r": | ||
| request = self._get_execute_request(message_id, f"setwd('{path}')", True) | ||
| # This does not actually change the working directory, but sets the user.dir property | ||
| elif language == "java": | ||
| request = self._get_execute_request( | ||
| message_id, f'System.setProperty("user.dir", "{path}");', True | ||
| ) | ||
| elif language == "bash": | ||
| request = self._get_execute_request(message_id, f"cd '{path}'", True) |
There was a problem hiding this comment.
T-43: same problem for cwd. It comes from the user (CreateContext.cwd) and is interpolated raw, so a path with a space breaks %cd and a path with a ' breaks JS, R and bash. Quote it per language:
| if language == "python": | |
| request = self._get_execute_request(message_id, f"%cd {path}", True) | |
| elif language in ("javascript", "typescript"): | |
| request = self._get_execute_request( | |
| message_id, f"process.chdir('{path}')", True | |
| ) | |
| elif language == "r": | |
| request = self._get_execute_request(message_id, f"setwd('{path}')", True) | |
| # This does not actually change the working directory, but sets the user.dir property | |
| elif language == "java": | |
| request = self._get_execute_request( | |
| message_id, f'System.setProperty("user.dir", "{path}");', True | |
| ) | |
| elif language == "bash": | |
| request = self._get_execute_request(message_id, f"cd '{path}'", True) | |
| if language == "python": | |
| request = self._get_execute_request( | |
| message_id, f"import os; os.chdir({json.dumps(path)})", True | |
| ) | |
| elif language in ("javascript", "typescript"): | |
| request = self._get_execute_request( | |
| message_id, f"process.chdir({json.dumps(path)})", True | |
| ) | |
| elif language == "r": | |
| request = self._get_execute_request( | |
| message_id, f"setwd({json.dumps(path)})", True | |
| ) | |
| # This does not actually change the working directory, but sets the user.dir property | |
| elif language == "java": | |
| request = self._get_execute_request( | |
| message_id, f"System.setProperty(\"user.dir\", {json.dumps(path)});", True | |
| ) | |
| elif language == "bash": | |
| request = self._get_execute_request( | |
| message_id, f"cd {shlex.quote(path)}", True | |
| ) |
|
|
||
| import httpx | ||
|
|
||
| LOCAL = os.getenv("E2B_LOCAL", False) |
There was a problem hiding this comment.
T-50: an empty env var means unset, and a flag-shaped var should be parsed with a comparison. Here the raw string is used for truthiness, so E2B_LOCAL=false or E2B_LOCAL=0 turns local mode on and get_envs returns the stub instead of the real envd envs. Compare against an explicit value, the way tests/conftest.py::is_debug already does:
| LOCAL = os.getenv("E2B_LOCAL", False) | |
| LOCAL = os.getenv("E2B_LOCAL", "false").lower() == "true" |
| response = await client.get( | ||
| f"http://localhost:{ENVD_PORT}/envs", headers=headers | ||
| ) | ||
| return response.json() |
There was a problem hiding this comment.
T-61: detect failure from the HTTP status. When envd answers 401 or 5xx, its error body is parsed and returned as the env-var map. That map then feeds the per-language _set_env_vars_code, so instead of a clear error you get garbage variables. Check the status before you trust the body:
| response = await client.get( | |
| f"http://localhost:{ENVD_PORT}/envs", headers=headers | |
| ) | |
| return response.json() | |
| response = await client.get( | |
| f"http://localhost:{ENVD_PORT}/envs", headers=headers | |
| ) | |
| response.raise_for_status() | |
| return response.json() |
|
|
||
| load_dotenv() | ||
|
|
||
| alias = os.getenv("E2B_DEBUG_TEMPLATE", "code-interpreter-debug") |
There was a problem hiding this comment.
T-50: an empty env var means unset. With the two-argument os.getenv, an exported but empty E2B_DEBUG_TEMPLATE= (for example from a copied .env) gives alias == '', and the build fails with an unhelpful name error instead of using the default. Use the or form:
| alias = os.getenv("E2B_DEBUG_TEMPLATE", "code-interpreter-debug") | |
| alias = os.getenv("E2B_DEBUG_TEMPLATE") or "code-interpreter-debug" |
| make_template(), | ||
| alias="code-interpreter-v1", |
There was a problem hiding this comment.
T-3 (required arguments are positional) / T-66 (deprecated surface points to the migration path): the template name is a required argument of Template.build(template, name, ...), and the alias= kwarg is deprecated ("(Deprecated) Alias name for the template. Use name instead." in e2b/template_sync/main.py). Template code that ships inside the SDK monorepo should show the current form, not the deprecated one:
| make_template(), | |
| alias="code-interpreter-v1", | |
| make_template(), | |
| "code-interpreter-v1", |
build_ci.py, build_debug.py and build_test.py need the same change (see the review summary).
…reter Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
| client, websockets, language, "/home/user" | ||
| ) | ||
| except Exception as e: | ||
| return PlainTextResponse(str(e), status_code=500) |
| try: | ||
| return await create_context(client, websockets, language, cwd) | ||
| except Exception as e: | ||
| return PlainTextResponse(str(e), status_code=500) |
This branch has not been deployed
2 participants
Summary
Moves the Code Interpreter sandbox template (
code-interpreter-v1), its HTTP test suite and thee2b-chartspackage from e2b-dev/code-interpreter (@2dd4042) into this repo, along with their CI/CD. As in #1768, the files are byte-identical copies apart from the intentional edits listed below.template/templates/code-interpreter/@e2b/code-interpreter-template(unchanged)tests/templates/code-interpreter/tests/(still its own uv project on PyPIe2b)chart_data_extractor/packages/charts-python/@e2b/data-extractor→@e2b/charts-python(PyPIe2b-chartsunchanged)pnpm-workspace.yamlnow includestemplates/*, so the template stays a versioned Changesets package.templates/baseandtemplates/httpbinhave nopackage.json, so pnpm ignores them. The unreleased.changeset/jupyter-unix-socket.mdis moved over as well.Intentional edits:
templates/code-interpreter/Makefile.uvx ruff@0.11.12, because bareruffisn't on PATH in Lint CI.package.json: follows thecode-interpreter-pythonscripts (postVersion --frozen,lock,postPublishvia trusted publishing). The unused eslint devDeps are dropped, andtestno longer passes-n 4(pytest-xdist isn't a dependency).pyproject.toml: URLs point at this repo.CI/CD
code_interpreter_template_tests.yml(reusable):build(build_ci.py, pinse2b_charts==<charts-python version>) →test(pytest intests/) →cleanup(deletes the CI template). It merges the oldbuild_test_template,code_interpreter_testsandcleanup_build_templateworkflows.charts_python_tests.yml(reusable): the oldcharts_tests.yml.code_interpreter_template_build.yml: manual dispatch (foxtrot/staging/juliett,skip_cache,push_docker_image) andworkflow_call. It replacesbuild_prod_template.ymland the release job that built the Docker image.sdk_tests.yml: new path-filtered production jobs, also added tostatus. The template filter leaves out*sharedon purpose, so lockfile bumps don't build a template.release.yml:publish.e2b-chartsis published bypnpm run -r postPublish.code-interpreter-template-buildjob runs afterpublishand builds foxtrot + Docker Hub frompublish.outputs.released_sha. That output is new inpublish_packages.yml, and building from it means the template pins the newly released charts version.Setup needed before the first release
E2B_PROD_API_KEY,E2B_STAGING_API_KEY,E2B_JULIETT_API_KEYand the varE2B_DOMAIN, using the same values as e2b-dev/code-interpreter.DOCKERHUB_*already exist here.e2b-charts(e2b-dev/E2B,release.yml, environmentdeployment) to replaceCHARTS_PYPI_TOKEN.The source removal in e2b-dev/code-interpreter is e2b-dev/code-interpreter#348; merge it after this PR.
Link to Devin session: https://app.devin.ai/sessions/0ca2a540e49045ba993f8f5ddc51ecac
Open in Devin Desktop: https://app.devin.ai/desktop/session/0ca2a540e49045ba993f8f5ddc51ecac?variant=devin
Requested by: @mishushakov