Skip to content

Conversation

nnshah1
Copy link
Contributor

@nnshah1 nnshah1 commented Oct 13, 2025

Summary

  • Updated run.sh to make interactive behavior explicit when using --mount-workspace
  • This allows for easier running of commands using run.sh --mount-workspace -- COMMAND from tools like claude code
  • Added -it flag explicitly to documentation examples for clarity
  • Removed automatic -it addition when --mount-workspace is used

Changes

  • Modified container/run.sh to remove automatic interactive flag setting
  • Updated documentation in container/README.md and docs/backends/sglang/README.md to explicitly include -it flag in examples
  • This makes the interactive behavior explicit rather than implicit when mounting workspace

Summary by CodeRabbit

  • Documentation

    • Updated container and backend guides to use interactive TTY (-it) in Docker run examples, including local development, image-specific runs, interactive shells, and pytest usage.
  • Chores

    • Adjusted run script behavior: no longer forces interactive mode in the workspace-mount path and no longer auto-injects a Hugging Face token when mounting the workspace. Users should explicitly pass interactive flags and required environment variables when running containers.

- Remove automatic -it flag when using --mount-workspace in run.sh
- Update README examples to explicitly include -it when needed
- Allows non-interactive usage of --mount-workspace for CI/automation
@nnshah1 nnshah1 requested review from a team as code owners October 13, 2025 15:33
@github-actions github-actions bot added the fix label Oct 13, 2025
Copy link
Contributor

coderabbitai bot commented Oct 13, 2025

Walkthrough

Adds -it to example docker run invocations in documentation. In run.sh, removes automatic HF_TOKEN injection and stops forcing -it when mounting workspace. Overall docker invocation remains similar, with interactive control moved to callers and environment token injection removed.

Changes

Cohort / File(s) Summary of Changes
Docs: interactive examples
container/README.md, docs/backends/sglang/README.md
Updated example docker commands to include -it for interactive TTY sessions when mounting the workspace and running shells/pytest. No logic changes.
Container run script behavior
container/run.sh
Removed automatic HF_TOKEN env injection when mounting workspace. Stopped auto-setting INTERACTIVE to -it in the HF_HOME mounting path, changing constructed docker run options only for that branch.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant RS as run.sh
  participant DO as Docker

  rect rgba(230, 240, 255, 0.5)
  note over Dev,DO: Previous behavior (workspace mounted)
  Dev->>RS: Invoke run.sh with --mount-workspace
  RS->>RS: Build docker options<br/>• Mount HF_HOME/workspace<br/>• Add HF_TOKEN env<br/>• Set INTERACTIVE = -it
  RS->>DO: docker run ... -it -v ... -e HF_TOKEN=... image cmd
  DO-->>Dev: Interactive TTY session with token injected
  end

  rect rgba(235, 255, 235, 0.5)
  note over Dev,DO: New behavior
  Dev->>RS: Invoke run.sh with --mount-workspace
  RS->>RS: Build docker options<br/>• Mount HF_HOME/workspace<br/>• Do not add HF_TOKEN<br/>• Do not force -it
  RS->>DO: docker run ... -v ... image cmd
  DO-->>Dev: Session behavior depends on caller flags (-it) and env
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hop to shells with flags so bright,
-it now guides my TTY light.
No secret tokens stashed within,
Just clean runs where we begin.
With paws on keys, I docker-spin—
A tidy warren, lean and thin. 🐇🚀

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description deviates from the repository’s template by using “## Summary” and “## Changes” sections instead of the required “#### Overview:” and “#### Details:”, and it also omits the “#### Where should the reviewer start?” and “#### Related Issues:” sections. Please update the description to use the template headings “#### Overview:”, “#### Details:”, “#### Where should the reviewer start?”, and “#### Related Issues:”, and fill in each section with the corresponding information.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly summarizes the primary change by indicating that the interactive behavior for --mount-workspace is now explicit, matching the core modifications in run.sh and documentation.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 90dc758 and 874fd7f.

📒 Files selected for processing (3)
  • container/README.md (5 hunks)
  • container/run.sh (0 hunks)
  • docs/backends/sglang/README.md (1 hunks)
💤 Files with no reviewable changes (1)
  • container/run.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build and Test - dynamo

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant