Remove catch-all exception logger for asyncio tasks #605
Conversation
|
|
||
|
|
||
| def asyncio_create_task_with_done_error_log( | ||
| def asyncio_create_task( |
There was a problem hiding this comment.
this is fine - i understand why we need tasks_registry, but I wonder if we audit the use-cases and eventually remove this method
There was a problem hiding this comment.
The primary reason I removed this is because each exception log causes a duplicate error in sentry. And we can just catch them by the raised error. Is there a scenario where we actually need this method/functionality?
There was a problem hiding this comment.
i believe we keep tasks in tasks_registry so that these tasks don't get garbage collected when they aren't assigned to some state in a worker, e.g.
i was thinking we would eventually remove it and replace it with asyncio.create_task
There was a problem hiding this comment.
Oh sure, you're talking about the method in general, not the change I made. Yeah eventually should be good to change to asyncio.create_task
|
|
||
|
|
||
| def asyncio_create_task_with_done_error_log( | ||
| def asyncio_create_task( | ||
| *args, | ||
| reraise_cancelled: bool = False, |
There was a problem hiding this comment.
this would be a regression to remove this, right? remind me again what happens to tasks normally when they get cancelled
There was a problem hiding this comment.
I'm not removing it though? It's just a method rename to not have "with done error log"
There was a problem hiding this comment.
Are you referring to the reraise_cancelled part? That part didn't make sense to me because we only check for an asyncio.CancelledError when trying to log (which seems like would never happen). Has that occurred before?
There was a problem hiding this comment.
yes, I'm referring to the reraise_cancelled part
There was a problem hiding this comment.
reading back the code, this is safe - all reraise_cancelled = True does is make more visible a cancel error when it actually gets cancelled
* [DOW-118] set up code linting and tests (#589) * adds github workflow * run black * run isort * adds precommit * adds vscode settings * adds pre-commit guidelines (#590) * creates docker image, updates telephony app deps (#601) * [DOW-105] refactor interruptions into the output device (#586) * [DOW-105] refactor interruptions into the output device (#562) * initial refactor works * remove notion of UtteranceAudioChunk and put all of the state in the callback * move per_chunk_allowance_seconds into output device * onboard onto vonage * rename to abstract output device and onboard other output devices * initial work to onboard twilio output device * twilio conversation works * some cleanup with better comments * unset poetry.lock * move abstract play method into ratelimitoutputdevice + dispatch to thread in fileoutputdevice * rename back to AsyncWorker * comments * work through a bit of mypy * asyncio.gather is g2g: * create interrupt lock * remove todo * remove last todo * remove log for interrupts * fmt * fix mypy * fix mypy * isort * creates first test and adds scaffolding * adds two other send_speech_to_output tests * make send_speech_to_output more efficient * adds tests for rate limit interruptions output device * makes some variables private and also makes the chunk id coming back from the mark match the incoming audio chunk * adds twilio output device tests * make typing better for output devices * fix mypy * resolve PR comments * resolve PR comments * [DOW-101] LiveKit integration (#591) * checkpoint * livekit v0 * in progress changes * integrate with worker * fix import * update deps and remove unneeded files * integrate it properly into app * fix interrupts * make transcript publish work * a confounding fix * isort * constants, some cleanup --------- Co-authored-by: Kian <[email protected]> * upgrade to latest cartesia 1.0.3 (#587) * upgrade to latest cartesia 1.0.3 * fixed linting conflict * finish streaming * make cartesia optional --------- Co-authored-by: Ajay Raj <[email protected]> * poetry version prerelease (#602) * feat: Add ability to configure OpenAI base URL in ChatGPTAgentConfig (#577) * feat: Add ability to configure OpenAI base URL in ChatGPTAgentConfig - Added `base_url` parameter to `ChatGPTAgentConfig` to allow customization of the OpenAI API base URL. - Updated `instantiate_openai_client` function to use the `base_url` parameter from the configuration. - Modified `ChatGPTAgent` to utilize the updated `instantiate_openai_client` function. - Added tests to verify the new `base_url` functionality in `tests/streaming/agent/test_base_agent.py`. This enhancement allows users to specify a custom OpenAI API base URL, providing greater flexibility in agent configuration. * adding capability to use the openai compatible endpoint with token estimation for llama * lint fix * changing openai base_url parameter for overall less code changes * missed logging update * Update vocode/streaming/agent/chat_gpt_agent.py * Update tests/streaming/agent/test_base_agent.py * fix test --------- Co-authored-by: Ajay Raj <[email protected]> * Support passthrough of AsyncHTTPTransport (#603) Support passthrough of AsyncHTTPTransport object * add script used to make PR * adds test target for vocodehq-public * Remove catch-all exception logger for asyncio tasks (#605) * remove error log from exception for asyncio tasks * remove log error on chatgpt query --------- Co-authored-by: Kian <[email protected]> Co-authored-by: rjheeta <[email protected]> Co-authored-by: Clay Elmore <[email protected]> Co-authored-by: vocode-petern <[email protected]> Co-authored-by: Adnaan Sachidanandan <[email protected]>
logger.exceptionstatement run whenever an exception occurs in any asyncio taskasyncio_create_task_with_done_error_log()toasyncio_create_task()based on aboveTested locally, raises the same errors in the console. Also confirmed ChatGPT errors are correct