Fix healthcheck and streaming with whisper #59
Conversation
Signed-off-by: AudranBert <[email protected]>
Signed-off-by: AudranBert <[email protected]>
Signed-off-by: AudranBert <[email protected]>
Signed-off-by: AudranBert <[email protected]>
Signed-off-by: AudranBert <[email protected]>
Signed-off-by: AudranBert <[email protected]>
Signed-off-by: AudranBert <[email protected]>
Signed-off-by: AudranBert <[email protected]>
Signed-off-by: AudranBert <[email protected]>
| text = "" | ||
| partial = None | ||
| async with websockets.connect(ws_api) as websocket: | ||
| duration = 0 | ||
| async with websockets.connect(ws_api, ping_interval=None, ping_timeout=None) as websocket: |
There was a problem hiding this comment.
ping_interval / ping timeout : is that related to TCP breaks (HTTP WS keepAlive) ? Check comment below on handling only ConnectionClosedOK.
There was a problem hiding this comment.
It is to avoid closing websocket when it is not wanted. When testing, if I did not send any messages for a long time, the websocket closed by itself. By disabling the ping it doesn't close anymore.
| logger.debug(f"??? {message}") | ||
| except asyncio.CancelledError: | ||
| logger.debug("Message processing thread stopped as websocket was closed.") | ||
| except websockets.exceptions.ConnectionClosedOK: |
There was a problem hiding this comment.
Currently, only ConnectionClosedOK is handled to cancel the websocket. Please review whether other exceptions could occur in this context. For example, a TCP RST might trigger a ConnectionClosed event, similar to how WebSocketDisconnect is handled in FastAPI.
Additionally, a finally block might be missing to handle unexpected exceptions and actively close the context for the current socket.
There was a problem hiding this comment.
In the case of that script, that is supposed to be just for testing the streaming locally I'm not sure if we can encounter other errors. I will add a finally to close the websocket and try to see if something else can be triggered.
There was a problem hiding this comment.
Ok,
Check actual implementation in the real Websocket server to ensure corner-cases are well handled please.
Signed-off-by: AudranBert <[email protected]>
Signed-off-by: AudranBert <[email protected]>
Signed-off-by: AudranBert <[email protected]>
Signed-off-by: AudranBert <[email protected]>
Signed-off-by: AudranBert <[email protected]>
Signed-off-by: AudranBert <[email protected]>
Signed-off-by: AudranBert <[email protected]>
This PR aims at fixing #49 #47 and #58 by:
What remains to be done: