Feat config server side store #5594
Conversation
…AI/OpenHands into feat-config-server-side-store
| SessionInitStoreImpl = get_impl(SessionInitStore, config.session_init_store_class) # type: ignore | ||
|
|
||
|
|
||
| @app.get('/session-init-data') |
There was a problem hiding this comment.
let's call this /user-config instead of session-init-data
There was a problem hiding this comment.
actually maybe /settings is better
| github_token='test-token', | ||
| selected_repository='test/repo', |
There was a problem hiding this comment.
definitely want to remove these two--this config is going to be reused across sessions
IMO the stuff in here should be 1:1 with the stuff in the settings modal
| @app.get('/session-init-data') | ||
| async def load_session_init_data( | ||
| github_auth: Annotated[str | None, Header()] = None, | ||
| ) -> SessionInitData | None: |
There was a problem hiding this comment.
I think probably we should create a new class for this config
…AI/OpenHands into feat-config-server-side-store
| @@ -0,0 +1,34 @@ | |||
| from __future__ import annotations | |||
There was a problem hiding this comment.
This is a hint that defers evaluation of annotations. Originally, this was going to be the default for future versions of python from 3.11 onwards, but they keep punting it.
Consider the following code:
from dataclasses import dataclass, field
@dataclass
class Node:
name: str
children: list[Node] = field(default_factory=list)
Without the annotation, the python will complain because the type hint for Node is used before the definition is complete. With the annotation, the evaluation for the type hint of Node is deferred until later and the code is accepted.
There are some internal changes on how type hints are processed (As strings), but those don't affect our use case
| @@ -0,0 +1,25 @@ | |||
| from __future__ import annotations | |||
There was a problem hiding this comment.
I understand the desire for an abstract class, but maybe for simplicity we just have the file implementation, and then any overriders inherit from that? Then we can just call it SettingsStore
There was a problem hiding this comment.
Also maybe we put these in a new dir, openhands/storage/settings/...
|
|
||
| @classmethod | ||
| async def get_instance(cls, config: AppConfig, token: str | None): | ||
| file_store = get_file_store(config.file_store, config.file_store_path) |
There was a problem hiding this comment.
I wonder if we should just have a singleton file_store. We do this in openhands/server/shared....
There was a problem hiding this comment.
We probably want a singleton of this class too...
I'm creating a ConversationStore, which now that I think about it is kind of silly (why give ourselves 10 classes to override instead of 1)...
Infrastructure for saving config on the backend rather than the frontend.
To run this PR locally, use the following command: