From f79d77476e4cf37b4d66d0f7a0bcc74818945bcb Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Wed, 22 Jun 2022 12:50:54 -0400 Subject: [PATCH 1/6] feat: improve typing of Settings classes Signed-off-by: Daniel Bluhm --- aries_cloudagent/config/base.py | 24 ++++++++++++++++++------ aries_cloudagent/config/base_context.py | 4 ++-- aries_cloudagent/config/settings.py | 10 +++++----- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/aries_cloudagent/config/base.py b/aries_cloudagent/config/base.py index 92c48f4756..05e61720a9 100644 --- a/aries_cloudagent/config/base.py +++ b/aries_cloudagent/config/base.py @@ -1,7 +1,7 @@ """Configuration base classes.""" from abc import ABC, abstractmethod -from typing import Mapping, Optional, Type, TypeVar +from typing import Any, Iterator, Mapping, Optional, Type, TypeVar from ..core.error import BaseError @@ -16,7 +16,7 @@ class SettingsError(ConfigError): """The base exception raised by `BaseSettings` implementations.""" -class BaseSettings(Mapping[str, object]): +class BaseSettings(Mapping[str, Any]): """Base settings class.""" @abstractmethod @@ -42,6 +42,10 @@ def get_bool(self, *var_names, default=None) -> bool: value = self.get_value(*var_names, default) if value is not None: value = bool(value and value not in ("false", "False", "0")) + + if value is None: + raise KeyError(f"{var_names} not found in settings") + return value def get_int(self, *var_names, default=None) -> int: @@ -54,6 +58,10 @@ def get_int(self, *var_names, default=None) -> int: value = self.get_value(*var_names, default) if value is not None: value = int(value) + + if value is None: + raise KeyError(f"{var_names} not found in settings") + return value def get_str(self, *var_names, default=None) -> str: @@ -66,10 +74,14 @@ def get_str(self, *var_names, default=None) -> str: value = self.get_value(*var_names, default=default) if value is not None: value = str(value) + + if value is None: + raise KeyError(f"{var_names} not found in settings") + return value @abstractmethod - def __iter__(self): + def __iter__(self) -> Iterator: """Iterate settings keys.""" def __getitem__(self, index): @@ -91,7 +103,7 @@ def copy(self) -> "BaseSettings": """Produce a copy of the settings instance.""" @abstractmethod - def extend(self, other: Mapping[str, object]) -> "BaseSettings": + def extend(self, other: Mapping[str, Any]) -> "BaseSettings": """Merge another mapping to produce a new settings instance.""" def __repr__(self) -> str: @@ -111,7 +123,7 @@ class BaseInjector(ABC): def inject( self, base_cls: Type[InjectType], - settings: Mapping[str, object] = None, + settings: Optional[Mapping[str, Any]] = None, ) -> InjectType: """ Get the provided instance of a given class identifier. @@ -129,7 +141,7 @@ def inject( def inject_or( self, base_cls: Type[InjectType], - settings: Mapping[str, object] = None, + settings: Optional[Mapping[str, Any]] = None, default: Optional[InjectType] = None, ) -> Optional[InjectType]: """ diff --git a/aries_cloudagent/config/base_context.py b/aries_cloudagent/config/base_context.py index 4fde34759d..788a91940f 100644 --- a/aries_cloudagent/config/base_context.py +++ b/aries_cloudagent/config/base_context.py @@ -1,7 +1,7 @@ """Base injection context builder classes.""" from abc import ABC, abstractmethod -from typing import Mapping +from typing import Any, Mapping, Optional from .injection_context import InjectionContext from .settings import Settings @@ -10,7 +10,7 @@ class ContextBuilder(ABC): """Base injection context builder class.""" - def __init__(self, settings: Mapping[str, object] = None): + def __init__(self, settings: Optional[Mapping[str, Any]] = None): """ Initialize an instance of the context builder. diff --git a/aries_cloudagent/config/settings.py b/aries_cloudagent/config/settings.py index cc20aeb6f1..291182b68a 100644 --- a/aries_cloudagent/config/settings.py +++ b/aries_cloudagent/config/settings.py @@ -1,14 +1,14 @@ """Settings implementation.""" -from typing import Mapping +from typing import Any, Mapping, MutableMapping, Optional from .base import BaseSettings -class Settings(BaseSettings): +class Settings(BaseSettings, MutableMapping[str, Any]): """Mutable settings implementation.""" - def __init__(self, values: Mapping[str, object] = None): + def __init__(self, values: Optional[Mapping[str, Any]] = None): """Initialize a Settings object. Args: @@ -90,12 +90,12 @@ def copy(self) -> BaseSettings: """Produce a copy of the settings instance.""" return Settings(self._values) - def extend(self, other: Mapping[str, object]) -> BaseSettings: + def extend(self, other: Mapping[str, Any]) -> BaseSettings: """Merge another settings instance to produce a new instance.""" vals = self._values.copy() vals.update(other) return Settings(vals) - def update(self, other: Mapping[str, object]): + def update(self, other: Mapping[str, Any]): """Update the settings in place.""" self._values.update(other) From 707ec093b09f37732793a027e8a445687c8ae928 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Wed, 22 Jun 2022 12:52:11 -0400 Subject: [PATCH 2/6] feat: add plugin settings object and helpers Signed-off-by: Daniel Bluhm --- aries_cloudagent/config/argparse.py | 10 ++- aries_cloudagent/config/base.py | 5 ++ aries_cloudagent/config/plugin_settings.py | 77 +++++++++++++++++++ .../config/tests/test_settings.py | 27 +++++++ 4 files changed, 115 insertions(+), 4 deletions(-) create mode 100644 aries_cloudagent/config/plugin_settings.py diff --git a/aries_cloudagent/config/argparse.py b/aries_cloudagent/config/argparse.py index 2c2ac13dd4..fa0ef66255 100644 --- a/aries_cloudagent/config/argparse.py +++ b/aries_cloudagent/config/argparse.py @@ -18,6 +18,8 @@ from .error import ArgsParseError from .util import BoundedInt, ByteSize +from .plugin_settings import PLUGIN_CONFIG_KEY + CAT_PROVISION = "general" CAT_START = "start" CAT_UPGRADE = "upgrade" @@ -630,17 +632,17 @@ def get_settings(self, args: Namespace) -> dict: if args.plugin_config: with open(args.plugin_config, "r") as stream: - settings["plugin_config"] = yaml.safe_load(stream) + settings[PLUGIN_CONFIG_KEY] = yaml.safe_load(stream) if args.plugin_config_values: - if "plugin_config" not in settings: - settings["plugin_config"] = {} + if PLUGIN_CONFIG_KEY not in settings: + settings[PLUGIN_CONFIG_KEY] = {} for value_str in chain(*args.plugin_config_values): key, value = value_str.split("=", maxsplit=1) value = yaml.safe_load(value) deepmerge.always_merger.merge( - settings["plugin_config"], + settings[PLUGIN_CONFIG_KEY], reduce(lambda v, k: {k: v}, key.split(".")[::-1], value), ) diff --git a/aries_cloudagent/config/base.py b/aries_cloudagent/config/base.py index 05e61720a9..3ba405c085 100644 --- a/aries_cloudagent/config/base.py +++ b/aries_cloudagent/config/base.py @@ -4,6 +4,7 @@ from typing import Any, Iterator, Mapping, Optional, Type, TypeVar from ..core.error import BaseError +from .plugin_settings import PluginSettings InjectType = TypeVar("InjectType") @@ -111,6 +112,10 @@ def __repr__(self) -> str: items = ("{}={}".format(k, self[k]) for k in self) return "<{}({})>".format(self.__class__.__name__, ", ".join(items)) + def for_plugin(self, plugin: str, default: Optional[Mapping[str, Any]] = None): + """Retrieve settings for plugin.""" + return PluginSettings.for_plugin(self, plugin, default) + class InjectionError(ConfigError): """The base exception raised by Injector and Provider implementations.""" diff --git a/aries_cloudagent/config/plugin_settings.py b/aries_cloudagent/config/plugin_settings.py new file mode 100644 index 0000000000..278ccf8de4 --- /dev/null +++ b/aries_cloudagent/config/plugin_settings.py @@ -0,0 +1,77 @@ +"""Settings implementation for plugins.""" + +from typing import Any, Mapping, Optional + +from .base import BaseSettings + + +PLUGIN_CONFIG_KEY = "plugin_config" + + +class PluginSettings(BaseSettings): + """Retrieve immutable settings for plugins. + + Plugin settings should be retrieved by calling: + + PluginSettings.for_plugin(settings, "my_plugin", {"default": "values"}) + + This will extract the PLUGIN_CONFIG_KEY in "settings" and return a new + PluginSettings instance. + """ + + def __init__(self, values: Optional[Mapping[str, Any]] = None): + """Initialize a Settings object. + + Args: + values: An optional dictionary of settings + """ + self._values = {} + if values: + self._values.update(values) + + def __contains__(self, index): + """Define 'in' operator.""" + return index in self._values + + def __iter__(self): + """Iterate settings keys.""" + return iter(self._values) + + def __len__(self): + """Fetch the length of the mapping.""" + return len(self._values) + + def __bool__(self): + """Convert settings to a boolean.""" + return True + + def copy(self) -> BaseSettings: + """Produce a copy of the settings instance.""" + return PluginSettings(self._values) + + def extend(self, other: Mapping[str, Any]) -> BaseSettings: + """Merge another settings instance to produce a new instance.""" + vals = self._values.copy() + vals.update(other) + return PluginSettings(vals) + + def get_value(self, *var_names: str, default: Any = None): + """Fetch a setting. + + Args: + var_names: A list of variable name alternatives + default: The default value to return if none are defined + """ + for k in var_names: + if k in self._values: + return self._values[k] + return default + + @classmethod + def for_plugin( + cls, + settings: BaseSettings, + plugin: str, + default: Optional[Mapping[str, Any]] = None, + ) -> "PluginSettings": + return cls(settings.get(PLUGIN_CONFIG_KEY, {}).get(plugin, default)) diff --git a/aries_cloudagent/config/tests/test_settings.py b/aries_cloudagent/config/tests/test_settings.py index 583d8a42ef..d95948aedd 100644 --- a/aries_cloudagent/config/tests/test_settings.py +++ b/aries_cloudagent/config/tests/test_settings.py @@ -2,8 +2,11 @@ from unittest import TestCase +from aries_cloudagent.config.plugin_settings import PluginSettings + from ..base import SettingsError from ..settings import Settings +from ..plugin_settings import PLUGIN_CONFIG_KEY class TestSettings(TestCase): @@ -59,3 +62,27 @@ def test_set_default(self): assert self.test_instance[self.test_key] == self.test_value self.test_instance.set_default("BOOL", "True") assert self.test_instance["BOOL"] == "True" + + def test_plugin_setting_retrieval(self): + plugin_setting_values = { + "value0": 0, + "value1": 1, + "value2": 2, + "value3": 3, + "value4": 4, + } + self.test_instance[PLUGIN_CONFIG_KEY] = {"my_plugin": plugin_setting_values} + + plugin_settings = self.test_instance.for_plugin("my_plugin") + assert isinstance(plugin_settings, PluginSettings) + assert plugin_settings._values == plugin_setting_values + for key in plugin_setting_values: + assert key in plugin_settings + assert plugin_settings[key] == plugin_setting_values[key] + assert ( + plugin_settings.get_value(self.test_key) == plugin_setting_values[key] + ) + with self.assertRaises(KeyError): + plugin_settings["MISSING"] + assert len(plugin_settings) == 5 + assert len(plugin_settings) == 5 From f203a5be2fc15fb7f1948f092211b6883b439572 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Wed, 22 Jun 2022 13:14:34 -0400 Subject: [PATCH 3/6] fix: misplaced helper method Signed-off-by: Daniel Bluhm --- aries_cloudagent/config/base.py | 5 ----- aries_cloudagent/config/settings.py | 5 +++++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/aries_cloudagent/config/base.py b/aries_cloudagent/config/base.py index 3ba405c085..05e61720a9 100644 --- a/aries_cloudagent/config/base.py +++ b/aries_cloudagent/config/base.py @@ -4,7 +4,6 @@ from typing import Any, Iterator, Mapping, Optional, Type, TypeVar from ..core.error import BaseError -from .plugin_settings import PluginSettings InjectType = TypeVar("InjectType") @@ -112,10 +111,6 @@ def __repr__(self) -> str: items = ("{}={}".format(k, self[k]) for k in self) return "<{}({})>".format(self.__class__.__name__, ", ".join(items)) - def for_plugin(self, plugin: str, default: Optional[Mapping[str, Any]] = None): - """Retrieve settings for plugin.""" - return PluginSettings.for_plugin(self, plugin, default) - class InjectionError(ConfigError): """The base exception raised by Injector and Provider implementations.""" diff --git a/aries_cloudagent/config/settings.py b/aries_cloudagent/config/settings.py index 291182b68a..d4a7677d13 100644 --- a/aries_cloudagent/config/settings.py +++ b/aries_cloudagent/config/settings.py @@ -3,6 +3,7 @@ from typing import Any, Mapping, MutableMapping, Optional from .base import BaseSettings +from .plugin_settings import PluginSettings class Settings(BaseSettings, MutableMapping[str, Any]): @@ -99,3 +100,7 @@ def extend(self, other: Mapping[str, Any]) -> BaseSettings: def update(self, other: Mapping[str, Any]): """Update the settings in place.""" self._values.update(other) + + def for_plugin(self, plugin: str, default: Optional[Mapping[str, Any]] = None): + """Retrieve settings for plugin.""" + return PluginSettings.for_plugin(self, plugin, default) From e464c64e315a963fdde511f0fc491b7332af17c4 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Wed, 22 Jun 2022 13:22:20 -0400 Subject: [PATCH 4/6] style: docstring for for_plugin Signed-off-by: Daniel Bluhm --- aries_cloudagent/config/plugin_settings.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/aries_cloudagent/config/plugin_settings.py b/aries_cloudagent/config/plugin_settings.py index 278ccf8de4..d1e78cdb74 100644 --- a/aries_cloudagent/config/plugin_settings.py +++ b/aries_cloudagent/config/plugin_settings.py @@ -74,4 +74,8 @@ def for_plugin( plugin: str, default: Optional[Mapping[str, Any]] = None, ) -> "PluginSettings": + """Construct a PluginSettings object from another settings object. + + PLUGIN_CONFIG_KEY is read from settings. + """ return cls(settings.get(PLUGIN_CONFIG_KEY, {}).get(plugin, default)) From a375b616aa9638944e5d92110469ef5fa1ed3235 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Wed, 22 Jun 2022 13:24:00 -0400 Subject: [PATCH 5/6] fix: test case checking wrong key Signed-off-by: Daniel Bluhm --- aries_cloudagent/config/tests/test_settings.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/aries_cloudagent/config/tests/test_settings.py b/aries_cloudagent/config/tests/test_settings.py index d95948aedd..3415d4c260 100644 --- a/aries_cloudagent/config/tests/test_settings.py +++ b/aries_cloudagent/config/tests/test_settings.py @@ -79,9 +79,7 @@ def test_plugin_setting_retrieval(self): for key in plugin_setting_values: assert key in plugin_settings assert plugin_settings[key] == plugin_setting_values[key] - assert ( - plugin_settings.get_value(self.test_key) == plugin_setting_values[key] - ) + assert plugin_settings.get_value(key) == plugin_setting_values[key] with self.assertRaises(KeyError): plugin_settings["MISSING"] assert len(plugin_settings) == 5 From 67173b2c1be8cd9e60a2a1492ba7c5e1f932fbcd Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Wed, 22 Jun 2022 13:30:54 -0400 Subject: [PATCH 6/6] fix: optional rather than error for setting get helpers Signed-off-by: Daniel Bluhm --- aries_cloudagent/config/base.py | 17 ++++------------- aries_cloudagent/transport/inbound/ws.py | 5 +++-- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/aries_cloudagent/config/base.py b/aries_cloudagent/config/base.py index 05e61720a9..cdc30ba507 100644 --- a/aries_cloudagent/config/base.py +++ b/aries_cloudagent/config/base.py @@ -20,7 +20,7 @@ class BaseSettings(Mapping[str, Any]): """Base settings class.""" @abstractmethod - def get_value(self, *var_names, default=None): + def get_value(self, *var_names, default: Optional[Any] = None) -> Any: """Fetch a setting. Args: @@ -32,7 +32,7 @@ def get_value(self, *var_names, default=None): """ - def get_bool(self, *var_names, default=None) -> bool: + def get_bool(self, *var_names, default: Optional[bool] = None) -> Optional[bool]: """Fetch a setting as a boolean value. Args: @@ -43,12 +43,9 @@ def get_bool(self, *var_names, default=None) -> bool: if value is not None: value = bool(value and value not in ("false", "False", "0")) - if value is None: - raise KeyError(f"{var_names} not found in settings") - return value - def get_int(self, *var_names, default=None) -> int: + def get_int(self, *var_names, default: Optional[int] = None) -> Optional[int]: """Fetch a setting as an integer value. Args: @@ -59,12 +56,9 @@ def get_int(self, *var_names, default=None) -> int: if value is not None: value = int(value) - if value is None: - raise KeyError(f"{var_names} not found in settings") - return value - def get_str(self, *var_names, default=None) -> str: + def get_str(self, *var_names, default: Optional[str] = None) -> Optional[str]: """Fetch a setting as a string value. Args: @@ -75,9 +69,6 @@ def get_str(self, *var_names, default=None) -> str: if value is not None: value = str(value) - if value is None: - raise KeyError(f"{var_names} not found in settings") - return value @abstractmethod diff --git a/aries_cloudagent/transport/inbound/ws.py b/aries_cloudagent/transport/inbound/ws.py index 0b74a46990..0348146b9e 100644 --- a/aries_cloudagent/transport/inbound/ws.py +++ b/aries_cloudagent/transport/inbound/ws.py @@ -2,6 +2,7 @@ import asyncio import logging +from typing import Optional from aiohttp import WSMessage, WSMsgType, web @@ -30,10 +31,10 @@ def __init__(self, host: str, port: int, create_session, **kwargs) -> None: self.host = host self.port = port self.site: web.BaseSite = None - self.heartbeat_interval: int = self.root_profile.settings.get_int( + self.heartbeat_interval: Optional[int] = self.root_profile.settings.get_int( "transport.ws.heartbeat_interval" ) - self.timout_interval: int = self.root_profile.settings.get_int( + self.timout_interval: Optional[int] = self.root_profile.settings.get_int( "transport.ws.timout_interval" )