[Inference Providers] fix inference with URL endpoints #3041
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
Counterpart of huggingface/huggingface_hub#3041 # TL;DR Specifying `endpointUrl` is currently failing because of this call: https://github.com/huggingface/huggingface.js/blob/d95c30026fb1775c47c5d9cd6a7890d945799a97/packages/inference/src/lib/getInferenceProviderMapping.ts#L122
| pytest.param( | ||
| "https://my-custom-endpoint.com/custom_path", | ||
| "model", | ||
| "https://my-custom-endpoint.com/custom_path/v1/chat/completions", | ||
| "dummy", | ||
| id="client_model_is_url", | ||
| ), |
There was a problem hiding this comment.
Are we sure this is the expected behavior? In the past I've made a difference between InferenceClient(model="https://...").chat_completion and InferenceClient(base_url="https://...").chat_completion but I haven't tested it for a very long time. My reasoning is that if we always add /v1/chat/completions to the URL, we can't provide a URL that does not support it (what if "https://..." is already the URL to the endpoint handling chat_completion?)
but ok to remove this distinction if it helps with clarity / aligns better with JS client
There was a problem hiding this comment.
I might be mistaken but I'm not sure we had this distinction in the pre-provider implementation of InferenceClient
model or base_url), when you call chat_completion you're calling /chat/completions route by default (same behavior as the OpenAI client), but agree that we cannot customize this route, which is a bit annoying but I'm not sure if it's worth doing it.
There was a problem hiding this comment.
Thanks for checking back. I took a look at #2540 and always adding (/v1)/chat/completions seems indeed the way to go (more consistent). Looks like we merged both behaviors even before the "pre-provider" implementation ^^
Co-authored-by: Lucain <[email protected]>
Co-authored-by: Lucain <[email protected]>
|
let's merge! failing tests are unrelated (need to check where it comes from and fix that in another PR). |
Counterpart of huggingface/huggingface_hub#3041 # TL;DR Specifying `endpointUrl` is currently failing because of this call: https://github.com/huggingface/huggingface.js/blob/d95c30026fb1775c47c5d9cd6a7890d945799a97/packages/inference/src/lib/getInferenceProviderMapping.ts#L122
No description provided.