diff --git a/nomad/cli/cli.py b/nomad/cli/cli.py index 3461cff3f58b9a1598bce51d6343ada72124d3e6..f5ecd70b58411563c4da1baf78838a2f6ebbf3d7 100644 --- a/nomad/cli/cli.py +++ b/nomad/cli/cli.py @@ -98,14 +98,10 @@ class POPO(dict): It uses a sub-command structure similar to the git command.''') @click.option('-v', '--verbose', help='sets log level to info', is_flag=True) @click.option('--debug', help='sets log level to debug', is_flag=True) -@click.option('--config', help='the config file to use') @click.pass_context -def cli(ctx, verbose: bool, debug: bool, config: str): +def cli(ctx, verbose: bool, debug: bool): nomad_config.meta.service = os.environ.get('NOMAD_SERVICE', 'cli') - if config is not None: - nomad_config.load_config(config_file=config) - if debug: utils.set_console_log_level(logging.DEBUG) elif verbose: diff --git a/nomad/config.py b/nomad/config.py index 890251f167b29d2ffbcfb98cdd46c06e9a056889..9310a2c3220c12962903fbd2c049f84cd6af16a1 100644 --- a/nomad/config.py +++ b/nomad/config.py @@ -194,7 +194,7 @@ def gui_url(page: str = None): return '%s/gui' % base -def check_config(): +def _check_config(): """Used to check that the current configuration is valid. Should only be called once after the final config is loaded. @@ -324,7 +324,7 @@ def normalize_loglevel(value, default_level=logging.INFO): return getattr(logging, plain_value) -transformations = { +_transformations = { 'console_log_level': normalize_loglevel, 'logstash_level': normalize_loglevel } @@ -334,113 +334,85 @@ transformations = { logger = logging.getLogger(__name__) -def apply(key, value) -> None: +def _apply(key, value, raise_error: bool = True) -> None: ''' - Changes the config according to given key and value. The keys are interpreted as paths - to config values with ``_`` as a separator. E.g. ``fs_staging`` leading to - ``config.fs.staging`` + Changes the config according to given key and value. The first part of a key + (with ``_`` as a separator) is interpreted as a group of settings. E.g. ``fs_staging`` + leading to ``config.fs.staging``. ''' - path = list(reversed(key.split('_'))) - child_segment = None - current_value = None - child_config = globals() - child_key = None + full_key = key + group_key, config_key = full_key.split('_', 1) + current = globals() + + if group_key not in current: + if key not in current: + if raise_error: + logger.error(f'config key does not exist: {full_key}') + return + else: + current = current[group_key] + if not isinstance(current, NomadConfig): + if raise_error: + logger.error(f'config key does not exist: {full_key}') + return - try: - while len(path) > 0: - if child_segment is None: - child_segment = path.pop() - else: - child_segment += '_' + path.pop() - - if child_segment in child_config: - current_value = child_config[child_segment] - - if current_value is None: - if len(path) == 0: - raise KeyError - - continue - if isinstance(current_value, NomadConfig): - child_config = current_value - current_value = None - child_segment = None - else: - if len(path) > 0: - raise KeyError() - - child_key = child_segment - break - - if child_key is None or current_value is None: - raise KeyError() - except KeyError: - return + if config_key not in current: + if raise_error: + logger.error(f'config key does not exist: {full_key}') + return - if not isinstance(value, type(current_value)): - try: - value = transformations.get(key, type(current_value))(value) - except Exception as e: - logger.error( - 'config key %s value %s has wrong type: %s' % (key, str(value), str(e))) + key = config_key - child_config[child_key] = value + try: + current_value = current[key] + if current_value is not None and not isinstance(value, type(current_value)): + value = _transformations.get(full_key, type(current_value))(value) + current[key] = value + logger.info(f'set config setting {full_key}={value}') + except Exception as e: + logger.error(f'cannot set config setting {full_key}={value}: {e}') -def load_config(config_file: str = os.environ.get('NOMAD_CONFIG', 'nomad.yaml')) -> None: - ''' - Loads the configuration from the ``config_file`` and environment. - Arguments: - config_file: Override the configfile, default is file stored in env variable - NOMAD_CONFIG or ``nomad.yaml``. - ''' - # load yaml and override defaults (only when not in test) - if os.path.exists(config_file): - with open(config_file, 'r') as stream: - try: - config_data = yaml.load(stream, Loader=getattr(yaml, 'FullLoader')) - except yaml.YAMLError as e: - logger.error('cannot read nomad config', exc_info=e) - - def adapt(config, new_config, child_key=None): - for key, value in new_config.items(): - if key in config: - if child_key is None: - qualified_key = key - else: - qualified_key = '%s_%s' % (child_key, key) - - current_value = config[key] - if isinstance(value, dict) and isinstance(current_value, NomadConfig): - adapt(current_value, value, qualified_key) - else: - if not isinstance(value, type(current_value)): - try: - value = transformations.get(qualified_key, type(current_value))(value) - except Exception as e: - logger.error( - 'config key %s value %s has wrong type: %s' % (key, str(value), str(e))) - else: - config[key] = value - logger.debug('override config key %s with value %s' % (key, str(value))) - else: - logger.error('config key %s does not exist' % key) - - if config_data is not None: - adapt(globals(), config_data) - - # load env and override yaml and defaults +def _apply_env_variables(): kwargs = { key[len('NOMAD_'):].lower(): value for key, value in os.environ.items() - if key.startswith('NOMAD_') - } + if key.startswith('NOMAD_') and key != 'NOMAD_CONFIG'} for key, value in kwargs.items(): - apply(key, value) + _apply(key, value, raise_error=False) + - check_config() +def _apply_nomad_yaml(): + config_file = os.environ.get('NOMAD_CONFIG', 'nomad.yaml') + + if not os.path.exists(config_file): + return + + with open(config_file, 'r') as stream: + try: + config_data = yaml.load(stream, Loader=getattr(yaml, 'FullLoader')) + except yaml.YAMLError as e: + logger.error(f'cannot read nomad config: {e}') + return + + for key, value in config_data.items(): + if isinstance(value, dict): + group_key = key + for key, value in value.items(): + _apply(f'{group_key}_{key}', value) + else: + _apply(key, value) + + +def load_config(): + ''' + Loads the configuration from nomad.yaml and environment. + ''' + _apply_nomad_yaml() + _apply_env_variables() + _check_config() load_config() diff --git a/tests/test_config.py b/tests/test_config.py index 546f7008b2b4d8ae1a567e2eb346c2a3861de651..06b8afbb6ce5fc3d445afbf620b5ad74f1e4d8ce 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -17,17 +17,72 @@ # import pytest +import os +import yaml from nomad import config +from .utils import assert_log + @pytest.fixture def with_config(): - old_value = config.fs.public + old_values = config.fs.public, config.fs.archive_version_suffix, config.auxfile_cutoff yield config - config.fs.public = old_value + config.fs.public, config.fs.archive_version_suffix, config.auxfile_cutoff = old_values + + +def test_apply(with_config, caplog): + config._apply('fs_public', 'test_value') + assert config.fs.public == 'test_value' + + config._apply('fs_archive_version_suffix', 'test_value') + assert config.fs.archive_version_suffix == 'test_value' + + config._apply('auxfile_cutoff', '200') + assert config.auxfile_cutoff == 200 + + config._apply('does_not_exist', 'test_value') + assert_log(caplog, 'ERROR', 'does_not_exist does not exist') + + config._apply('fs_does_not_exist', 'test_value') + assert_log(caplog, 'ERROR', 'fs_does_not_exist does not exist') + + config._apply('max_entry_download', 'not_a_number') + assert_log(caplog, 'ERROR', 'cannot set') + + +def test_env(with_config, monkeypatch): + monkeypatch.setattr('os.environ', dict(NOMAD_FS_PUBLIC='test_value')) + os.environ['NOMAD_FS_PUBLIC'] = 'test_value' + config._apply_env_variables() + assert config.fs.public == 'test_value' + + +def test_nomad_yaml(raw_files, with_config, monkeypatch, caplog): + config_data = { + 'fs': { + 'public': 'test_value', + 'archive_version_suffix': 'test_value', + 'does_not_exist': 'test_value' + }, + 'auxfile_cutoff': '200', + 'does_not_exist': 'test_value', + 'max_entry_download': 'not_a_number' + } + + test_nomad_yaml = os.path.join(config.fs.tmp, 'nomad_test.yaml') + monkeypatch.setattr('os.environ', dict(NOMAD_CONFIG=test_nomad_yaml)) + with open(test_nomad_yaml, 'w') as file: + yaml.dump(config_data, file) + + config.load_config() + os.remove(test_nomad_yaml) -def test_apply(with_config): - config.apply('fs_public', 'test_value') assert config.fs.public == 'test_value' + assert config.fs.archive_version_suffix == 'test_value' + assert config.auxfile_cutoff == 200 + assert_log(caplog, 'ERROR', 'does_not_exist does not exist') + assert_log(caplog, 'ERROR', 'fs_does_not_exist does not exist') + assert_log(caplog, 'ERROR', 'cannot set') diff --git a/tests/utils.py b/tests/utils.py index 6dd3836071406b84f24db23dfaa21cb9491dab96..50de7785d897d27d904138f432d826bac3001c40 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -41,7 +41,13 @@ def assert_log(caplog, level: str, event_part: str) -> LogRecord: record = None for record in caplog.get_records(when='call'): if record.levelname == level: - if (event_part in json.loads(record.msg)['event']): + try: + event_data = json.loads(record.msg) + present = event_part in event_data['event'] + except Exception: + present = event_part in record.msg + + if present: record = record # No need to look for more matches since we aren't counting matches. break