From f984a918b959a46e49fe8456a327eca8d44313de Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Tue, 30 Mar 2021 07:56:38 +0200 Subject: [PATCH 1/2] Fix log message string formatting Use logging message format with args, instead of direct `%` or `str.format`. Resolve #855. --- cdist/argparse.py | 8 +-- cdist/config.py | 87 ++++++++++++-------------- cdist/core/cdist_type.py | 6 +- cdist/core/explorer.py | 26 ++++---- cdist/core/manifest.py | 2 +- cdist/emulator.py | 37 +++++------ cdist/exec/local.py | 16 +++-- cdist/exec/remote.py | 14 ++--- cdist/install.py | 2 +- cdist/inventory.py | 36 +++++------ cdist/preos.py | 6 +- cdist/preos/debootstrap/debootstrap.py | 41 ++++++------ cdist/scan/scan.py | 4 +- cdist/util/ipaddr.py | 6 +- 14 files changed, 140 insertions(+), 151 deletions(-) diff --git a/cdist/argparse.py b/cdist/argparse.py index 88759d7b..cadac39a 100644 --- a/cdist/argparse.py +++ b/cdist/argparse.py @@ -533,10 +533,10 @@ def parse_and_configure(argv, singleton=True): log = logging.getLogger("cdist") - log.verbose("version %s" % cdist.VERSION) - log.trace('command line args: {}'.format(cfg.command_line_args)) - log.trace('configuration: {}'.format(cfg.get_config())) - log.trace('configured args: {}'.format(args)) + log.verbose("version %s", cdist.VERSION) + log.trace('command line args: %s', cfg.command_line_args) + log.trace('configuration: %s', cfg.get_config()) + log.trace('configured args: %s', args) check_beta(vars(args)) diff --git a/cdist/config.py b/cdist/config.py index 19d5bd70..d6fec55f 100644 --- a/cdist/config.py +++ b/cdist/config.py @@ -273,15 +273,15 @@ class Config: host_tags = None host_base_path, hostdir = cls.create_host_base_dirs( host, base_root_path) - log.debug("Base root path for target host \"{}\" is \"{}\"".format( - host, host_base_path)) + log.debug("Base root path for target host \"%s\" is \"%s\"", + host, host_base_path) hostcnt += 1 if args.parallel: pargs = (host, host_tags, host_base_path, hostdir, args, True, configuration) - log.trace(("Args for multiprocessing operation " - "for host {}: {}".format(host, pargs))) + log.trace("Args for multiprocessing operation for host %s: %s", + host, pargs) process_args.append(pargs) else: try: @@ -298,10 +298,10 @@ class Config: except cdist.Error: failed_hosts.append(host) elif args.parallel: - log.trace("Multiprocessing start method is {}".format( - multiprocessing.get_start_method())) - log.trace(("Starting multiprocessing Pool for {} " - "parallel host operation".format(args.parallel))) + log.trace("Multiprocessing start method is %s", + multiprocessing.get_start_method()) + log.trace("Starting multiprocessing Pool for %d parallel host" + " operation", args.parallel) results = mp_pool_run(cls.onehost, process_args, @@ -396,16 +396,13 @@ class Config: remote_exec, remote_copy, cleanup_cmd = cls._resolve_remote_cmds( args) - log.debug("remote_exec for host \"{}\": {}".format( - host, remote_exec)) - log.debug("remote_copy for host \"{}\": {}".format( - host, remote_copy)) + log.debug("remote_exec for host \"%s\": %s", host, remote_exec) + log.debug("remote_copy for host \"%s\": %s", host, remote_copy) family = cls._address_family(args) - log.debug("address family: {}".format(family)) + log.debug("address family: %s", family) target_host = cls.resolve_target_addresses(host, family) - log.debug("target_host for host \"{}\": {}".format( - host, target_host)) + log.debug("target_host for host \"%s\": %s", host, target_host) local = cdist.exec.local.Local( target_host=target_host, @@ -474,8 +471,8 @@ class Config: """Do what is most often done: deploy & cleanup""" start_time = time.time() - self.log.info("Starting {} run".format( - 'dry' if self.dry_run else 'configuration')) + self.log.info("Starting %s run", + 'dry' if self.dry_run else 'configuration') self._init_files_dirs() @@ -493,9 +490,9 @@ class Config: self._remove_files_dirs() self.local.save_cache(start_time) - self.log.info("Finished {} run in {:.2f} seconds".format( + self.log.info("Finished %s run in %.2f seconds", 'dry' if self.dry_run else 'successful', - time.time() - start_time)) + time.time() - start_time) def cleanup(self): self.log.debug("Running cleanup commands") @@ -519,8 +516,8 @@ class Config: self.local.object_path, self.local.type_path, self.local.object_marker_name): if cdist_object.cdist_type.is_install: - self.log.debug(("Running in config mode, ignoring install " - "object: {0}").format(cdist_object)) + self.log.debug("Running in config mode, ignoring install " + "object: %s", cdist_object) else: yield cdist_object @@ -565,8 +562,7 @@ class Config: return objects_changed def _iterate_once_parallel(self): - self.log.debug("Iteration in parallel mode in {} jobs".format( - self.jobs)) + self.log.debug("Iteration in parallel mode in %d jobs", self.jobs) objects_changed = False cargo = [] @@ -588,8 +584,8 @@ class Config: self.object_prepare(cargo[0]) objects_changed = True elif cargo: - self.log.trace("Multiprocessing start method is {}".format( - multiprocessing.get_start_method())) + self.log.trace("Multiprocessing start method is %s", + multiprocessing.get_start_method()) self.log.trace("Multiprocessing cargo: %s", cargo) @@ -603,9 +599,8 @@ class Config: "sequentially")) self.explorer.transfer_type_explorers(cargo_types.pop()) else: - self.log.trace(("Starting multiprocessing Pool for {} " - "parallel types explorers transferring".format( - nt))) + self.log.trace("Starting multiprocessing Pool for %d " + "parallel types explorers transferring", nt) args = [ (ct, ) for ct in cargo_types ] @@ -614,8 +609,8 @@ class Config: self.log.trace(("Multiprocessing for parallel transferring " "types' explorers finished")) - self.log.trace(("Starting multiprocessing Pool for {} parallel " - "objects preparation".format(n))) + self.log.trace("Starting multiprocessing Pool for %d parallel " + "objects preparation", n) args = [ (c, False, ) for c in cargo ] @@ -667,10 +662,10 @@ class Config: self.object_run(chunk[0]) objects_changed = True elif chunk: - self.log.trace("Multiprocessing start method is {}".format( - multiprocessing.get_start_method())) - self.log.trace(("Starting multiprocessing Pool for {} " - "parallel object run".format(n))) + self.log.trace("Multiprocessing start method is %s", + multiprocessing.get_start_method()) + self.log.trace("Starting multiprocessing Pool for %d " + "parallel object run", n) args = [ (c, ) for c in chunk ] @@ -794,9 +789,9 @@ class Config: def object_prepare(self, cdist_object, transfer_type_explorers=True): """Prepare object: Run type explorer + manifest""" self._handle_deprecation(cdist_object) - self.log.verbose("Preparing object {}".format(cdist_object.name)) - self.log.verbose( - "Running manifest and explorers for " + cdist_object.name) + self.log.verbose("Preparing object %s", cdist_object.name) + self.log.verbose("Running manifest and explorers for %s", + cdist_object.name) self.explorer.run_type_explorers(cdist_object, transfer_type_explorers) try: self.manifest.run_type_manifest(cdist_object) @@ -810,13 +805,13 @@ class Config: def object_run(self, cdist_object): """Run gencode and code for an object""" try: - self.log.verbose("Running object " + cdist_object.name) + self.log.verbose("Running object %s", cdist_object.name) if cdist_object.state == core.CdistObject.STATE_DONE: raise cdist.Error(("Attempting to run an already finished " - "object: %s"), cdist_object) + "object: {}").format(cdist_object)) # Generate - self.log.debug("Generating code for %s" % (cdist_object.name)) + self.log.debug("Generating code for %s", cdist_object.name) cdist_object.code_local = self.code.run_gencode_local(cdist_object) cdist_object.code_remote = self.code.run_gencode_remote( cdist_object) @@ -825,20 +820,20 @@ class Config: # Execute if cdist_object.code_local or cdist_object.code_remote: - self.log.info("Processing %s" % (cdist_object.name)) + self.log.info("Processing %s", cdist_object.name) if not self.dry_run: if cdist_object.code_local: - self.log.trace("Executing local code for %s" - % (cdist_object.name)) + self.log.trace("Executing local code for %s", + cdist_object.name) self.code.run_code_local(cdist_object) if cdist_object.code_remote: - self.log.trace("Executing remote code for %s" - % (cdist_object.name)) + self.log.trace("Executing remote code for %s", + cdist_object.name) self.code.transfer_code_remote(cdist_object) self.code.run_code_remote(cdist_object) # Mark this object as done - self.log.trace("Finishing run of " + cdist_object.name) + self.log.trace("Finishing run of %s", cdist_object.name) cdist_object.state = core.CdistObject.STATE_DONE except cdist.Error as e: raise cdist.CdistObjectError(cdist_object, e) diff --git a/cdist/core/cdist_type.py b/cdist/core/cdist_type.py index c0329c8a..274de989 100644 --- a/cdist/core/cdist_type.py +++ b/cdist/core/cdist_type.py @@ -82,9 +82,9 @@ class CdistType: yield cls(base_path, name) except InvalidTypeError as e: # ignore invalid type, log warning and continue - msg = "Ignoring invalid type '%s' at '%s' defined at '%s'" % ( - e.type_path, e.type_absolute_path, e.source_path) - cls.log.warning(msg) + cls.log.warning("Ignoring invalid type '%s' at '%s' defined" + " at '%s'", e.type_path, e.type_absolute_path, + e.source_path) # remove invalid from runtime conf dir os.remove(e.type_absolute_path) diff --git a/cdist/core/explorer.py b/cdist/core/explorer.py index a3baa959..caa12a7d 100644 --- a/cdist/core/explorer.py +++ b/cdist/core/explorer.py @@ -131,18 +131,17 @@ class Explorer: self._run_global_explorer(explorer, out_path) def _run_global_explorers_parallel(self, out_path): - self.log.debug("Running global explorers in {} parallel jobs".format( - self.jobs)) - self.log.trace("Multiprocessing start method is {}".format( - multiprocessing.get_start_method())) - self.log.trace(("Starting multiprocessing Pool for global " - "explorers run")) + self.log.debug("Running global explorers in %s parallel jobs", + self.jobs) + self.log.trace("Multiprocessing start method is %s", + multiprocessing.get_start_method()) + self.log.trace("Starting multiprocessing Pool for global explorers" + " run") args = [ (e, out_path, ) for e in self.list_global_explorer_names() ] mp_pool_run(self._run_global_explorer, args, jobs=self.jobs) - self.log.trace(("Multiprocessing run for global explorers " - "finished")) + self.log.trace("Multiprocessing run for global explorers finished") # logger is not pickable, so remove it when we pickle def __getstate__(self): @@ -184,15 +183,14 @@ class Explorer: in the object. """ - self.log.verbose("Running type explorers for {}".format( - cdist_object.cdist_type)) + self.log.verbose("Running type explorers for %s", + cdist_object.cdist_type) if transfer_type_explorers: self.log.trace("Transferring type explorers for type: %s", cdist_object.cdist_type) self.transfer_type_explorers(cdist_object.cdist_type) else: - self.log.trace(("No need for transferring type explorers for " - "type: %s"), + self.log.trace("No need for transferring type explorers for %s", cdist_object.cdist_type) self.log.trace("Transferring object parameters for object: %s", cdist_object.name) @@ -236,8 +234,8 @@ class Explorer: remote side.""" if cdist_type.explorers: if cdist_type.name in self._type_explorers_transferred: - self.log.trace(("Skipping retransfer of type explorers " - "for: %s"), cdist_type) + self.log.trace("Skipping retransfer of type explorers for: %s", + cdist_type) else: source = os.path.join(self.local.type_path, cdist_type.explorer_path) diff --git a/cdist/core/manifest.py b/cdist/core/manifest.py index 390340d4..3148d66c 100644 --- a/cdist/core/manifest.py +++ b/cdist/core/manifest.py @@ -161,7 +161,7 @@ class Manifest: raise NoInitialManifestError(initial_manifest, user_supplied) message_prefix = "initialmanifest" - self.log.verbose("Running initial manifest " + initial_manifest) + self.log.verbose("Running initial manifest %s", initial_manifest) which = "init" if self.local.save_output_streams: stderr_path = os.path.join(self.local.stderr_base_path, which) diff --git a/cdist/emulator.py b/cdist/emulator.py index f1db862e..f09c282d 100644 --- a/cdist/emulator.py +++ b/cdist/emulator.py @@ -107,8 +107,8 @@ class Emulator: self.record_requirements() self.record_auto_requirements() self.record_parent_child_relationships() - self.log.trace("Finished %s %s" % ( - self.cdist_object.path, self.parameters)) + self.log.trace("Finished %s %s", self.cdist_object.path, + self.parameters) def __init_log(self): """Setup logging facility""" @@ -170,7 +170,7 @@ class Emulator: # And finally parse/verify parameter self.args = parser.parse_args(self.argv[1:]) - self.log.trace('Args: %s' % self.args) + self.log.trace('Args: %s', self.args) def init_object(self): # Initialize object - and ensure it is not in args @@ -241,8 +241,8 @@ class Emulator: raise cdist.Error(errmsg) else: if self.cdist_object.exists: - self.log.debug(('Object %s override forced with ' - 'CDIST_OVERRIDE'), self.cdist_object.name) + self.log.debug('Object %s override forced with CDIST_OVERRIDE', + self.cdist_object.name) self.cdist_object.create(True) else: self.cdist_object.create() @@ -260,8 +260,8 @@ class Emulator: parent = self.cdist_object.object_from_name(__object_name) parent.typeorder.append(self.cdist_object.name) if self._order_dep_on(): - self.log.trace(('[ORDER_DEP] Adding %s to typeorder dep' - ' for %s'), depname, parent.name) + self.log.trace('[ORDER_DEP] Adding %s to typeorder dep for %s', + depname, parent.name) parent.typeorder_dep.append(depname) elif self._order_dep_on(): self.log.trace('[ORDER_DEP] Adding %s to global typeorder dep', @@ -301,16 +301,14 @@ class Emulator: try: cdist_object = self.cdist_object.object_from_name(requirement) except core.cdist_type.InvalidTypeError as e: - self.log.error(("%s requires object %s, but type %s does not" - " exist. Defined at %s" % ( - self.cdist_object.name, - requirement, e.name, self.object_source))) + self.log.error("%s requires object %s, but type %s does not" + " exist. Defined at %s", self.cdist_object.name, + requirement, e.name, self.object_source) raise except core.cdist_object.MissingObjectIdError: - self.log.error(("%s requires object %s without object id." - " Defined at %s" % (self.cdist_object.name, - requirement, - self.object_source))) + self.log.error("%s requires object %s without object id." + " Defined at %s", self.cdist_object.name, + requirement, self.object_source) raise self.log.debug("Recording requirement %s for %s", @@ -380,10 +378,9 @@ class Emulator: self.env['require'] += " " + lastcreatedtype else: self.env['require'] = lastcreatedtype - self.log.debug(("Injecting require for " - "CDIST_ORDER_DEPENDENCY: %s for %s"), - lastcreatedtype, - self.cdist_object.name) + self.log.debug("Injecting require for" + " CDIST_ORDER_DEPENDENCY: %s for %s", + lastcreatedtype, self.cdist_object.name) except IndexError: # if no second last line, we are on the first type, # so do not set a requirement @@ -391,7 +388,7 @@ class Emulator: if "require" in self.env: requirements = self.env['require'] - self.log.debug("reqs = " + requirements) + self.log.debug("reqs = %s", requirements) for requirement in self._parse_require(requirements): # Ignore empty fields - probably the only field anyway if len(requirement) == 0: diff --git a/cdist/exec/local.py b/cdist/exec/local.py index e0aab190..6713cd13 100644 --- a/cdist/exec/local.py +++ b/cdist/exec/local.py @@ -154,8 +154,8 @@ class Local: with open(self.object_marker_file, 'w') as fd: fd.write("%s\n" % self.object_marker_name) - self.log.trace("Object marker %s saved in %s" % ( - self.object_marker_name, self.object_marker_file)) + self.log.trace("Object marker %s saved in %s", + self.object_marker_name, self.object_marker_file) def _init_cache_dir(self, cache_dir): home_dir = cdist.home_dir() @@ -289,14 +289,12 @@ class Local: return cache_subpath def save_cache(self, start_time=time.time()): - self.log.trace("cache subpath pattern: {}".format( - self.cache_path_pattern)) + self.log.trace("cache subpath pattern: %s", self.cache_path_pattern) cache_subpath = self._cache_subpath(start_time, self.cache_path_pattern) - self.log.debug("cache subpath: {}".format(cache_subpath)) + self.log.debug("cache subpath: %s", cache_subpath) destination = os.path.join(self.cache_path, cache_subpath) - self.log.trace(("Saving cache: " + self.base_path + " to " + - destination)) + self.log.trace("Saving cache %s to %s", self.base_path, destination) if not os.path.exists(destination): shutil.move(self.base_path, destination) @@ -332,7 +330,7 @@ class Local: # Iterate over all directories and link the to the output dir for conf_dir in self.conf_dirs: - self.log.debug("Checking conf_dir %s ..." % (conf_dir)) + self.log.debug("Checking conf_dir %s ...", conf_dir) for sub_dir in CONF_SUBDIRS_LINKED: current_dir = os.path.join(conf_dir, sub_dir) @@ -350,7 +348,7 @@ class Local: if os.path.exists(dst): os.unlink(dst) - self.log.trace("Linking %s to %s ..." % (src, dst)) + self.log.trace("Linking %s to %s ...", src, dst) try: os.symlink(src, dst) except OSError as e: diff --git a/cdist/exec/remote.py b/cdist/exec/remote.py index e5af2f34..ea85da4c 100644 --- a/cdist/exec/remote.py +++ b/cdist/exec/remote.py @@ -176,19 +176,19 @@ class Remote: # create archive tarpath, fcnt = autil.tar(source, self.archiving_mode) if tarpath is None: - self.log.trace(("Files count {} is lower than {} limit, " - "skipping archiving").format( - fcnt, autil.FILES_LIMIT)) + self.log.trace("Files count %d is lower than %d limit, " + "skipping archiving", + fcnt, autil.FILES_LIMIT) else: - self.log.trace(("Archiving mode, tarpath: %s, file count: " - "%s"), tarpath, fcnt) + self.log.trace("Archiving mode, tarpath: %s, file count: " + "%s", tarpath, fcnt) # get archive name tarname = os.path.basename(tarpath) self.log.trace("Archiving mode tarname: %s", tarname) # archive path at the remote desttarpath = os.path.join(destination, tarname) - self.log.trace( - "Archiving mode desttarpath: %s", desttarpath) + self.log.trace("Archiving mode desttarpath: %s", + desttarpath) # transfer archive to the remote side self.log.trace("Archiving mode: transferring") self._transfer_file(tarpath, desttarpath) diff --git a/cdist/install.py b/cdist/install.py index 7c894fe5..d077baef 100644 --- a/cdist/install.py +++ b/cdist/install.py @@ -47,4 +47,4 @@ class Install(cdist.config.Config): yield cdist_object else: self.log.debug("Running in install mode, ignoring non install" - "object: {0}".format(cdist_object)) + "object: %s", cdist_object) diff --git a/cdist/inventory.py b/cdist/inventory.py index 0387f326..106052a2 100644 --- a/cdist/inventory.py +++ b/cdist/inventory.py @@ -92,7 +92,7 @@ class Inventory: self.init_db() def init_db(self): - self.log.trace("Init db: {}".format(self.db_basedir)) + self.log.trace("Init db: %s", self.db_basedir) if not os.path.exists(self.db_basedir): os.makedirs(self.db_basedir, exist_ok=True) elif not os.path.isdir(self.db_basedir): @@ -182,9 +182,9 @@ class Inventory: configuration = cfg.get_config(section='GLOBAL') determine_default_inventory_dir(args, configuration) - log.debug("Using inventory: {}".format(args.inventory_dir)) - log.trace("Inventory args: {}".format(vars(args))) - log.trace("Inventory command: {}".format(args.subcommand)) + log.debug("Using inventory: %s", args.inventory_dir) + log.trace("Inventory args: %s", vars(args)) + log.trace("Inventory command: %s", args.subcommand) if args.subcommand == "list": c = InventoryList(hosts=args.host, istag=args.tag, @@ -237,16 +237,16 @@ class InventoryList(Inventory): def _do_list(self, it_tags, it_hosts, check_func): if (it_tags is not None): param_tags = set(it_tags) - self.log.trace("param_tags: {}".format(param_tags)) + self.log.trace("param_tags: %s", param_tags) else: param_tags = set() for host in it_hosts: - self.log.trace("host: {}".format(host)) + self.log.trace("host: %s", host) tags = self._get_host_tags(host) if tags is None: - self.log.debug("Host \'{}\' not found, skipped".format(host)) + self.log.debug("Host \'%s\' not found, skipped", host) continue - self.log.trace("tags: {}".format(tags)) + self.log.trace("tags: %s", tags) if check_func(tags, param_tags): yield host, tags @@ -308,11 +308,11 @@ class InventoryHost(Inventory): def _action(self, host): if self.action == "add": - self.log.debug("Adding host \'{}\'".format(host)) + self.log.debug("Adding host \'%s\'", host) elif self.action == "del": - self.log.debug("Deleting host \'{}\'".format(host)) + self.log.debug("Deleting host \'%s\'", host) hostpath = self._host_path(host) - self.log.trace("hostpath: {}".format(hostpath)) + self.log.trace("hostpath: %s", hostpath) if self.action == "add" and not os.path.exists(hostpath): self._new_hostpath(hostpath) else: @@ -372,23 +372,23 @@ class InventoryTag(Inventory): print("Host \'{}\' does not exist, skipping".format(host), file=sys.stderr) return - self.log.trace("existing host_tags: {}".format(host_tags)) + self.log.trace("existing host_tags: %s", host_tags) if self.action == "del" and self.all: host_tags = set() else: for tag in self.input_tags: if self.action == "add": - self.log.debug("Adding tag \'{}\' for host \'{}\'".format( - tag, host)) + self.log.debug("Adding tag \'%s\' for host \'%s\'", + tag, host) host_tags.add(tag) elif self.action == "del": - self.log.debug("Deleting tag \'{}\' for host " - "\'{}\'".format(tag, host)) + self.log.debug("Deleting tag \'%s\' for host \'%s\'", + tag, host) if tag in host_tags: host_tags.remove(tag) - self.log.trace("new host tags: {}".format(host_tags)) + self.log.trace("new host tags: %s", host_tags) if not self._write_host_tags(host, host_tags): - self.log.trace("{} does not exist, skipped".format(host)) + self.log.trace("%s does not exist, skipped", host) def run(self): if self.allhosts: diff --git a/cdist/preos.py b/cdist/preos.py index f8a5dd67..45711a41 100644 --- a/cdist/preos.py +++ b/cdist/preos.py @@ -49,7 +49,7 @@ def scan_preos_dir_plugins(dir): c = cm[1] yield from preos_plugin(c) except ImportError as e: - log.warning("Cannot import '{}': {}".format(module_name, e)) + log.warning("Cannot import '%s': %s", module_name, e) def find_preos_plugins(): @@ -102,7 +102,7 @@ class PreOS: parser.add_argument('remainder_args', nargs=argparse.REMAINDER) args = parser.parse_args(argv[1:]) cdist.argparse.handle_loglevel(args) - log.debug("preos args : {}".format(args)) + log.debug("preos args : %s", args) conf_dirs = util.resolve_conf_dirs_from_config_and_args(args) @@ -122,7 +122,7 @@ class PreOS: func_args = [preos, args.remainder_args, ] else: func_args = [args.remainder_args, ] - log.info("Running preos : {}".format(preos_name)) + log.info("Running preos : %s", preos_name) func(*func_args) else: raise cdist.Error( diff --git a/cdist/preos/debootstrap/debootstrap.py b/cdist/preos/debootstrap/debootstrap.py index a20cdb9c..f1f750ee 100644 --- a/cdist/preos/debootstrap/debootstrap.py +++ b/cdist/preos/debootstrap/debootstrap.py @@ -166,7 +166,7 @@ class Debian: args.pxe_boot_dir = os.path.realpath(args.pxe_boot_dir) cdist.argparse.handle_loglevel(args) - log.debug("preos: {}, args: {}".format(cls._preos_name, args)) + log.debug("preos: %s, args: %s", cls._preos_name, args) try: env = vars(args) new_env = {} @@ -190,27 +190,30 @@ class Debian: env = new_env env.update(os.environ) cls.update_env(env) - log.debug("preos: {} env: {}".format(cls._preos_name, env)) + log.debug("preos: %s env: %s", cls._preos_name, env) + + if log.getEffectiveLevel() <= logging.INFO: + info_msg = ["Running preos: {}, suite: {}, arch: {}".format( + cls._preos_name, args.suite, args.arch), ] + if args.mirror: + info_msg.append("mirror: {}".format(args.mirror)) + if args.script: + info_msg.append("script: {}".format(args.script)) + if args.bootstrap: + info_msg.append("bootstrapping") + if args.configure: + info_msg.append("configuring") + if args.pxe_boot_dir: + info_msg.append("creating PXE") + if args.drive: + info_msg.append("creating bootable drive") + log.info(info_msg) + cmd = os.path.join(cls._files_dir, "code") - info_msg = ["Running preos: {}, suite: {}, arch: {}".format( - cls._preos_name, args.suite, args.arch), ] - if args.mirror: - info_msg.append("mirror: {}".format(args.mirror)) - if args.script: - info_msg.append("script: {}".format(args.script)) - if args.bootstrap: - info_msg.append("bootstrapping") - if args.configure: - info_msg.append("configuring") - if args.pxe_boot_dir: - info_msg.append("creating PXE") - if args.drive: - info_msg.append("creating bootable drive") - log.info(info_msg) - log.debug("cmd={}".format(cmd)) + log.debug("cmd=%s", cmd) subprocess.check_call(cmd, env=env, shell=True) except subprocess.CalledProcessError as e: - log.error("preos {} failed: {}".format(cls._preos_name, e)) + log.error("preos %s failed: %s", cls._preos_name, e) class Ubuntu(Debian): diff --git a/cdist/scan/scan.py b/cdist/scan/scan.py index b1d0e9e1..9a0bcc52 100644 --- a/cdist/scan/scan.py +++ b/cdist/scan/scan.py @@ -94,7 +94,7 @@ class Trigger(object): def trigger(self, interface): packet = IPv6(dst=f"ff02::1%{interface}") / ICMPv6EchoRequest() - log.debug(f"Sending request on {interface}") + log.debug("Sending request on %s", interface) send(packet, verbose=self.verbose) @@ -114,7 +114,7 @@ class Scanner(object): def handle_pkg(self, pkg): if ICMPv6EchoReply in pkg: host = pkg['IPv6'].src - log.verbose(f"Host {host} is alive") + log.verbose("Host %s is alive", host) dir = os.path.join(self.outdir, host) fname = os.path.join(dir, "last_seen") diff --git a/cdist/util/ipaddr.py b/cdist/util/ipaddr.py index 95ca74ee..d9e5f498 100644 --- a/cdist/util/ipaddr.py +++ b/cdist/util/ipaddr.py @@ -42,8 +42,7 @@ def resolve_target_host_name(host, family=0): # gethostbyaddr returns triple # (hostname, aliaslist, ipaddrlist) host_name = socket.gethostbyaddr(ip_addr)[0] - log.debug("derived host_name for host \"{}\": {}".format( - host, host_name)) + log.debug("derived host_name for host \"%s\": %s", host, host_name) except (socket.gaierror, socket.herror) as e: # in case of error provide empty value host_name = '' @@ -54,8 +53,7 @@ def resolve_target_fqdn(host): log = logging.getLogger(host) try: host_fqdn = socket.getfqdn(host) - log.debug("derived host_fqdn for host \"{}\": {}".format( - host, host_fqdn)) + log.debug("derived host_fqdn for host \"%s\": %s", host, host_fqdn) except socket.herror as e: # in case of error provide empty value host_fqdn = '' From 4c2d273f07e6cedb3dcb539d132cb0c3f8176d42 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Tue, 30 Mar 2021 07:56:38 +0200 Subject: [PATCH 2/2] Unify string formatting Use one way of string formatting: replace old `%` style with new `str.format`. Resolve #855. --- cdist/config.py | 4 ++-- cdist/core/cdist_object.py | 14 +++++++------- cdist/core/cdist_type.py | 4 ++-- cdist/core/code.py | 8 ++++---- cdist/core/explorer.py | 6 +++--- cdist/core/manifest.py | 11 +++++------ cdist/emulator.py | 12 ++++++------ cdist/exec/local.py | 11 ++++++----- cdist/exec/remote.py | 11 +++++------ cdist/message.py | 2 +- cdist/scan/scan.py | 2 +- cdist/shell.py | 2 +- cdist/test/cdist_object/__init__.py | 27 +++++++++++++++------------ cdist/test/emulator/__init__.py | 4 ++-- cdist/test/exec/remote.py | 8 ++++---- cdist/test/message/__init__.py | 2 +- cdist/util/fsproperty.py | 4 ++-- 17 files changed, 67 insertions(+), 65 deletions(-) diff --git a/cdist/config.py b/cdist/config.py index d6fec55f..adc460de 100644 --- a/cdist/config.py +++ b/cdist/config.py @@ -190,7 +190,7 @@ class Config: fd.write(sys.stdin.read()) except (IOError, OSError) as e: raise cdist.Error(("Creating tempfile for stdin data " - "failed: %s" % e)) + "failed: {}").format(e)) args.manifest = initial_manifest_temp_path atexit.register(lambda: os.remove(initial_manifest_temp_path)) @@ -764,7 +764,7 @@ class Config: raise cdist.UnresolvableRequirementsError( ("The requirements of the following objects could not be " - "resolved:\n%s") % ("\n".join(info_string))) + "resolved:\n{}").format("\n".join(info_string))) def _handle_deprecation(self, cdist_object): cdist_type = cdist_object.cdist_type diff --git a/cdist/core/cdist_object.py b/cdist/core/cdist_object.py index eea2c255..ccf7392d 100644 --- a/cdist/core/cdist_object.py +++ b/cdist/core/cdist_object.py @@ -34,17 +34,17 @@ class IllegalObjectIdError(cdist.Error): self.message = message or 'Illegal object id' def __str__(self): - return '%s: %s' % (self.message, self.object_id) + return '{}: {}'.format(self.message, self.object_id) class MissingObjectIdError(cdist.Error): def __init__(self, type_name): self.type_name = type_name - self.message = ("Type %s requires object id (is not a " - "singleton type)") % self.type_name + self.message = ("Type {} requires object id (is not a " + "singleton type)").format(self.type_name) def __str__(self): - return '%s' % (self.message) + return '{}'.format(self.message) class CdistObject: @@ -142,7 +142,7 @@ class CdistObject: if self.object_marker in self.object_id.split(os.sep): raise IllegalObjectIdError( self.object_id, ('object_id may not contain ' - '\'%s\'') % self.object_marker) + '\'{}\'').format(self.object_marker)) if '//' in self.object_id: raise IllegalObjectIdError( self.object_id, 'object_id may not contain //') @@ -189,7 +189,7 @@ class CdistObject: object_id=object_id) def __repr__(self): - return '' % self.name + return ''.format(self.name) def __eq__(self, other): """define equality as 'name is the same'""" @@ -277,7 +277,7 @@ class CdistObject: os.makedirs(path, exist_ok=allow_overwrite) except EnvironmentError as error: raise cdist.Error(('Error creating directories for cdist object: ' - '%s: %s') % (self, error)) + '{}: {}').format(self, error)) def requirements_unfinished(self, requirements): """Return state whether requirements are satisfied""" diff --git a/cdist/core/cdist_type.py b/cdist/core/cdist_type.py index 274de989..b68448bc 100644 --- a/cdist/core/cdist_type.py +++ b/cdist/core/cdist_type.py @@ -34,7 +34,7 @@ class InvalidTypeError(cdist.Error): self.source_path = os.path.realpath(self.type_absolute_path) def __str__(self): - return "Invalid type '%s' at '%s' defined at '%s'" % ( + return "Invalid type '{}' at '{}' defined at '{}'".format( self.type_path, self.type_absolute_path, self.source_path) @@ -109,7 +109,7 @@ class CdistType: return cls._instances[name] def __repr__(self): - return '' % self.name + return ''.format(self.name) def __eq__(self, other): return isinstance(other, self.__class__) and self.name == other.name diff --git a/cdist/core/code.py b/cdist/core/code.py index 1e9b4f80..12888ba4 100644 --- a/cdist/core/code.py +++ b/cdist/core/code.py @@ -122,8 +122,8 @@ class Code: def _run_gencode(self, cdist_object, which): cdist_type = cdist_object.cdist_type - script = os.path.join(self.local.type_path, - getattr(cdist_type, 'gencode_%s_path' % which)) + gencode_attr = getattr(cdist_type, 'gencode_{}_path'.format(which)) + script = os.path.join(self.local.type_path, gencode_attr) if os.path.isfile(script): env = os.environ.copy() env.update(self.env) @@ -167,8 +167,8 @@ class Code: def _run_code(self, cdist_object, which, env=None): which_exec = getattr(self, which) - script = os.path.join(which_exec.object_path, - getattr(cdist_object, 'code_%s_path' % which)) + code_attr = getattr(cdist_object, 'code_{}_path'.format(which)) + script = os.path.join(which_exec.object_path, code_attr) if which_exec.save_output_streams: stderr_path = os.path.join(cdist_object.stderr_path, 'code-' + which) diff --git a/cdist/core/explorer.py b/cdist/core/explorer.py index caa12a7d..48414ade 100644 --- a/cdist/core/explorer.py +++ b/cdist/core/explorer.py @@ -160,8 +160,8 @@ class Explorer: self.remote.transfer(self.local.global_explorer_path, self.remote.global_explorer_path, self.jobs) - self.remote.run(["chmod", "0700", - "%s/*" % (self.remote.global_explorer_path)]) + self.remote.run(["chmod", "0700", "{}/*".format( + self.remote.global_explorer_path)]) def run_global_explorer(self, explorer): """Run the given global explorer and return it's output.""" @@ -242,7 +242,7 @@ class Explorer: destination = os.path.join(self.remote.type_path, cdist_type.explorer_path) self.remote.transfer(source, destination) - self.remote.run(["chmod", "0700", "%s/*" % (destination)]) + self.remote.run(["chmod", "0700", "{}/*".format(destination)]) self._type_explorers_transferred.append(cdist_type.name) def transfer_object_parameters(self, cdist_object): diff --git a/cdist/core/manifest.py b/cdist/core/manifest.py index 3148d66c..09e74dac 100644 --- a/cdist/core/manifest.py +++ b/cdist/core/manifest.py @@ -80,13 +80,12 @@ class NoInitialManifestError(cdist.Error): if user_supplied: if os.path.islink(manifest_path): - self.message = "%s: %s -> %s" % ( - msg_header, manifest_path, - os.path.realpath(manifest_path)) + self.message = "{}: {} -> {}".format( + msg_header, manifest_path, os.path.realpath(manifest_path)) else: - self.message = "%s: %s" % (msg_header, manifest_path) + self.message = "{}: {}".format(msg_header, manifest_path) else: - self.message = "%s" % (msg_header) + self.message = "{}".format(msg_header) def __str__(self): return repr(self.message) @@ -107,7 +106,7 @@ class Manifest: self._open_logger() self.env = { - 'PATH': "%s:%s" % (self.local.bin_path, os.environ['PATH']), + 'PATH': "{}:{}".format(self.local.bin_path, os.environ['PATH']), # for use in type emulator '__cdist_type_base_path': self.local.type_path, '__global': self.local.base_path, diff --git a/cdist/emulator.py b/cdist/emulator.py index f09c282d..c55b47d2 100644 --- a/cdist/emulator.py +++ b/cdist/emulator.py @@ -36,8 +36,8 @@ from cdist.core.manifest import Manifest class MissingRequiredEnvironmentVariableError(cdist.Error): def __init__(self, name): self.name = name - self.message = ("Emulator requires the environment variable %s to be " - "setup" % self.name) + self.message = ("Emulator requires the environment variable {} to be " + "setup").format(self.name) def __str__(self): return self.message @@ -231,13 +231,13 @@ class Emulator: if self.cdist_object.exists and 'CDIST_OVERRIDE' not in self.env: obj_params = self._object_params_in_context() if obj_params != self.parameters: - errmsg = ("Object %s already exists with conflicting " - "parameters:\n%s: %s\n%s: %s" % ( + errmsg = ("Object {} already exists with conflicting " + "parameters:\n{}: {}\n{}: {}").format( self.cdist_object.name, " ".join(self.cdist_object.source), obj_params, self.object_source, - self.parameters)) + self.parameters) raise cdist.Error(errmsg) else: if self.cdist_object.exists: @@ -292,7 +292,7 @@ class Emulator: fd.write(chunk) chunk = self._read_stdin() except EnvironmentError as e: - raise cdist.Error('Failed to read from stdin: %s' % e) + raise cdist.Error('Failed to read from stdin: {}'.format(e)) def record_requirement(self, requirement): """record requirement and return recorded requirement""" diff --git a/cdist/exec/local.py b/cdist/exec/local.py index 6713cd13..370336ad 100644 --- a/cdist/exec/local.py +++ b/cdist/exec/local.py @@ -152,7 +152,7 @@ class Local: def _setup_object_marker_file(self): with open(self.object_marker_file, 'w') as fd: - fd.write("%s\n" % self.object_marker_name) + fd.write("{}\n".format(self.object_marker_name)) self.log.trace("Object marker %s saved in %s", self.object_marker_name, self.object_marker_file) @@ -184,7 +184,7 @@ class Local: """ assert isinstance(command, (list, tuple)), ( - "list or tuple argument expected, got: %s" % command) + "list or tuple argument expected, got: {}".format(command)) quiet = self.quiet_mode or quiet_mode do_save_output = save_output and not quiet and self.save_output_streams @@ -352,8 +352,9 @@ class Local: try: os.symlink(src, dst) except OSError as e: - raise cdist.Error("Linking %s %s to %s failed: %s" % ( - sub_dir, src, dst, e.__str__())) + raise cdist.Error( + "Linking {} {} to {} failed: {}".format( + sub_dir, src, dst, e.__str__())) def _link_types_for_emulator(self): """Link emulator to types""" @@ -366,5 +367,5 @@ class Local: os.symlink(src, dst) except OSError as e: raise cdist.Error( - "Linking emulator from %s to %s failed: %s" % ( + "Linking emulator from {} to {} failed: {}".format( src, dst, e.__str__())) diff --git a/cdist/exec/remote.py b/cdist/exec/remote.py index ea85da4c..af9e70cb 100644 --- a/cdist/exec/remote.py +++ b/cdist/exec/remote.py @@ -24,12 +24,10 @@ import os import glob import subprocess import logging -import multiprocessing import cdist import cdist.exec.util as util import cdist.util.ipaddr as ipaddr -from cdist.mputil import mp_pool_run def _wrap_addr(addr): @@ -262,9 +260,10 @@ class Remote: # remotely in e.g. csh and setting up CDIST_REMOTE_SHELL to e.g. # /bin/csh will execute this script in the right way. if env: - remote_env = [" export %s=%s;" % item for item in env.items()] - string_cmd = ("/bin/sh -c '" + " ".join(remote_env) + - " ".join(command) + "'") + remote_env = [" export {env[0]}={env[1]};".format(env=item) + for item in env.items()] + string_cmd = ("/bin/sh -c '{}{}'").format(" ".join(remote_env), + " ".join(command)) cmd.append(string_cmd) else: cmd.extend(command) @@ -278,7 +277,7 @@ class Remote: """ assert isinstance(command, (list, tuple)), ( - "list or tuple argument expected, got: %s" % command) + "list or tuple argument expected, got: {}".format(command)) close_stdout = False close_stderr = False diff --git a/cdist/message.py b/cdist/message.py index ffa8c2bb..0c4e21a6 100644 --- a/cdist/message.py +++ b/cdist/message.py @@ -70,7 +70,7 @@ class Message: with open(self.global_messages, 'a') as fd: for line in content: - fd.write("%s:%s" % (self.prefix, line)) + fd.write("{}:{}".format(self.prefix, line)) def merge_messages(self): self._merge_messages() diff --git a/cdist/scan/scan.py b/cdist/scan/scan.py index 9a0bcc52..faee8a56 100644 --- a/cdist/scan/scan.py +++ b/cdist/scan/scan.py @@ -93,7 +93,7 @@ class Trigger(object): time.sleep(self.sleeptime) def trigger(self, interface): - packet = IPv6(dst=f"ff02::1%{interface}") / ICMPv6EchoRequest() + packet = IPv6(dst="ff02::1{}".format(interface)) / ICMPv6EchoRequest() log.debug("Sending request on %s", interface) send(packet, verbose=self.verbose) diff --git a/cdist/shell.py b/cdist/shell.py index 04a68937..05803556 100644 --- a/cdist/shell.py +++ b/cdist/shell.py @@ -65,7 +65,7 @@ class Shell: def _init_environment(self): self.env = os.environ.copy() additional_env = { - 'PATH': "%s:%s" % (self.local.bin_path, os.environ['PATH']), + 'PATH': "{}:{}".format(self.local.bin_path, os.environ['PATH']), # for use in type emulator '__cdist_type_base_path': self.local.type_path, '__cdist_manifest': "cdist shell", diff --git a/cdist/test/cdist_object/__init__.py b/cdist/test/cdist_object/__init__.py index a9c20cd3..11619e96 100644 --- a/cdist/test/cdist_object/__init__.py +++ b/cdist/test/cdist_object/__init__.py @@ -86,8 +86,7 @@ class ObjectClassTestCase(test.CdistTestCase): def test_create_singleton(self): """Check whether creating an object without id (singleton) works""" - singleton = self.expected_objects[0].object_from_name( - "__test_singleton") + self.expected_objects[0].object_from_name("__test_singleton") # came here - everything fine def test_create_singleton_not_singleton_type(self): @@ -126,16 +125,16 @@ class ObjectIdTestCase(test.CdistTestCase): def test_object_id_contains_object_marker(self): cdist_type = core.CdistType(type_base_path, '__third') - illegal_object_id = ( - 'object_id/may/not/contain/%s/anywhere' % OBJECT_MARKER_NAME) + illegal_object_id = 'object_id/may/not/contain/{}/anywhere'.format( + OBJECT_MARKER_NAME) with self.assertRaises(core.IllegalObjectIdError): core.CdistObject(cdist_type, self.object_base_path, OBJECT_MARKER_NAME, illegal_object_id) def test_object_id_contains_object_marker_string(self): cdist_type = core.CdistType(type_base_path, '__third') - illegal_object_id = ( - 'object_id/may/contain_%s_in_filename' % OBJECT_MARKER_NAME) + illegal_object_id = 'object_id/may/contain_{}_in_filename'.format( + OBJECT_MARKER_NAME) core.CdistObject(cdist_type, self.object_base_path, OBJECT_MARKER_NAME, illegal_object_id) # if we get here, the test passed @@ -195,28 +194,32 @@ class ObjectTestCase(test.CdistTestCase): def test_path(self): self.assertEqual(self.cdist_object.path, - "__third/moon/%s" % OBJECT_MARKER_NAME) + "__third/moon/{}".format(OBJECT_MARKER_NAME)) def test_absolute_path(self): self.assertEqual(self.cdist_object.absolute_path, os.path.join(self.object_base_path, - "__third/moon/%s" % OBJECT_MARKER_NAME)) + "__third/moon/{}".format( + OBJECT_MARKER_NAME))) def test_code_local_path(self): self.assertEqual(self.cdist_object.code_local_path, - "__third/moon/%s/code-local" % OBJECT_MARKER_NAME) + "__third/moon/{}/code-local".format( + OBJECT_MARKER_NAME)) def test_code_remote_path(self): self.assertEqual(self.cdist_object.code_remote_path, - "__third/moon/%s/code-remote" % OBJECT_MARKER_NAME) + "__third/moon/{}/code-remote".format( + OBJECT_MARKER_NAME)) def test_parameter_path(self): self.assertEqual(self.cdist_object.parameter_path, - "__third/moon/%s/parameter" % OBJECT_MARKER_NAME) + "__third/moon/{}/parameter".format( + OBJECT_MARKER_NAME)) def test_explorer_path(self): self.assertEqual(self.cdist_object.explorer_path, - "__third/moon/%s/explorer" % OBJECT_MARKER_NAME) + "__third/moon/{}/explorer".format(OBJECT_MARKER_NAME)) def test_parameters(self): expected_parameters = {'planet': 'Saturn', 'name': 'Prometheus'} diff --git a/cdist/test/emulator/__init__.py b/cdist/test/emulator/__init__.py index befd7b57..4b2bc3ba 100644 --- a/cdist/test/emulator/__init__.py +++ b/cdist/test/emulator/__init__.py @@ -84,8 +84,8 @@ class EmulatorTestCase(test.CdistTestCase): def test_illegal_object_id_requirement(self): argv = ['__file', '/tmp/foobar'] - self.env['require'] = ( - "__file/bad/id/with/%s/inside") % self.local.object_marker_name + self.env['require'] = "__file/bad/id/with/{}/inside".format( + self.local.object_marker_name) emu = emulator.Emulator(argv, env=self.env) self.assertRaises(core.IllegalObjectIdError, emu.run) diff --git a/cdist/test/exec/remote.py b/cdist/test/exec/remote.py index a7fe384d..b23f8447 100644 --- a/cdist/test/exec/remote.py +++ b/cdist/test/exec/remote.py @@ -47,8 +47,8 @@ class RemoteTestCase(test.CdistTestCase): args = (self.target_host,) kwargs.setdefault('base_path', self.base_path) user = getpass.getuser() - kwargs.setdefault('remote_exec', 'ssh -o User=%s -q' % user) - kwargs.setdefault('remote_copy', 'scp -o User=%s -q' % user) + kwargs.setdefault('remote_exec', 'ssh -o User={} -q'.format(user)) + kwargs.setdefault('remote_copy', 'scp -o User={} -q'.format(user)) if 'stdout_base_path' not in kwargs: stdout_path = os.path.join(self.temp_dir, 'stdout') os.makedirs(stdout_path, exist_ok=True) @@ -170,7 +170,7 @@ class RemoteTestCase(test.CdistTestCase): r = self.create_remote(remote_exec=remote_exec, remote_copy=remote_copy) self.assertEqual(r.run('true', return_output=True), - "%s\n" % self.target_host[0]) + "{}\n".format(self.target_host[0])) def test_run_script_target_host_in_env(self): handle, remote_exec_path = self.mkstemp(dir=self.temp_dir) @@ -185,7 +185,7 @@ class RemoteTestCase(test.CdistTestCase): with os.fdopen(handle, "w") as fd: fd.writelines(["#!/bin/sh\n", "true"]) self.assertEqual(r.run_script(script, return_output=True), - "%s\n" % self.target_host[0]) + "{}\n".format(self.target_host[0])) def test_run_script_with_env_target_host_in_env(self): handle, script = self.mkstemp(dir=self.temp_dir) diff --git a/cdist/test/message/__init__.py b/cdist/test/message/__init__.py index 61cd5d97..55040fc9 100644 --- a/cdist/test/message/__init__.py +++ b/cdist/test/message/__init__.py @@ -67,7 +67,7 @@ class MessageTestCase(test.CdistTestCase): def test_message_merge_prefix(self): """Ensure messages are merged and are prefixed""" - expectedcontent = "%s:%s" % (self.prefix, self.content) + expectedcontent = "{}:{}".format(self.prefix, self.content) out = self.message.env['__messages_out'] diff --git a/cdist/util/fsproperty.py b/cdist/util/fsproperty.py index 1d76fd76..09e9cc19 100644 --- a/cdist/util/fsproperty.py +++ b/cdist/util/fsproperty.py @@ -30,7 +30,7 @@ class AbsolutePathRequiredError(cdist.Error): self.path = path def __str__(self): - return 'Absolute path required, got: %s' % self.path + return 'Absolute path required, got: {}'.format(self.path) class FileList(collections.MutableSequence): @@ -218,7 +218,7 @@ class FileBasedProperty: def _get_attribute(self, instance, owner): name = self._get_property_name(owner) - attribute_name = '__%s' % name + attribute_name = '__{}'.format(name) if not hasattr(instance, attribute_name): path = self._get_path(instance) attribute_instance = self.attribute_class(path)