Remove singleton config (#3614)

* Remove singleton config

* Fix tests

* Fix logging reset

* Fix pre-commit
This commit is contained in:
Graham Neubig
2024-08-28 15:05:49 -04:00
committed by GitHub
parent f8c4d1df45
commit c6ba0e8339
5 changed files with 13 additions and 53 deletions

View File

@@ -12,7 +12,6 @@ import toml
from dotenv import load_dotenv
from openhands.core import logger
from openhands.core.utils import Singleton
load_dotenv()
@@ -143,7 +142,7 @@ class AgentConfig:
@dataclass
class SecurityConfig(metaclass=Singleton):
class SecurityConfig:
"""Configuration for security related functionalities.
Attributes:
@@ -176,7 +175,7 @@ class SecurityConfig(metaclass=Singleton):
@dataclass
class SandboxConfig(metaclass=Singleton):
class SandboxConfig:
"""Configuration for the sandbox.
Attributes:
@@ -243,7 +242,7 @@ class UndefinedString(str, Enum):
@dataclass
class AppConfig(metaclass=Singleton):
class AppConfig:
"""Configuration for the app.
Attributes:
@@ -567,7 +566,12 @@ def load_from_toml(cfg: AppConfig, toml_file: str = 'config.toml'):
sandbox_config = SandboxConfig(**toml_config['sandbox'])
# update the config object with the new values
AppConfig(sandbox=sandbox_config, **core_config)
cfg.sandbox = sandbox_config
for key, value in core_config.items():
if hasattr(cfg, key):
setattr(cfg, key, value)
else:
logger.openhands_logger.warning(f'Unknown core config key: {key}')
except (TypeError, KeyError) as e:
logger.openhands_logger.warning(
f'Cannot parse config from toml, toml values have not been applied.\nError: {e}',

View File

@@ -1,3 +0,0 @@
from openhands.core.utils.singleton import Singleton
__all__ = ['Singleton']

View File

@@ -1,37 +0,0 @@
import dataclasses
from openhands.core import logger
class Singleton(type):
_instances: dict = {}
def __call__(cls, *args, **kwargs):
if cls not in cls._instances:
cls._instances[cls] = super(Singleton, cls).__call__(*args, **kwargs)
else:
# allow updates, just update existing instance
# perhaps not the most orthodox way to do it, though it simplifies client code
# useful for pre-defined groups of settings
instance = cls._instances[cls]
for key, value in kwargs.items():
if hasattr(instance, key):
setattr(instance, key, value)
else:
logger.openhands_logger.warning(
f'Unknown key for {cls.__name__}: "{key}"'
)
return cls._instances[cls]
@classmethod
def reset(cls):
# used by pytest to reset the state of the singleton instances
for instance_type, instance in cls._instances.items():
print('resetting... ', instance_type)
for field_info in dataclasses.fields(instance_type):
if dataclasses.is_dataclass(field_info.type):
setattr(instance, field_info.name, field_info.type())
elif field_info.default_factory is not dataclasses.MISSING:
setattr(instance, field_info.name, field_info.default_factory())
else:
setattr(instance, field_info.name, field_info.default)

View File

@@ -40,7 +40,6 @@ def temp_toml_file(tmp_path):
@pytest.fixture
def default_config(monkeypatch):
# Fixture to provide a default AppConfig instance
AppConfig.reset()
yield AppConfig()
@@ -501,8 +500,8 @@ def test_api_keys_repr_str():
def test_max_iterations_and_max_budget_per_task_from_toml(temp_toml_file):
temp_toml = """
[core]
max_iterations = 100
max_budget_per_task = 4.0
max_iterations = 42
max_budget_per_task = 4.7
"""
config = AppConfig()
@@ -511,8 +510,8 @@ max_budget_per_task = 4.0
load_from_toml(config, temp_toml_file)
assert config.max_iterations == 100
assert config.max_budget_per_task == 4.0
assert config.max_iterations == 42
assert config.max_budget_per_task == 4.7
def test_get_llm_config_arg(temp_toml_file):

View File

@@ -87,9 +87,6 @@ def test_app_config_attributes_masking(test_handler):
assert 'e2b-xyz789' not in log_output
assert 'ghp_abcdefghijklmnopqrstuvwxyz' not in log_output
# reset the AppConfig
AppConfig.reset()
def test_sensitive_env_vars_masking(test_handler):
logger, stream = test_handler