Skip to content

Conversation

@Bobbins228
Copy link

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:

Error Type: BadRequestError                                                                                                                                     │
│ Details: Error code: 400 - {'error': {'detail': {'errors': [{'loc': ['body', 'metadata'], 'msg': 'Input should be a valid dictionary', 'type': 'dict_type'}]}}}

Closes: #215

Test Plan

Run the following commands

# Note a Llama Stack Server must be running
# Create a venv
uv sync --python 3.12
# Install the LSC with the new code changes
uv pip install -e .
# List the available models
llama-stack-client models list
# Register the granite-embedding-30m embedding model NOTE must have sentence-transformers as an inference provider
llama-stack-client models register granite-embedding-30m --provider-id "sentence-transformers" --provider-model-id ibm-granite/granite-embedding-30m-english --metadata '{"embedding_dimension": 384}' --model-type embedding
# Verify the embedding model added are present
llama-stack-client models list

Copy link

@varshaprasad96 varshaprasad96 left a 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

@Bobbins228
Copy link
Author

@varshaprasad96 Happy to update docs as a follow on when this is merged 👍

@ashwinb
Copy link
Contributor

ashwinb commented Jul 8, 2025

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")
Copy link
Member

@raghotham raghotham Jul 8, 2025

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.

Copy link
Member

@raghotham raghotham left a 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.

@Bobbins228
Copy link
Author

Thanks @ashwinb and @raghotham I'll provide it there and close this after

@Bobbins228
Copy link
Author

Closing in favour of #11

@Bobbins228 Bobbins228 closed this Jul 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[cli] unable to register embedding model

5 participants