Logfire Integration #3444
Logfire Integration #3444
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@krrishdholakia What if users set multiple callbacks for the failure_handler? Shouldn't these check statements in |
|
it's inside a for-loop, so it would work either way - let me know if you hit any issues |
Oh yeah, please review this PR. I have changed |
|
@elisalimli can you add a screenshot of this passing your testing |
|
litellm/utils.py
Outdated
| # this only logs streaming once, complete_streaming_response exists i.e when stream ends | ||
| if self.stream: | ||
| if "complete_streaming_response" not in kwargs: | ||
| break |
There was a problem hiding this comment.
can we use 'continue' instead of 'break' here - so other logging integrations aren't impacted @elisalimli
There was a problem hiding this comment.
Good catch! I have realized that we're using break statement not only for the logfire callback, but also others. Therefore, I pushed a fix in ed7c9e4
litellm/utils.py
Outdated
| @@ -2355,7 +2381,9 @@ def _failure_handler_helper_fn( | |||
| def failure_handler( | |||
| self, exception, traceback_exception, start_time=None, end_time=None | |||
| ): | |||
| print_verbose(f"Logging Details LiteLLM-Failure Call") | |||
| print_verbose( | |||
| f"eLogging Details LiteLLM-Failure Call: {litellm.failure_callback}" | |||
There was a problem hiding this comment.
cleanup 'eLogging' to 'Logging'
|
Thanks @elisalimli LGTM - just some cleanup, and we should be good to merge. Appreciate you adding the testing for this |
|
Thanks @elisalimli |
Tasks
Fixes #3414