Fallback API for Inference #4099
Conversation
chenghao-mou
left a comment
chenghao-mou
left a comment
There was a problem hiding this comment.
LGTM.
Not sure if this is discussed before: there is a side effect on the logs/metrics part—if a fallback is chosen inside gateway, we will still report metrics/logs under _opts.model.
|
Yeah, that's interesting i wonder if we should find a way to report back the model used by Inference. Also, @chenghao-mou can you coordinate with me before merging and releasing? |
|
Also, I just pushed a small change to address 3.9 type checks, hopefully. |
Co-authored-by: Théo Monnom <[email protected]>
adrian-cowham
force-pushed
the
ac/inference-fallbacks
branch
from
d5f1c8f to
adb0c19
Compare
theomonnom
left a comment
theomonnom
left a comment
There was a problem hiding this comment.
Otherwise lgtm! Nice work!
| name: Required[str] | ||
| """Model name (e.g. "cartesia/sonic", "elevenlabs/eleven_flash_v2", "rime/arcana").""" | ||
|
|
||
| voice: Required[str | None] |
There was a problem hiding this comment.
Can the voice be None? Is this different than omitting it (removing the Required flag)
| class Fallback(TypedDict, total=False): | ||
| """Configuration for fallback models when the primary model fails.""" | ||
|
|
||
| models: Required[Sequence[FallbackModelType]] | ||
| """Fallback models in priority order.""" | ||
|
|
||
| connection: ConnectionOptions | ||
| """Connection options for fallback attempts.""" |
There was a problem hiding this comment.
I'm wondering if this class is really useful.
Maybe the argument inside the TTS/STT can directly accept a Sequence of FallbackModelType and the ConnectionOptions can directly be inside the constructor?
Probably nitpicking but maybe we can also re-use
?I'm not 100% sure, wdyt?
There was a problem hiding this comment.
Right now, we already have conn_options as a parameter in stream or _recognize_impl, but it is often used with the default value somewhere behind the scenes (not exposed to the user). I think it is a good idea to expose this at the constructor level for all of them (we can start with inference in this PR) with the existing APIConnectOptions.
That would be nice-to-have, and we can do that in a later PR. And yes, I should be able to do that. |
No description provided.