From 94e99cc46cb90b01ebf11dcdd2f810cd24cabc9d Mon Sep 17 00:00:00 2001 From: Jan Segre Date: Tue, 23 May 2023 20:33:34 +0200 Subject: [PATCH] refactor(settings): make HathorSettings always return the same instance This wasn't _exactly_ the case before this commit. On one side when using the deprecated module settings the same module would always be loaded, and the same `SETTINGS` property would be accessed, but reloading the module (or at least leaving that up to importlib) unnecessary. On the new YAML system the file is reloaded everytime, this is also unnecessary. This commit will make it the last HathorSettings instantiated will be returned, as long as the source is the same, otherwise an exception is raised. There is a little small caveat that is fixed in this commit is that in the YAML file could in theory be altered between different calls to HathorSettings, which would lead to different content being loaded silently. What happens now is that the first time it is loaded is what is used for every call, any change in the file won't have any effect. --- hathor/builder/cli_builder.py | 5 +- hathor/conf/get_settings.py | 88 +++++++++++++++++++---------------- 2 files changed, 51 insertions(+), 42 deletions(-) diff --git a/hathor/builder/cli_builder.py b/hathor/builder/cli_builder.py index 7d3a9e040..420917a0e 100644 --- a/hathor/builder/cli_builder.py +++ b/hathor/builder/cli_builder.py @@ -55,7 +55,7 @@ def check_or_raise(self, condition: bool, message: str) -> None: def create_manager(self, reactor: PosixReactorBase, args: Namespace) -> HathorManager: import hathor from hathor.conf import HathorSettings - from hathor.conf.get_settings import get_settings_filepath, get_settings_module + from hathor.conf.get_settings import get_settings_source from hathor.daa import TestMode, _set_test_mode from hathor.event.storage import EventMemoryStorage, EventRocksDBStorage, EventStorage from hathor.event.websocket.factory import EventWebsocketFactory @@ -74,8 +74,7 @@ def create_manager(self, reactor: PosixReactorBase, args: Namespace) -> HathorMa settings = HathorSettings() # only used for logging its location - settings_module = get_settings_module() - settings_source = settings_module.__file__ if settings_module is not None else get_settings_filepath() + settings_source = get_settings_source() self.log = logger.new() self.reactor = reactor diff --git a/hathor/conf/get_settings.py b/hathor/conf/get_settings.py index d555204f1..2cf8b46d7 100644 --- a/hathor/conf/get_settings.py +++ b/hathor/conf/get_settings.py @@ -14,8 +14,7 @@ import importlib import os -from types import ModuleType -from typing import Optional +from typing import NamedTuple, Optional from structlog import get_logger @@ -24,8 +23,14 @@ logger = get_logger() -_settings_filepath: Optional[str] = None -_config_file: Optional[str] = None + +class _SettingsMetadata(NamedTuple): + source: str + is_yaml: bool + settings: Settings + + +_settings_singleton: Optional[_SettingsMetadata] = None def HathorSettings() -> Settings: @@ -37,52 +42,57 @@ def HathorSettings() -> Settings: If neither is set, or if the module import fails, the mainnet configuration is returned. """ - settings_module = get_settings_module() - if settings_module is not None: - log = logger.new() - log.warn( - "Setting a config module via the 'HATHOR_CONFIG_FILE' env var will be deprecated soon. " - "Use the '--config-yaml' CLI option or the 'HATHOR_CONFIG_YAML' env var to set a yaml filepath instead." - ) - settings = getattr(settings_module, 'SETTINGS') - assert isinstance(settings, Settings) - return settings + settings_module_filepath = os.environ.get('HATHOR_CONFIG_FILE') + if settings_module_filepath is not None: + return _load_settings_singleton(settings_module_filepath, is_yaml=False) - settings_filepath = get_settings_filepath() + settings_yaml_filepath = os.environ.get('HATHOR_CONFIG_YAML', conf.MAINNET_SETTINGS_FILEPATH) + return _load_settings_singleton(settings_yaml_filepath, is_yaml=True) - return Settings.from_yaml(filepath=settings_filepath) +def get_settings_source() -> str: + """ Returns the path of the settings module or YAML file that was loaded. + + XXX: Will raise an assertion error if HathorSettings() wasn't used before. + """ + global _settings_singleton + assert _settings_singleton is not None, 'HathorSettings() not called before' + return _settings_singleton.source -def get_settings_module() -> Optional[ModuleType]: - global _config_file - # Import config file for network - config_file = os.environ.get('HATHOR_CONFIG_FILE') - if _config_file is None: - _config_file = config_file - elif _config_file != config_file: - raise Exception('loading config twice with a different file') - if not config_file: - return None +def _load_settings_singleton(source: str, *, is_yaml: bool) -> Settings: + global _settings_singleton - try: - module = importlib.import_module(config_file) - except ModuleNotFoundError: - default_file = 'hathor.conf.mainnet' - module = importlib.import_module(default_file) + if _settings_singleton is not None: + if _settings_singleton.is_yaml != is_yaml: + raise Exception('loading config twice with a different file type') + if _settings_singleton.source != source: + raise Exception('loading config twice with a different file') - return module + return _settings_singleton.settings + settings_loader = _load_yaml_settings if is_yaml else _load_module_settings + _settings_singleton = _SettingsMetadata( + source=source, + is_yaml=is_yaml, + settings=settings_loader(source) + ) -def get_settings_filepath() -> str: - global _settings_filepath + return _settings_singleton.settings - new_settings_filepath = os.environ.get('HATHOR_CONFIG_YAML', conf.MAINNET_SETTINGS_FILEPATH) - if _settings_filepath is not None and _settings_filepath != new_settings_filepath: - raise Exception('loading config twice with a different file') +def _load_module_settings(module_path: str) -> Settings: + log = logger.new() + log.warn( + "Setting a config module via the 'HATHOR_CONFIG_FILE' env var will be deprecated soon. " + "Use the '--config-yaml' CLI option or the 'HATHOR_CONFIG_YAML' env var to set a yaml filepath instead." + ) + settings_module = importlib.import_module(module_path) + settings = getattr(settings_module, 'SETTINGS') + assert isinstance(settings, Settings) + return settings - _settings_filepath = new_settings_filepath - return new_settings_filepath +def _load_yaml_settings(filepath: str) -> Settings: + return Settings.from_yaml(filepath=filepath)