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

[Examples] add a new example of tabby #2597

Merged
merged 3 commits into from
Nov 26, 2023

Conversation

sunny0826
Copy link
Contributor

Add a Tabby example to guide users in quickly deploying Tabby on any cloud using SkyPilot.

Copy link
Collaborator

@cblmemo cblmemo left a comment

Choose a reason for hiding this comment

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

Thanks for the exciting demo!!! 🚀 Tried the YAML and it works great 💪🏻. Left several nits ;)
Also, a discussion: There is a lot of introduction content for the skypilot repository, which seems a little bit redundant inside the skypilot repo. Maybe we could remove some of them? cc @Michaelvll for a look 👀

llm/tabby/README.md Outdated Show resolved Hide resolved
llm/tabby/README.md Outdated Show resolved Hide resolved
llm/tabby/docker-compose.cuda.yaml Outdated Show resolved Hide resolved
llm/tabby/docker-compose.yaml Show resolved Hide resolved
llm/tabby/tabbyml.yaml Outdated Show resolved Hide resolved
llm/tabby/tabbyml.yaml Outdated Show resolved Hide resolved
llm/tabby/tabbyml.yaml Outdated Show resolved Hide resolved
llm/tabby/tabbyml.yaml Outdated Show resolved Hide resolved
llm/tabby/tabbyml.yaml Outdated Show resolved Hide resolved
llm/tabby/README.md Outdated Show resolved Hide resolved
@wsxiaoys
Copy link

wsxiaoys commented Sep 26, 2023

nit: example of tabbyml -> example of tabby

Thanks for the PR! ❤️

@sunny0826 sunny0826 changed the title [Examples] add a new example of tabbyml [Examples] add a new example of tabby Sep 27, 2023
@sunny0826
Copy link
Contributor Author

sunny0826 commented Sep 29, 2023

@cblmemo Sorry for the somewhat late reply, I've been attending KubeCon China for the last few days. I fixed and tested most of what was in the review feedback.

Unfortunately, I found a bug in my testing: when I opened the port with ports: 8080, the record 8080 was not added to the network security group created by skypilot, which prevented me from accessing Tabby via public IP. After I manually added port 8080, Tabby was able to access it properly.

I haven't read the source code, so I'm not sure if this phenomenon is unique to Azure or if all clouds have similar issues.

Additional context

  • cloud: Azure
  • region australiaeast
image

@cblmemo
Copy link
Collaborator

cblmemo commented Sep 29, 2023

@cblmemo Sorry for the somewhat late reply, I've been attending KubeCon China for the last few days. I fixed and tested most of what was in the review feedback.

Unfortunately, I found a bug in my testing: when I opened the port with ports: 8080, the record 8080 was not added to the network security group created by skypilot, which prevented me from accessing Tabby via public IP. After I manually added port 8080, Tabby was able to access it properly.

I haven't read the source code, so I'm not sure if this phenomenon is unique to Azure or if all clouds have similar issues.

Additional context

  • cloud: Azure
  • region australiaeast
image

Thanks for reporting that! I tried this YAML with accelerators: V100:1 commented out (due to quota issues) several times, and one of them failed. It seems like an occasional happened bug. Will dive deep into it 🫡

Copy link
Collaborator

@Michaelvll Michaelvll left a comment

Choose a reason for hiding this comment

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

Thanks for adding this tabby example @sunny0826! This is fantastic! Left a question for the toml file. Otherwise, LGTM.

llm/tabby/tabby/config.toml Show resolved Hide resolved
Copy link

@wsxiaoys wsxiaoys left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

llm/tabby/docker-compose.yaml Outdated Show resolved Hide resolved
llm/tabby/docker-compose.yaml Outdated Show resolved Hide resolved
llm/tabby/docker-compose.yaml Outdated Show resolved Hide resolved
llm/tabby/tabby.yaml Outdated Show resolved Hide resolved
@Michaelvll
Copy link
Collaborator

Thanks for adding this example @sunny0826 @wsxiaoys! I am going to merge it soon. : )

@Michaelvll Michaelvll merged commit 62a81c5 into skypilot-org:master Nov 26, 2023
18 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.

4 participants