-
Notifications
You must be signed in to change notification settings - Fork 51
Add a llama-stack tutorial to the getting started guide #767
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add a llama-stack tutorial to the getting started guide #767
Conversation
WalkthroughAdds a new interactive Bash tutorial at Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (runs script)
participant Script as llama_stack_tutorial.sh
participant API as LLAMA_STACK_URL (HTTP API)
participant Formatter as jq / cat
User->>Script: start (optionally --no-wait)
Script->>API: GET /health, /version, /models, /tools, /safety, /inference...
API-->>Script: JSON responses
Script->>Formatter: format JSON (jq if available, else cat)
Formatter-->>Script: formatted output
Script-->>User: display section, pause (if interactive)
alt user continues
User->>Script: proceed to next section
else user exits
User->>Script: quit
end
Script-->>User: final recap
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (3)
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 |
|
Hi @anik120. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scripts/llama_stack_tutorial.sh (1)
69-76: Refactor repeated jq availability checks into a helper function.The jq availability check is performed multiple times (lines 70, 153, 199), and
command -v jq &> /dev/nullis duplicated. This violates DRY and makes maintenance harder if the check logic needs to change.+# Helper function to check if jq is available +has_jq() { + command -v jq &> /dev/null +} + -# Check if jq is available -if ! command -v jq &> /dev/null; then +if ! has_jq; then echo "⚠️ Warning: 'jq' is not installed. Output will be less formatted." echo " Install with: apt-get install jq (Ubuntu/Debian), yum install jq (RHEL/CentOS), brew install jq (macOS)" JQ_CMD="cat" else JQ_CMD="jq ." fi ...and replace subsequent checks: -if command -v jq &> /dev/null; then +if has_jq; then # ... processing ... fiAlso applies to: 153-172, 199-203
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)scripts/llama_stack_tutorial.sh(1 hunks)
⏰ 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). (2)
- GitHub Check: e2e_tests (ci)
- GitHub Check: e2e_tests (azure)
🔇 Additional comments (2)
scripts/llama_stack_tutorial.sh (2)
43-49:evalpattern poses security risk; verify controlled usage.The
run_commandfunction usesevalto execute dynamically constructed commands. While the commands are internally controlled here,evalis inherently risky and difficult to audit.Consider alternatives:
- Use bash arrays with command substitution instead of
eval- Document explicitly why
evalis necessary (e.g., supporting command expansion in$cmd)- Add input validation/escaping if external input is ever passed to
run_commandCurrent usage appears safe since
$cmdis only constructed internally, but monitor for future changes that might introduce external input.
1-330: Comprehensive tutorial script with clear educational structure.The script effectively guides users through Llama Stack features with good UX patterns: interactive pauses, color formatting, fallback handling for missing dependencies, and command-line options. Content is well-organized and pedagogically sound.
Minor observation: The script assumes LLAMA_STACK_URL (hardcoded to
localhost:8321) is accessible and running. Consider adding a brief connectivity check at startup to provide early feedback if the server is unreachable. This would improve the user experience when the server is not running.
tisnik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, why not to use it
|
@anik120 LGTM, but please look at shellcheck bug (unused RED control code) |
37a5e2e to
00755b0
Compare
This will be helpful for folks who are new to llama-stack
00755b0 to
28cf6df
Compare
Description
This will be helpful for folks who are new to llama-stack
Type of change
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
New Features
Documentation