Skip to content

Commit

Permalink
add extended generic section (#5932)
Browse files Browse the repository at this point in the history
Co-authored-by: Christophe Elek <[email protected]>
Co-authored-by: Engel Nyst <[email protected]>
  • Loading branch information
3 people authored Feb 24, 2025
1 parent f4c5bbd commit fa50e0c
Show file tree
Hide file tree
Showing 5 changed files with 218 additions and 0 deletions.
2 changes: 2 additions & 0 deletions openhands/core/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
OH_MAX_ITERATIONS,
get_field_info,
)
from openhands.core.config.extended_config import ExtendedConfig
from openhands.core.config.llm_config import LLMConfig
from openhands.core.config.sandbox_config import SandboxConfig
from openhands.core.config.security_config import SecurityConfig
Expand All @@ -28,6 +29,7 @@
'LLMConfig',
'SandboxConfig',
'SecurityConfig',
'ExtendedConfig',
'load_app_config',
'load_from_env',
'load_from_toml',
Expand Down
2 changes: 2 additions & 0 deletions openhands/core/config/app_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
OH_MAX_ITERATIONS,
model_defaults_to_dict,
)
from openhands.core.config.extended_config import ExtendedConfig
from openhands.core.config.llm_config import LLMConfig
from openhands.core.config.sandbox_config import SandboxConfig
from openhands.core.config.security_config import SecurityConfig
Expand Down Expand Up @@ -52,6 +53,7 @@ class AppConfig(BaseModel):
default_agent: str = Field(default=OH_DEFAULT_AGENT)
sandbox: SandboxConfig = Field(default_factory=SandboxConfig)
security: SecurityConfig = Field(default_factory=SecurityConfig)
extended: ExtendedConfig = Field(default_factory=lambda: ExtendedConfig({}))
runtime: str = Field(default='docker')
file_store: str = Field(default='local')
file_store_path: str = Field(default='/tmp/openhands_file_store')
Expand Down
40 changes: 40 additions & 0 deletions openhands/core/config/extended_config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
from pydantic import RootModel


class ExtendedConfig(RootModel[dict]):
"""Configuration for extended functionalities.
This is implemented as a root model so that the entire input is stored
as the root value. This allows arbitrary keys to be stored and later
accessed via attribute or dictionary-style access.
"""

@property
def root(self) -> dict: # type annotation to help mypy
return super().root

def __str__(self) -> str:
# Use the root dict to build a string representation.
attr_str = [f'{k}={repr(v)}' for k, v in self.root.items()]
return f"ExtendedConfig({', '.join(attr_str)})"

def __repr__(self) -> str:
return self.__str__()

@classmethod
def from_dict(cls, data: dict) -> 'ExtendedConfig':
# Create an instance directly by wrapping the input dict.
return cls(data)

def __getitem__(self, key: str) -> object:
# Provide dictionary-like access via the root dict.
return self.root[key]

def __getattr__(self, key: str) -> object:
# Fallback for attribute access using the root dict.
try:
return self.root[key]
except KeyError as e:
raise AttributeError(
f"'ExtendedConfig' object has no attribute '{key}'"
) from e
5 changes: 5 additions & 0 deletions openhands/core/config/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
OH_DEFAULT_AGENT,
OH_MAX_ITERATIONS,
)
from openhands.core.config.extended_config import ExtendedConfig
from openhands.core.config.llm_config import LLMConfig
from openhands.core.config.sandbox_config import SandboxConfig
from openhands.core.config.security_config import SecurityConfig
Expand Down Expand Up @@ -134,6 +135,10 @@ def load_from_toml(cfg: AppConfig, toml_file: str = 'config.toml') -> None:
for key, value in toml_config.items():
if isinstance(value, dict):
try:
if key.lower() == 'extended':
# For ExtendedConfig (RootModel), pass the entire dict as the root value
cfg.extended = ExtendedConfig(value)
continue
if key is not None and key.lower() == 'agent':
# Every entry here is either a field for the default `agent` config group, or itself a group
# The best way to tell the difference is to try to parse it as an AgentConfig object
Expand Down
169 changes: 169 additions & 0 deletions tests/unit/test_config_extended.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
import os

import pytest

from openhands.core.config.app_config import AppConfig
from openhands.core.config.extended_config import ExtendedConfig
from openhands.core.config.utils import load_from_toml


