litellm icon indicating copy to clipboard operation
litellm copied to clipboard

Logfire Integration

Open elisalimli opened this issue 9 months ago • 4 comments

Tasks

  • [x] Add tests
  • [x] Documentation

Fixes #3414

image

elisalimli avatar May 04 '24 12:05 elisalimli

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
litellm ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 14, 2024 6:33pm

vercel[bot] avatar May 04 '24 12:05 vercel[bot]

@krrishdholakia What if users set multiple callbacks for the failure_handler? Shouldn't these check statements in failure_handler be if instead of elif? https://github.com/BerriAI/litellm/blob/main/litellm/utils.py#L2362

elisalimli avatar May 04 '24 12:05 elisalimli

it's inside a for-loop, so it would work either way - let me know if you hit any issues

krrishdholakia avatar May 04 '24 15:05 krrishdholakia

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 elifs to ifs it should not an issue as you just have said. Let me know if you want to revert that change to back.

elisalimli avatar May 04 '24 15:05 elisalimli

@elisalimli can you add a screenshot of this passing your testing

krrishdholakia avatar May 07 '24 14:05 krrishdholakia

@elisalimli can you add a screenshot of this passing your testing

pytest ./tests/test_logfire.py

image

elisalimli avatar May 07 '24 16:05 elisalimli

Thanks @elisalimli

LGTM - just some cleanup, and we should be good to merge. Appreciate you adding the testing for this

krrishdholakia avatar May 14 '24 16:05 krrishdholakia

Thanks @elisalimli

krrishdholakia avatar May 14 '24 18:05 krrishdholakia