-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: convert models API to use a FastAPI router #4407
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
✱ Stainless preview buildsThis PR will update the Edit this comment to update it. It will appear in the SDK's changelogs. ✅ llama-stack-client-node studio · code · diff
⚡ llama-stack-client-python studio · conflict
⚡ llama-stack-client-go studio · conflict
This comment is auto-generated by GitHub Actions and is automatically kept up to date as you push. |
a17e181 to
2d787c5
Compare
78b26ec to
d925af3
Compare
622891a to
6ddbe30
Compare
|
Hi @nathan-weinberg , are you going to update unit test at https://github.com/llamastack/llama-stack/tree/main/tests/unit in follow up PR? Thanks |
What needs updating? |
|
@nathan-weinberg For example, in https://github.com/llamastack/llama-stack/tree/main/tests/unit/core/routers |
That would be done as a separate PR then, since it does not pertain to what this PR is doing could you open an issue to track it? |
|
This pull request has merge conflicts that must be resolved before it can be merged. @nathan-weinberg please rebase it. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork |
| async def get_model(self, model_id: str) -> Model: | ||
| async def get_model(self, request_or_model_id: GetModelRequest | str) -> Model: | ||
| # Support both the public Models API (GetModelRequest) and internal ModelStore interface (string) | ||
| if isinstance(request_or_model_id, str): |
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.
| if isinstance(request_or_model_id, str): | |
| if isinstance(request_or_model_id, GetModelRequest): | |
| model_id = request_or_model_id.model_id | |
| else: | |
| model_id = request_or_model_id |
you could also use *, in the signature to be extra strict.
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.
Done - curious, what does the * in the signature do?
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.
Hmmm, tests don't seem to like it
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.
Done - curious, what does the * in the signature do?
this means everything after this must be passed as a keyword argument.
6ddbe30 to
7000d0b
Compare
26c25ec to
82a4d4d
Compare
Migrate from @webmethod decorators to FastAPI router pattern Signed-off-by: Nathan Weinberg <nweinber@redhat.com>
82a4d4d to
a2e2669
Compare
What does this PR do?
Migrate from @webmethod decorators to FastAPI router pattern
Closes #4347