OpenHands icon indicating copy to clipboard operation
OpenHands copied to clipboard

Config rework

Open enyst opened this issue 1 year ago • 3 comments

Not for review yet. It's partially backported (messy stuff) and Claude was playing too much. 😅

This PR proposes a fix, the addition of a new small config for memory, and a few refactorings for readability:

  • [x] fix regression when the user has custom llms defined, and an [llm] section: if some attributes are missing from a custom llm definition, but are defined in [llm], it should fallback to those in [llm].
  • [x] break up the embeddings part from LlmConfig into a MemoryConfig
  • [x] refactoring: we're starting to have too many classes and they have duplicate methods, introduce an ABC
  • [x] refactoring: break up the long functions loading from toml and env, into loading per dataclass: each little config knows better what it needs to load and how to deal with some specifics.
  • [ ] better documentation.

enyst avatar Aug 18 '24 23:08 enyst

Great job tackling this! Having more docstrings alone will make a difference.

tobitege avatar Aug 19 '24 04:08 tobitege

Hmm... is your branch a bit outdated? 🤔

tobitege avatar Sep 12 '24 18:09 tobitege

I thought I did git pull upstream main... 🤷 Maybe it got outdated again because y'all are too fast and hard working! 😅

enyst avatar Sep 12 '24 18:09 enyst