-
Notifications
You must be signed in to change notification settings - Fork 97
fix: model register missing model-type and not accepting metadata #252
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
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.
The changes look good. @Bobbins228 just one nit - could you please add the output and use of this command to the docs (in detailed tutorial - https://llama-stack.readthedocs.io/en/latest/getting_started/detailed_tutorial.html#step-3-run-client-cli), so that users are aware that there is an easy way to register the model even after server has started. Thanks!
/lgtm
/approve
|
@varshaprasad96 Happy to update docs as a follow on when this is merged 👍 |
|
Could you add this PR to the https://github.com/llamastack/llama-stack-client-python repo instead? The meta-llama repo is now read-only. We will archive it shortly. |
| @click.option("--provider-id", help="Provider ID for the model", default=None) | ||
| @click.option("--provider-model-id", help="Provider's model ID", default=None) | ||
| @click.option("--metadata", help="JSON metadata for the model", default=None) | ||
| @click.option("--model-type", help="Model type", default="llm") |
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.
add the allowed values for Model type in the help.
raghotham
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.
small nit. thanks for the change.
|
Thanks @ashwinb and @raghotham I'll provide it there and close this after |
|
Closing in favour of #11 |
What does this PR do?
[Provide a short summary of what this PR does and why. Link to relevant issues if applicable.]
Added the ability to specify the model type when registering models.
Also fixed a bug with passing metadata which would result in the following error:
Closes: #215
Test Plan
Run the following commands