-
Notifications
You must be signed in to change notification settings - Fork 6.4k
Helper for dynamically fetching MistralAI models #19911
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?
Helper for dynamically fetching MistralAI models #19911
Conversation
@logan-markewich Can you please help me out with unit tests? I see there's only a mock api key. Should I mock the API response too? It is not obvious to me how the model responses are mocked for example. |
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.
Not super sure I agree with this PR: I understand where it is coming from (we would not have to manually add a model every time mistral drops one), but there are several pitfalls that make this approach not super efficient:
- We are adding more code than we are deleting
- We are performing synchronous calls to an API simply to fetch metadata, which in general I would avoid
- We are applying this solution only to Mistral: if we adopt this approach, it would be good to have something similar for all LLMs that support this, maybe adding a base class in the core framework
I would therefore suggest not to make this change
|
I'll be honest, after thinking about this, I'd rather we just maintain the static list for now. The current code introduces extra latency, blocks async event loops, and in general is not extendable to most LLMs. Plus it might not always be clear how to map their API results to features that we need to know about (thinking, tool calling, modalities, etc.) |
Description
I've introduced helper class for dynamically loading MistralAI models to replace static constatants from helpers. This loads the models once per client lifetime by default (as discussed on the original issue) with the option for users to also refresh model list manually.
Fixes #17442
New Package?
Did I fill in the
tool.llamahub
section in thepyproject.toml
and provide a detailed README.md for my new integration or package?Version Bump?
Did I bump the version in the
pyproject.toml
file of the package I am updating? (Except for thellama-index-core
package)Type of Change
Please delete options that are not relevant.
How Has This Been Tested?
Your pull-request will likely not be merged unless it is covered by some form of impactful unit testing.
Suggested Checklist:
uv run make format; uv run make lint
to appease the lint gods