fix: Add support for output messages for sync/async #1188
Conversation
dosubot
bot
added
the
size:M
|
Possibly also resolves #1189 |
|
The stream tests I added work in that the stream parameter works with the litellm client and the updated handling. However, the result provided by the litellm client when using a mock_response is not actually representative of how streaming will work in a normal setting unfortunately. in actuality, the streamed chunks won't be populated automatically for the response. so I will need to remove the handling I added, as it only works for litellm client mocked responses. I will add a separate PR for streaming and will perhaps drop a note to the lite-lm maintainers to add some clarity around the client when using mock_response in the case of streaming. |
|
pinging @anticorrelator here to take a look cc @nate-mar |
|
Github pushes are down atm so I can't push my latest changes. FYI |
nate-mar
changed the title
| @@ -31,6 +31,7 @@ dependencies = [ | |||
| "openinference-instrumentation>=0.1.17", | |||
| "openinference-semantic-conventions>=0.1.9", | |||
| "wrapt", | |||
| "setuptools", | |||
| @@ -39,6 +40,7 @@ test = [ | |||
| "opentelemetry-sdk", | |||
| "opentelemetry-instrumentation-httpx", | |||
| "tenacity", | |||
| "tokenizers==0.20.3; python_version == '3.8'" | |||
There was a problem hiding this comment.
fixes build error related to bad version of tokenizers that needed to be yanked. for now, force an earlier version pre-the bad version; related to huggingface/tokenizers#1691
Yep, should resolve this too. |
|
hi @nate-mar if we want to handle streaming it looks like we need to add another handler for the All of their streaming logic is there and we can hook into what they're already doing to instrument the stream (such as compiling the final streamed message and attaching it to our span). Currently we don't handle this streaming return type and only handle |
|
Sounds good @anticorrelator ! For now, if you wouldn't mind giving me a review on this one so we can get it across the line first, that'd be great. Thanks! |
resolves #1060