Skip to content
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

Add k8s with llama server example #39

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

CaptainVincent
Copy link
Member

I tried to start the server not using the kubectl.sh run command, but rather by applying a YAML file.
However, I encountered an odd phenomenon where the network connection has not been established.

Therefore, I've submitted a pull request for this version.
Later, I'll push the apply yaml file approach to another branch for backup to check issue after openvino task.

Copy link
Member

juntao commented Jan 7, 2024

Hello, I am a code review bot on flows.network. Here are my reviews of code commits in this PR.


Commit 78d2f94261a6f45f39d694061c4fb2658374d994

The key change in this pull request is the addition of a Kubernetes example that demonstrates running a WasmEdge LLAMA chat server application with Containerd over Kubernetes. The pull request includes new files and modifications to existing files.

Potential problems that could be addressed include:

  1. The README.md file seems to be missing the output of the test command in the "Run LLAMA chat server app" section. This could provide more information to users who want to verify the success of the deployment.

  2. The .github/workflows/kubernetes-containerd-llama-server.yml file has a long shell script in the run step, which may make it difficult to read and maintain. Consider breaking it down into separate run steps or putting it in a separate shell script file.

  3. The llama_server_application.sh script seems to include hardcoded paths and environment variable assignments. It could be made more flexible and reusable by allowing users to configure these paths and variables.

  4. The use of sudo in the llama_server_application.sh script may cause problems if the script is run by a user without sudo privileges. Consider removing or rethinking the need for sudo in the script.

  5. The patch does not include any validation or checks after running the application in Kubernetes. It would be helpful to include some validation steps to ensure the application has been deployed successfully and is running as expected.

Overall, the changes seem to introduce a new Kubernetes example, but there are some areas for improvement to make the example more user-friendly and robust.


- name: Install containerd, WasmEdge, and crun with supprt of plugins and nn-preoload
run: |
sed 's|https://github.com/containers/crun|-b enable-wasmedge-plugin https://github.com/CaptainVincent/crun|g' containerd/install.sh | bash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's put the patch in https://github.com/second-state/crun instead of a personal repo.

@CaptainVincent CaptainVincent merged commit 3d876fc into second-state:main Jan 8, 2024
42 of 44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants