OpenHands icon indicating copy to clipboard operation
OpenHands copied to clipboard

Separate event store from event stream

Open tofarr opened this issue 9 months ago • 1 comments

  • [ ] This change is worth documenting at https://docs.all-hands.dev/
  • [ ] Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below

End-user friendly description of the problem this fixes or functionality that this introduces. This PR separates the concerns of threading and coordination from the concerns of event storage - a new class StoredEventList is introduced to this end, and EventStream now extends this.


Give a summary of what the PR does, explaining any non-trivial design decisions. The event stream is designed to coordinate read / write access for events in a conversation. This is overkill for the return values from the conversation manager - we just want to expose a way to read the existing events for a conversation in it's return values. (This extra flexibility is required in the SAAS env where clustering is in play, and the conversation may be running remotely in a different worker / process).


Link of any specific issues this addresses.


To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:ecdb72e-nikolaik   --name openhands-app-ecdb72e   docker.all-hands.dev/all-hands-ai/openhands:ecdb72e

tofarr avatar Mar 30 '25 23:03 tofarr

@enyst / @rbren - I've made some updates here...

  • Renamed StoredEventList -> EventStore (Following the pattern of SettingsStore, ConversationStore, etc...)
  • Changed the EventStore class so that it is possible to create instances without recalculating the cur_id - so we can create these easily and cheaply. (The default_value is -1 and we use the dataclass post_init to fix this if it is < 0)
  • The conversation manager now returns an EventStore instance, so it is not possible for somebody to accidentally cast it back to a stream.

tofarr avatar Apr 01 '25 23:04 tofarr