-
Notifications
You must be signed in to change notification settings - Fork 0
feature/track costs general #52
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: master
Are you sure you want to change the base?
Conversation
* working fetch from models.dev * fix credo * qr * fix dialyzer * fix test
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.
Really nice feature! And I have some questions.
|
||
if is_nil(get_endpoint(resp["data"]["endpoints"], provider)) do | ||
@cache_mod.delete(key) | ||
# Retry only once |
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.
How can we reach a case where the cache contains a key (model) but no associated endpoint?
If I’m correct, when the TTL expires, the entire cache entry should be removed, not just the value.
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.
lets say we cache a response once, but suddenly the model uses a new provider (openrouter deploys a new provider for eg: deepseek or whatever), this new provider will not be in cache
endpoints | ||
|> Enum.filter(&(&1["provider_name"] == provider)) | ||
|> then(fn | ||
[endpoint | _tail] -> endpoint |
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.
In cases where there are multiple endpoints for the same model-provider, do you know if the information can change significantly?
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.
This case happened to me with google vertex and generative api, is not common, basically that case, and the price keeps the same
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.
let's say a provider can deploy one or more endpoints (eg: novita, deepinfra...) but usually will be the same pricing
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.
All right!
options | ||
) | ||
|
||
# Validate that if any price is provided, both must be provided |
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 couldn`t we calculate the price and ignore the other when we receive only one, instead of raising an error?
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.
It is an user bad config, it is ok to raise an error in that case. The idea is to indicate the tokens tariff or let it auto fetch.
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.
Ok, and why don’t you raise an error when both are nil
?
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.
In that case, it will be fetched from "models.dev" api or openrouter depending on the case
|
||
@spec models_dev_fetcher(atom(), String.t()) :: map() | nil | ||
def models_dev_fetcher(provider, model) when provider in [:open_ai, :google] do | ||
cache_key = "models_dev_api" |
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.
It could be interesting to store the pricing data under different keys using multiple requests, perhaps one per model-provider pair.
Correct me if I'm wrong. With the actual version, if the single request fails, we will lose all the information.
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.
mmmm we have retries 👀
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.
Ok, if you think that’s enough, it’s fine for me
Related: #42
Utility to easily calculate cost info via settings for openai, openrouter and google
TODO: in other pr, usehttps://models.dev/
to fetch models prices