From 13db0d6bc627feb1948ee0125e7fc1acc55d8a01 Mon Sep 17 00:00:00 2001 From: Alexander Zaft Date: Mon, 26 Aug 2024 14:50:58 +0200 Subject: [PATCH] generalConfig, config: use pathlib - switch to pathlib - represent multiple confdirs as list of Paths internally, not string with pathsep Change-Id: I1418e561641e27cd904af0762be056cd66ee1919 Reviewed-on: https://forge.frm2.tum.de/review/c/secop/frappy/+/34464 Reviewed-by: Enrico Faulhaber Tested-by: Jenkins Automated Tests Reviewed-by: Alexander Zaft Reviewed-by: Markus Zolliker --- frappy/config.py | 27 +++++++++++++-------------- frappy/lib/__init__.py | 33 ++++++++++++++++++++------------- frappy/persistent.py | 12 ++++++------ frappy/server.py | 8 ++++---- frappy_psi/sea.py | 26 ++++++++++++++------------ test/test_config.py | 6 +++--- test/test_persistent.py | 5 +++-- 7 files changed, 63 insertions(+), 54 deletions(-) diff --git a/frappy/config.py b/frappy/config.py index fdd875e..0ef7d83 100644 --- a/frappy/config.py +++ b/frappy/config.py @@ -20,6 +20,7 @@ # ***************************************************************************** import os +from pathlib import Path import re from collections import Counter @@ -128,8 +129,7 @@ class Config(dict): def process_file(filename, log): - with open(filename, 'rb') as f: - config_text = f.read() + config_text = filename.read_bytes() node = NodeCollector() mods = Collector(Mod) ns = {'Node': node.add, 'Mod': mods.add, 'Param': Param, 'Command': Param, 'Group': Group} @@ -149,22 +149,21 @@ def process_file(filename, log): def to_config_path(cfgfile, log): candidates = [cfgfile + e for e in ['_cfg.py', '.py', '']] if os.sep in cfgfile: # specified as full path - filename = cfgfile if os.path.exists(cfgfile) else None + file = Path(cfgfile) if Path(cfgfile).exists() else None else: - for filename in [os.path.join(d, candidate) - for d in generalConfig.confdir.split(os.pathsep) - for candidate in candidates]: - if os.path.exists(filename): + for file in [Path(d) / candidate + for d in generalConfig.confdir + for candidate in candidates]: + if file.exists(): break else: - filename = None - - if filename is None: + file = None + if file is None: raise ConfigError(f"Couldn't find cfg file {cfgfile!r} in {generalConfig.confdir}") - if not filename.endswith('_cfg.py'): - log.warning("Config files should end in '_cfg.py': %s", os.path.basename(filename)) - log.debug('Using config file %s for %s', filename, cfgfile) - return filename + if not file.name.endswith('_cfg.py'): + log.warning("Config files should end in '_cfg.py': %s", file.name) + log.debug('Using config file %s for %s', file, cfgfile) + return file def load_config(cfgfiles, log): diff --git a/frappy/lib/__init__.py b/frappy/lib/__init__.py index 4e99530..d93e7c9 100644 --- a/frappy/lib/__init__.py +++ b/frappy/lib/__init__.py @@ -31,6 +31,7 @@ import threading import traceback from configparser import ConfigParser from os import environ, path +from pathlib import Path SECoP_DEFAULT_PORT = 10767 @@ -70,28 +71,32 @@ class GeneralConfig: """ cfg = {} mandatory = 'piddir', 'logdir', 'confdir' - repodir = path.abspath(path.join(path.dirname(__file__), '..', '..')) + repodir = Path(__file__).parents[2].expanduser().resolve() # create default paths - if (path.splitext(sys.executable)[1] == ".exe" - and not path.basename(sys.executable).startswith('python')): + if (Path(sys.executable).suffix == ".exe" + and not Path(sys.executable).name.startswith('python')): # special MS windows environment - self.update_defaults(piddir='./', logdir='./log', confdir='./') + self.update_defaults(piddir=Path('./'), logdir=Path('./log'), confdir=Path('./')) elif path.exists(path.join(repodir, 'cfg')): # running from git repo - self.set_default('confdir', path.join(repodir, 'cfg')) + self.set_default('confdir', repodir / 'cfg') # take logdir and piddir from /cfg/generalConfig.cfg else: # running on installed system (typically with systemd) - self.update_defaults(piddir='/var/run/frappy', logdir='/var/log', confdir='/etc/frappy') + self.update_defaults( + piddir=Path('/var/run/frappy'), + logdir=Path('/var/log'), + confdir=Path('/etc/frappy') + ) if configfile is None: configfile = environ.get('FRAPPY_CONFIG_FILE') if configfile: - configfile = path.expanduser(configfile) - if not path.exists(configfile): + configfile = Path(configfile).expanduser() + if not configfile.exists(): raise FileNotFoundError(configfile) else: - configfile = path.join(self['confdir'], 'generalConfig.cfg') - if not path.exists(configfile): + configfile = self['confdir'] / 'generalConfig.cfg' + if not configfile.exists(): configfile = None if configfile: parser = ConfigParser() @@ -100,16 +105,16 @@ class GeneralConfig: # only the FRAPPY section is relevant, other sections might be used by others for key, value in parser['FRAPPY'].items(): if value.startswith('./'): - cfg[key] = path.abspath(path.join(repodir, value)) + cfg[key] = (repodir / value).absolute() else: # expand ~ to username, also in path lists separated with ':' cfg[key] = ':'.join(path.expanduser(v) for v in value.split(':')) if cfg.get('confdir') is None: - cfg['confdir'] = path.dirname(configfile) + cfg['confdir'] = configfile.parent for key in mandatory: env = environ.get(f'FRAPPY_{key.upper()}') if env is not None: - cfg[key] = env + cfg[key] = Path(env) missing_keys = [ key for key in mandatory if cfg.get(key) is None and self.defaults.get(key) is None @@ -119,6 +124,8 @@ class GeneralConfig: raise KeyError(f"missing value for {' and '.join(missing_keys)} in {configfile}") raise KeyError('missing %s' % ' and '.join('FRAPPY_%s' % k.upper() for k in missing_keys)) + if isinstance(cfg['confdir'], Path): + cfg['confdir'] = [cfg['confdir']] # this is not customizable cfg['basedir'] = repodir self._config = cfg diff --git a/frappy/persistent.py b/frappy/persistent.py index ed62a62..e4ca43d 100644 --- a/frappy/persistent.py +++ b/frappy/persistent.py @@ -75,9 +75,9 @@ class PersistentMixin(Module): def __init__(self, name, logger, cfgdict, srv): super().__init__(name, logger, cfgdict, srv) - persistentdir = os.path.join(generalConfig.logdir, 'persistent') + persistentdir = generalConfig.logdir / 'persistent' os.makedirs(persistentdir, exist_ok=True) - self.persistentFile = os.path.join(persistentdir, f'{self.secNode.equipment_id}.{self.name}.json') + self.persistentFile = persistentdir / f'{self.secNode.equipment_id}.{self.name}.json' self.initData = {} # "factory" settings loaded = self.loadPersistentData() for pname, pobj in self.parameters.items(): @@ -147,10 +147,10 @@ class PersistentMixin(Module): if getattr(v, 'persistent', False)} if data != self.persistentData: self.persistentData = data - persistentdir = os.path.dirname(self.persistentFile) - tmpfile = self.persistentFile + '.tmp' - if not os.path.isdir(persistentdir): - os.makedirs(persistentdir, exist_ok=True) + persistentdir = self.persistentFile.parent + tmpfile = self.persistentFile.parent / (self.persistentFile.name + '.tmp') + if not persistentdir.is_dir(): + persistentdir.mkdir(parents=True, exist_ok=True) try: with open(tmpfile, 'w', encoding='utf-8') as f: json.dump(self.persistentData, f, indent=2) diff --git a/frappy/server.py b/frappy/server.py index 0b1f4fb..d82c72d 100644 --- a/frappy/server.py +++ b/frappy/server.py @@ -112,7 +112,7 @@ class Server: raise ConfigError('No interface specified in configuration or arguments!') self._cfgfiles = cfgfiles - self._pidfile = os.path.join(generalConfig.piddir, name + '.pid') + self._pidfile = generalConfig.piddir / (name + '.pid') signal.signal(signal.SIGINT, self.signal_handler) signal.signal(signal.SIGTERM, self.signal_handler) @@ -127,9 +127,9 @@ class Server: def start(self): if not DaemonContext: raise ConfigError('can not daemonize, as python-daemon is not installed') - piddir = os.path.dirname(self._pidfile) - if not os.path.isdir(piddir): - os.makedirs(piddir) + piddir = self._pidfile.parent + if not piddir.is_dir(): + piddir.mkdir(parents=True) pidfile = pidlockfile.TimeoutPIDLockFile(self._pidfile) if pidfile.is_locked(): diff --git a/frappy_psi/sea.py b/frappy_psi/sea.py index f68f7f4..d6f2dba 100644 --- a/frappy_psi/sea.py +++ b/frappy_psi/sea.py @@ -34,7 +34,7 @@ import json import threading import time import os -from os.path import expanduser, join, exists +from pathlib import Path from frappy.client import ProxyClient from frappy.datatypes import ArrayOf, BoolType, \ @@ -72,19 +72,21 @@ SERVICE_NAMES = { 'addon': 'addons', } -SEA_DIR = expanduser('~/sea') +SEA_DIR = Path('~/sea').expanduser() seaconfdir = os.environ.get('FRAPPY_SEA_DIR') -if seaconfdir is None or not exists(seaconfdir): - for confdir in generalConfig.confdir.split(os.pathsep): - seaconfdir = join(confdir, 'sea') - if exists(seaconfdir): +if seaconfdir is None or not Path(seaconfdir).expanduser().absolute().exists(): + for confdir in generalConfig.confdir: + seaconfdir = confdir / 'sea' + if seaconfdir.exists(): break +else: + seaconfdir = Path(seaconfdir).expanduser().absolute() def get_sea_port(instance): for filename in ('sea_%s.tcl' % instance, 'sea.tcl'): try: - with open(join(SEA_DIR, filename), encoding='utf-8') as f: + with open(SEA_DIR / filename, encoding='utf-8') as f: for line in f: linesplit = line.split() if len(linesplit) == 3: @@ -375,16 +377,16 @@ class SeaConfigCreator(SeaClient): stripped, _, ext = filename.rpartition('.') service = SERVICE_NAMES[ext] seaconn = 'sea_' + service - cfgfile = join(seaconfdir, stripped + '_cfg.py') - with open(cfgfile, 'w', encoding='utf-8') as fp: + cfgfile = seaconfdir / (stripped + '_cfg.py') + with cfgfile.open('w', encoding='utf-8') as fp: fp.write(CFG_HEADER % {'config': filename, 'seaconn': seaconn, 'service': service, 'nodedescr': description.get(filename, filename)}) for obj in descr: fp.write(CFG_MODULE % {'modcls': modcls[obj], 'module': obj, 'seaconn': seaconn}) content = json.dumps(descr).replace('}, {', '},\n{').replace('[{', '[\n{').replace('}]}, ', '}]},\n\n') result.append('%s\n' % cfgfile) - with open(join(seaconfdir, filename + '.json'), 'w', encoding='utf-8') as fp: - fp.write(content + '\n') + fpath = seaconfdir / (filename + '.json') + fpath.write_text(content + '\n', encoding='utf-8') result.append('%s: %s' % (filename, ','.join(n for n in descr))) raise SystemExit('; '.join(result)) @@ -482,7 +484,7 @@ class SeaModule(Module): cfgdict['description'] = '%s@%s%s' % ( name, json_file, '' if rel_paths == '.' else f' (rel_paths={rel_paths})') - with open(join(seaconfdir, json_file), encoding='utf-8') as fp: + with open(seaconfdir / json_file, encoding='utf-8') as fp: content = json.load(fp) descr = content[sea_object] if rel_paths == '*' or not rel_paths: diff --git a/test/test_config.py b/test/test_config.py index 5095a76..5c1c3ec 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -64,7 +64,7 @@ def direc(tmp_path_factory): ff.touch() fff.touch() pyfile.write_text(PY_FILE) - generalConfig.testinit(confdir=f'{a}:{b}', piddir=str(d)) + generalConfig.testinit(confdir=[a, b], piddir=d) return d @@ -81,7 +81,7 @@ files = [('config', 'a/config_cfg.py'), @pytest.mark.parametrize('file, res', files) def test_to_cfg_path(log, direc, file, res): - assert to_config_path(file, log).endswith(res) + assert str(to_config_path(file, log)).endswith(res) def test_cfg_not_existing(direc, log): @@ -132,7 +132,7 @@ def do_asserts(ret): def test_process_file(direc, log): - ret = process_file(str(direc / 'a' / 'pyfile_cfg.py'), log) + ret = process_file(direc / 'a' / 'pyfile_cfg.py', log) do_asserts(ret) diff --git a/test/test_persistent.py b/test/test_persistent.py index 1dd74fc..7520a04 100644 --- a/test/test_persistent.py +++ b/test/test_persistent.py @@ -22,6 +22,7 @@ import json import os from os.path import join +from pathlib import Path import pytest from frappy.config import Param from frappy.core import Module, ScaledInteger, IntRange, StringType, StructOf @@ -74,7 +75,7 @@ save_tests = [ ] @pytest.mark.parametrize('cfg, data', save_tests) def test_save(tmpdir, cfg, data): - generalConfig.logdir = tmpdir + generalConfig.logdir = Path(tmpdir) cfg['description'] = '' m = Mod('m', logger, cfg, ServerStub('savetest')) @@ -101,7 +102,7 @@ load_tests = [ ] @pytest.mark.parametrize('cfg, data, written', load_tests) def test_load(tmpdir, cfg, data, written): - generalConfig.logdir = tmpdir + generalConfig.logdir = Path(tmpdir) os.makedirs(join(tmpdir, 'persistent'), exist_ok=True) with open(join(tmpdir, 'persistent', 'loadtest.m.json'), 'w', encoding='utf-8') as f: