-
Notifications
You must be signed in to change notification settings - Fork 234
Update demos to show serving models. #1474
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
Conversation
Reviewer's GuideThis PR refactors the model-serving logic in the CLI, consolidates YAML generation under a single guard, and enhances both the demo script and system tests to showcase serving models via HTTP endpoints with customizable ports and API flags. Sequence Diagram for Model Serving and AccesssequenceDiagram
actor UserCLI
participant ramalamaCLI
participant ModelService
actor UserBrowser
UserCLI->>ramalamaCLI: ramalama serve --port <port> --name <name> -d <model> [--api <api_type>]
activate ramalamaCLI
ramalamaCLI->>ModelService: Start(model, port, api_type)
activate ModelService
ModelService-->>ramalamaCLI: Service Active on http://localhost:<port>
deactivate ModelService
ramalamaCLI-->>UserCLI: Service <name> is running
deactivate ramalamaCLI
UserBrowser->>ModelService: HTTP GET http://localhost:<port>/
activate ModelService
ModelService-->>UserBrowser: HTTP 200 OK (Model UI/Response)
deactivate ModelService
UserCLI->>ramalamaCLI: ramalama stop <name>
activate ramalamaCLI
ramalamaCLI->>ModelService: Stop()
activate ModelService
ModelService-->>ramalamaCLI: Service Stopped
deactivate ModelService
ramalamaCLI-->>UserCLI: Service <name> stopped
deactivate ramalamaCLI
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @rhatdan - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
docs/demo/ramalama.sh
Outdated
# exec_color "firefox http://localhost:8085/v1/openai" | ||
|
||
echo_color "Stop the ramalama container" | ||
exec_color "ramalama stop granite-service-pod" |
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.
issue (bug_risk): Mismatch between service name and stop command
The stop command targets granite-service-pod
, but the service was started as granite-service
. Please use the same name for both commands to ensure the correct container is stopped.
@@ -99,10 +99,27 @@ serve() { | |||
exec_color "podman ps " | |||
echo "" | |||
|
|||
echo_color "Use web browser to show interaction" | |||
exec_color "firefox http://localhost:8080" |
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.
suggestion: Hardcoded browser command may not be portable
Use $BROWSER
or xdg-open
to ensure compatibility across different environments.
Suggested implementation:
echo_color "Use web browser to show interaction"
exec_color "${BROWSER:-xdg-open} http://localhost:8080"
# echo_color "Use web browser to show interaction"
# exec_color "${BROWSER:-xdg-open} http://localhost:8085"
061e00e
to
17be460
Compare
Signed-off-by: Daniel J Walsh <[email protected]>
Summary by Sourcery
Enhance the serve command to safely handle generation flags and update demos and tests to showcase model serving endpoints with port and API options
Enhancements:
Documentation:
Tests: