Add websockets for streaming to a frontend#1414
Conversation
|
@sonichi the code formatting test is failing on scripts that I didn't write. Also, I have no idea why adding websockets would interfere with the WebSurfer tests, but it's happening with other PR's as well. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1414 +/- ##
==========================================
- Coverage 33.12% 4.17% -28.95%
==========================================
Files 42 41 -1
Lines 5051 5054 +3
Branches 1157 1225 +68
==========================================
- Hits 1673 211 -1462
- Misses 3250 4833 +1583
+ Partials 128 10 -118
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@ekzhu Is this more along the lines of what you were thinking? |
| self._oai_messages = defaultdict(list) | ||
| self._oai_system_message = [{"content": system_message, "role": "system"}] | ||
| self.description = description if description is not None else system_message | ||
| self.streaming_transmitter = streaming_transmitter |
There was a problem hiding this comment.
| self.streaming_transmitter = streaming_transmitter | |
| self._streaming_transmitter = streaming_transmitter |
There was a problem hiding this comment.
Use private as it is not really called directly from outside of the class.
There was a problem hiding this comment.
This file is not needed any more.
There was a problem hiding this comment.
Create tests in test/agentchat/test_conversable_agent_streaming.py.
| self, | ||
| *, | ||
| config_list: Optional[List[Dict[str, Any]]] = None, | ||
| streaming_transmitter: Optional[StreamingTransmitter] = None, |
There was a problem hiding this comment.
streaming_transmitter goes into the extra_kwargs list see above. It will be taken out of the base_config dictionary in the constructor. See how cache is used in create
Thanks for the update! Yes this is what I was thinking. We don't need to have separate @ragyabraham @joshkyh @victordibia @davorrunje does this current setup for streaming make sense to your guys from application point of view? |
There was a problem hiding this comment.
A giant PR is coming its way changing this file completely. #1345.
So perhaps it is a good idea to wait until that one gets merged before continue working on this one.
I'll defer to @ragyabraham, as I don't work usually work with sockets. :) |
|
@Tylersuard do you want me to look into failing tests? |
|
looks like we might be getting a dangling PR here. @davorrunje would you be interested in taking over this? |
|
@davorrunje @ekzhu Yes Davor, please look into the failing tests. Eric, I am still not sure what is required of me at this point. You asked me to write a protocol, and I think I did that? What else do I need to write or change?
…On Sun, Feb 4, 2024 at 2:10 AM, Eric Zhu ***@***.***(mailto:On Sun, Feb 4, 2024 at 2:10 AM, Eric Zhu <<a href=)> wrote:
looks like we might be getting a dangling PR here.
***@***.***(https://github.com/davorrunje) would you be interested in taking over this?
—
Reply to this email directly, [view it on GitHub](#1414 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AJ6H6YLQBFUR2FYPFCDUQA3YR5NBPAVCNFSM6AAAAABCMLELACVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRVGY3TINJZHA).
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
@Tylersuard you can see my comments in the code. There is a recent change to the |
@sonichi
Why are these changes needed?
If a user wants to have a frontend for an autogen-based chat, websockets will be required to stream information out of autogen. These changes will allow the user to stream content from a conversation to any address they choose. See https://websockets.readthedocs.io/en/stable/
Related issue number
#1199 , #1143 , #217 , #394
Checks