-
Notifications
You must be signed in to change notification settings - Fork 59
feat: start a recipe with llama-stack backend #3016
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?
Conversation
6954ab1
to
a5f9985
Compare
if (model) { | ||
// upload model to podman machine if user system is supported | ||
if (!recipeComponents.inferenceServer) { |
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: use a single if model && !recipeComponents.inferenceServer
labels: Record<string, string> = {}, | ||
): Promise<PodInfo> { | ||
const modelId = model ? model.id : '0'; |
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.
I don't really like that, use something more understandable like <none>
or none
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.
Sure, me neither:
Note: I'm not very happy with the way the applicationManager functions accept an optional model, depending on if we want to use llama-stack or not. Let's first discuss the high-level architecture of this solution, I can change these implementation details when we agree on the architecture
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.
Sorry I went to code too fast 🙏
Let's first discuss the high-level architecture of this solution,
What depends on the modelId
? We use it for operation like hasApplicationPod
& removeApplication
. Maybe we should only use the recipeId as key?
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.
I'm not very comfortable changing the labels, as the model label is also used for the tasks (the user can start the same recipe with different models, and we want to differentiate the tasks for them).
I have pushed a new commit changing how the parameters are passed: having explicit options inferModel / startLlamaStack, which enforce the other parameters (the model
parameter for the moment). I also changed this arbitrary 0
model Id to <none>
@axel7083 WDYT?
Signed-off-by: Philippe Martin <phmartin@redhat.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
a6227e1
to
49e4639
Compare
Please add "llama-stack" here https://github.com/containers/podman-desktop-extension-ai-lab/blob/main/packages/frontend/src/lib/RecipeCardTags.ts#L25 so it works with other tags (I think this should be only change to make it work) |
labels?: { [key: string]: string }, | ||
): Promise<RecipeComponents> { | ||
const localFolder = path.join(this.appUserDirectory, recipe.id); | ||
public async buildRecipe(options: ApplicationOptions, labels?: { [key: string]: string }): Promise<RecipeComponents> { |
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.
why not including the labels in the options? feel weird to move everything inside ApplicationOptions but the labels?
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.
First reason is because labels is not part of the parameters of requestPullApplication
, and I would like to keep the same options structure everywhere
Signed-off-by: Philippe Martin <phmartin@redhat.com>
8608fe7
to
ce79877
Compare
Signed-off-by: Philippe Martin <phmartin@redhat.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
c22c5d9
to
7db6fe5
Compare
Signed-off-by: Philippe Martin phmartin@redhat.com
What does this PR do?
This PR is a first step towards being able to run a recipe using llama stack as backend.
This PR covers:
backend="llama-stack"
Screenshot / video of UI
llama-stack-recipe-2.mp4
What issues does this PR fix or reference?
Fixes #2625
How to test this PR?
~/.local/share/containers/podman-desktop/extensions-storage/redhat.ai-lab/user-catalog.json
file: