docs: add sandbox-runtime install step to deployment guide#526
docs: add sandbox-runtime install step to deployment guide#526ColeMurray wants to merge 1 commit intomainfrom
Conversation
The getting started guide was missing the Python dependency installation step for Modal deployment. Users following the guide would hit "No module named 'sandbox_runtime'" during terraform apply because sandbox-runtime is a sibling package that must be installed separately. Closes #521
📝 WalkthroughWalkthroughDocumentation updates clarifying prerequisite Python dependency installation for Modal deployment and adding troubleshooting guidance for missing sandbox_runtime module errors during Terraform deployment. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![open-inspect[bot]](https://avatars.githubusercontent.com/u/2492022?s=60&v=4){kind=small}
| modal setup | ||
|
|
||
| # Install Python dependencies for Modal deployment | ||
| cd packages/modal-infra && uv sync --frozen --extra dev && cd - |
There was a problem hiding this comment.
This new command appears in the "Required Tools" block before the repo has been cloned, so does not exist yet. Could we move this step to after /, or reword this section so the command is only introduced once the reader is inside the repository?
| ``` | ||
|
|
||
| This installs all Modal deployment dependencies including `sandbox_runtime` (resolved via | ||
| `[tool.uv.sources]` in `pyproject.toml`). If you installed Modal via `pipx install modal` alone, it |
There was a problem hiding this comment.
The troubleshooting text says is sufficient even when Modal was installed via , but Terraform still invokes plain from . That command will continue to use the pipx-managed CLI unless we also document , activate the project virtualenv, or change the deploy script to call the virtualenv's . As written, the guide may still reproduce the same .
There was a problem hiding this comment.
Summary
This PR updates docs/GETTING_STARTED.md to add a Modal Python dependency installation step and a troubleshooting note for sandbox_runtime import failures. The goal is good, but the current instructions introduce one impossible step and one place where the documented fix still does not line up with the actual deploy path.
- PR Title and number:
docs: add sandbox-runtime install step to deployment guide(#526) - Author:
@ColeMurray - Files changed count, additions/deletions:
1 file,+22/-2
Critical Issues
- [Functionality]
docs/GETTING_STARTED.md:65- The newuv synccommand is added in the "Required Tools" section before the repository has been cloned, sopackages/modal-infradoes not exist yet. This makes the guide fail for a reader following it top-to-bottom. Move this command to a post-clone step or reword the section so it only lists globally installable tooling. - [Functionality]
docs/GETTING_STARTED.md:438anddocs/GETTING_STARTED.md:774- The docs now sayuv sync --frozen --extra devfixes thesandbox_runtimeimport problem even when Modal was installed withpipx, but Terraform still calls plainmodal deployfromterraform/modules/modal-app/scripts/deploy.sh. That deploy path will continue to use the pipx-managed CLI unless the docs also tell readers to run through the project virtualenv (for exampleuv run modal ...or an activated.venv) or the deploy script is updated accordingly.
Suggestions
- None beyond the blocking issues above.
Nitpicks
- None.
Positive Feedback
- The troubleshooting section is targeting a real failure mode and explains the missing sibling dependency clearly.
- Calling out
sandbox_runtimeand thepyproject.tomlsource mapping makes the root cause easier to understand than a generic install note would.
Questions
- None.
Verdict
Request Changes: the documentation changes are directionally correct, but they currently do not give a reliable end-to-end deployment flow.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/GETTING_STARTED.md`:
- Around line 64-65: Remove the premature Modal dependency installation from the
Prerequisites section: delete the command "cd packages/modal-infra && uv sync
--frozen --extra dev && cd -" (currently in GETTING_STARTED.md Prerequisites) so
it doesn't run before the repo is cloned; the dependency installation already
exists later in Phase 1 after the clone, so keep that placement instead (or if
you prefer, move the command and add a clear note to run it only after
completing Step 1/cloning the repository).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| # Install Python dependencies for Modal deployment | ||
| cd packages/modal-infra && uv sync --frozen --extra dev && cd - |
There was a problem hiding this comment.
Remove this step from Prerequisites—repository doesn't exist yet.
This command runs cd packages/modal-infra, but the repository hasn't been cloned until Step 1 (line 73). Users following the Prerequisites section sequentially will hit a "No such file or directory" error.
The command is correctly placed later in Phase 1 (lines 437-438) after the repository exists. Consider either:
- Removing lines 64-65 entirely (recommended), or
- Adding a note that this step should be run after cloning the repository in Step 1
📝 Suggested fix
Remove the Modal dependency installation from Prerequisites:
# Python 3.12+, uv, and Modal CLI
brew install python@3.12 uv
pipx install modal
modal setup
-
-# Install Python dependencies for Modal deployment
-cd packages/modal-infra && uv sync --frozen --extra dev && cd -
# Wrangler CLI (for initial R2 bucket setup)
npm install -g wranglerThe installation step is already correctly positioned in Phase 1 (line 437-438) after the repository is cloned.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Install Python dependencies for Modal deployment | |
| cd packages/modal-infra && uv sync --frozen --extra dev && cd - | |
| # Python 3.12+, uv, and Modal CLI | |
| brew install python@3.12 uv | |
| pipx install modal | |
| modal setup | |
| # Wrangler CLI (for initial R2 bucket setup) | |
| npm install -g wrangler |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/GETTING_STARTED.md` around lines 64 - 65, Remove the premature Modal
dependency installation from the Prerequisites section: delete the command "cd
packages/modal-infra && uv sync --frozen --extra dev && cd -" (currently in
GETTING_STARTED.md Prerequisites) so it doesn't run before the repo is cloned;
the dependency installation already exists later in Phase 1 after the clone, so
keep that placement instead (or if you prefer, move the command and add a clear
note to run it only after completing Step 1/cloning the repository).
2 participants
Summary
uv sync --frozen --extra devstep to Prerequisites and Phase 1 deployment instructions in GETTING_STARTED.mdNo module named 'sandbox_runtime'errorThe deployment guide was missing the Python dependency installation step for Modal. Users following the guide would install
modalviapipx(isolated env) but never installsandbox_runtime, which is a sibling package required at deploy time (deploy.py → src.app → sandbox_runtime). This causedterraform applyto fail withModuleNotFoundError.The CI workflow already handles this correctly (
pip install -e packages/sandbox-runtimeinterraform.yml), but the gap was in the manual deployment docs.Closes #521
Test plan
terraform applysucceeds for Modal deploymentuv sync --frozen --extra devinpackages/modal-infraresolvessandbox_runtimevia the[tool.uv.sources]path dependencySummary by CodeRabbit