Separate event store from event stream
- [ ] 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
@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.