def test_extended_config_from_dict():
"""
Test that ExtendedConfig.from_dict successfully creates an instance
from a dictionary containing arbitrary extra keys.
"""
data = {'foo': 'bar', 'baz': 123, 'flag': True}
ext_cfg = ExtendedConfig.from_dict(data)

# Check that the keys are accessible both as attributes and via __getitem__
assert ext_cfg.foo == 'bar'
assert ext_cfg['baz'] == 123
assert ext_cfg.flag is True
# Verify the root dictionary contains all keys
assert ext_cfg.root == data


def test_extended_config_empty():
"""
Test that an empty ExtendedConfig can be created and accessed.
"""
ext_cfg = ExtendedConfig.from_dict({})
assert ext_cfg.root == {}

# Creating directly should also work
ext_cfg2 = ExtendedConfig({})
assert ext_cfg2.root == {}


def test_extended_config_str_and_repr():
"""
Test that __str__ and __repr__ return the correct string representations
of the ExtendedConfig instance.
"""
data = {'alpha': 'test', 'beta': 42}
ext_cfg = ExtendedConfig.from_dict(data)
string_repr = str(ext_cfg)
repr_str = repr(ext_cfg)

# Ensure the representations include our key/value pairs
assert "alpha='test'" in string_repr
assert 'beta=42' in string_repr

# __repr__ should match __str__
assert string_repr == repr_str


def test_extended_config_getitem_and_getattr():
"""
Test that __getitem__ and __getattr__ can be used to access values
in the ExtendedConfig instance.
"""
data = {'key1': 'value1', 'key2': 2}
ext_cfg = ExtendedConfig.from_dict(data)

# Attribute access
assert ext_cfg.key1 == 'value1'
# Dictionary-style access
assert ext_cfg['key2'] == 2


def test_extended_config_invalid_key():
"""
Test that accessing a non-existent key via attribute access raises AttributeError.
"""
data = {'existing': 'yes'}
ext_cfg = ExtendedConfig.from_dict(data)

with pytest.raises(AttributeError):
_ = ext_cfg.nonexistent

with pytest.raises(KeyError):
_ = ext_cfg['nonexistent']


def test_app_config_extended_from_toml(tmp_path: os.PathLike) -> None:
"""
Test that the [extended] section in a TOML file is correctly loaded into
AppConfig.extended and that it accepts arbitrary keys.
"""
# Create a temporary TOML file with multiple sections including [extended]
config_content = """
[core]
workspace_base = "/tmp/workspace"
[llm]
model = "test-model"
api_key = "toml-api-key"
[extended]
custom1 = "custom_value"
custom2 = 42
llm = "overridden" # even a key like 'llm' is accepted in extended
[agent]
memory_enabled = true
"""
config_file = tmp_path / 'config.toml'
config_file.write_text(config_content)

# Load the TOML into the AppConfig instance
config = AppConfig()
load_from_toml(config, str(config_file))

# Verify that extended section is applied
assert config.extended.custom1 == 'custom_value'
assert config.extended.custom2 == 42
# Even though 'llm' is defined in extended, it should not affect the main llm config.
assert config.get_llm_config().model == 'test-model'


def test_app_config_extended_default(tmp_path: os.PathLike) -> None:
"""
Test that if there is no [extended] section in the TOML file,
AppConfig.extended remains its default (empty) ExtendedConfig.
"""
config_content = """
[core]
workspace_base = "/tmp/workspace"
[llm]
model = "test-model"
api_key = "toml-api-key"
[agent]
memory_enabled = true
"""
config_file = tmp_path / 'config.toml'
config_file.write_text(config_content)

config = AppConfig()
load_from_toml(config, str(config_file))

# Extended config should be empty
assert config.extended.root == {}


def test_app_config_extended_random_keys(tmp_path: os.PathLike) -> None:
"""
Test that the extended section accepts arbitrary keys,
including ones not defined in any schema.
"""
config_content = """
[core]
workspace_base = "/tmp/workspace"
[extended]
random_key = "random_value"
another_key = 3.14
"""
config_file = tmp_path / 'config.toml'
config_file.write_text(config_content)

config = AppConfig()
load_from_toml(config, str(config_file))

# Verify that extended config holds the arbitrary keys with correct values.
assert config.extended.random_key == 'random_value'
assert config.extended.another_key == 3.14
# Verify the root dictionary contains all keys
assert config.extended.root == {'random_key': 'random_value', 'another_key': 3.14}

0 comments on commit fa50e0c

Please sign in to comment.