Changeset View
Standalone View
swh/core/config.py
Show First 20 Lines • Show All 280 Lines • ▼ Show 20 Lines | if global_conf: | ||||
conf.update(load_global_config()) | conf.update(load_global_config()) | ||||
conf.update(priority_read(swh_config_paths(name), default_conf)) | conf.update(priority_read(swh_config_paths(name), default_conf)) | ||||
return conf | return conf | ||||
class SWHConfig: | class SWHConfig: | ||||
"""Mixin to add configuration parsing abilities to classes | """Mixin to add configuration parsing abilities to classes. | ||||
The class should override the class attributes: | The class should override the class attribute DEFAULT_CONFIG to provide default | ||||
- DEFAULT_CONFIG (default configuration to be parsed) | key/value entries which need to be set if not defined in the configuration file. | ||||
- CONFIG_BASE_FILENAME (the filename of the configuration to be used) | |||||
This class defines one classmethod, func:`parse_config_file`, which parses a | |||||
This class defines one classmethod, parse_config_file, which | configuration file, provided through the SWH_CONFIG_FILENAME environment variable. | ||||
parses a configuration file using the default config as set in the | Eventually enriched with default configuration as per the class attribute | ||||
class attribute. | DEFAULT_CONFIG. | ||||
""" | """ | ||||
tenma: `assert "SWH_CONFIG_FILENAME" in os.environ, "SWH_CONFIG_FILENAME environment variable is… | |||||
DEFAULT_CONFIG = {} # type: Dict[str, Tuple[str, Any]] | DEFAULT_CONFIG: Dict[str, Any] = {} | ||||
Done Inline ActionsWARNING: mutable value in class definition. As with function definitions, this will only be eval'd once.
I think this class is meant to be instanciated, we don't want to share config between instances. In that case, use factory like attrs ones or write a constructor.
For this, we better think of a strategy that does not involves user discipline but works automatically. tenma: WARNING: mutable value in class definition. As with function definitions, this will only be… | |||||
Done Inline Actionsalternatively, we can just make it None vlorentz: alternatively, we can just make it None | |||||
Done Inline ActionsNo, often a bad idea to have nullable types where it is not necessary, for reliability reasons. In this mixin, only a declaration, which conveniently serves as a spec btw, and in user class only an initialization is needed. In fact it could be seen as a lazy constant, which is very safe (reliable) construct. tenma: No, often a bad idea to have nullable types where it is not necessary, for reliability reasons. | |||||
Done Inline Actions
I'm opened to an example of what's implied by that suggestion. Please, for example, wrap the snippet in tiple backquote <sample-code> so it's nicely rendered here ;) ardumont: > WARNING
> ...
I'm opened to an example of what's implied by that suggestion.
Please, for… | |||||
Done Inline Actionsto be complete, here is @tenma's suggestion: @attr.s class Config: conf = attr.ib(factory=dict) ardumont: to be complete, here is @tenma's suggestion:
```
@attr.s
class Config:
conf = attr.ib… | |||||
CONFIG_BASE_FILENAME = "" # type: Optional[str] | |||||
@classmethod | @classmethod | ||||
def parse_config_file( | def parse_config_file(cls): | ||||
cls, | |||||
base_filename=None, | |||||
config_filename=None, | |||||
additional_configs=None, | |||||
global_config=True, | |||||
): | |||||
"""Parse the configuration file associated to the current class. | """Parse the configuration file associated to the current class. | ||||
By default, parse_config_file will load the configuration | By default, func:`parse_config_file` will load the configuration from the file | ||||
cls.CONFIG_BASE_FILENAME from one of the Software Heritage | referenced by the environment variable SWH_CONFIG_FILENAME. If not set, this | ||||
configuration directories, in order, unless it is overridden | will raise a ValueError requesting the environment variable to be set. | ||||
by base_filename or config_filename (which shortcuts the file | |||||
lookup completely). | |||||
Args: | |||||
- base_filename (str): overrides the default | |||||
cls.CONFIG_BASE_FILENAME | |||||
- config_filename (str): sets the file to parse instead of | |||||
the defaults set from cls.CONFIG_BASE_FILENAME | |||||
- additional_configs: (list of default configuration dicts) | |||||
allows to override or extend the configuration set in | |||||
cls.DEFAULT_CONFIG. | |||||
- global_config (bool): Load the global configuration (default: | |||||
True) | |||||
""" | |||||
if config_filename: | |||||
config_filenames = [config_filename] | |||||
elif "SWH_CONFIG_FILENAME" in os.environ: | |||||
config_filenames = [os.environ["SWH_CONFIG_FILENAME"]] | |||||
else: | |||||
if not base_filename: | |||||
base_filename = cls.CONFIG_BASE_FILENAME | |||||
config_filenames = swh_config_paths(base_filename) | |||||
if not additional_configs: | |||||
additional_configs = [] | |||||
full_default_config = merge_default_configs( | Raises:" | ||||
cls.DEFAULT_CONFIG, *additional_configs | ValueError if SWH_CONFIG_FILENAME is undefined | ||||
) | |||||
config = {} | """ | ||||
if global_config: | |||||
config = load_global_config() | |||||
config.update(priority_read(config_filenames, full_default_config)) | if "SWH_CONFIG_FILENAME" not in os.environ: | ||||
raise ValueError("SWH_CONFIG_FILENAME environment variable is undefined.") | |||||
Done Inline Actionss/should be defined/is undefined vlorentz: s/should be defined/is undefined | |||||
return config | cfg_path = os.environ["SWH_CONFIG_FILENAME"] | ||||
cfg = read_raw_config(config_basepath(cfg_path)) | |||||
Done Inline ActionsA default config, albeit empty, is defined (as dict), and merge_config has no input domain restriction. So this conditional is not needed. tenma: A default config, albeit empty, is defined (as dict), and `merge_config` has no input domain… | |||||
Done Inline ActionsI repeat, this branch will always match, so no need to keep it. tenma: I repeat, this branch will always match, so no need to keep it. | |||||
Done Inline Actionsyes, I did not adapt it yet since i'm unclear as to what i need to do for the DEFAULT_CONFIG field remark first... ardumont: yes, I did not adapt it yet since i'm unclear as to what i need to do for the DEFAULT_CONFIG… | |||||
if cls.DEFAULT_CONFIG: | |||||
cfg = merge_configs(cls.DEFAULT_CONFIG, cfg) | |||||
return cfg |
assert "SWH_CONFIG_FILENAME" in os.environ, "SWH_CONFIG_FILENAME environment variable is undefined." est encore plus clair, ça dit "vous avez pas suivi le contrat"