From 82eff4a1ca30c5dd61705ca0b67ef05cc173d3ea Mon Sep 17 00:00:00 2001 From: stdweird Date: Tue, 23 Feb 2016 08:49:47 +0100 Subject: [PATCH 1/5] vsc.utils.run: improve handling tasks and pgids --- lib/vsc/utils/run.py | 60 +++++++++++++++++++++++++++++++------------- setup.py | 2 +- 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/lib/vsc/utils/run.py b/lib/vsc/utils/run.py index dfe2b0f2..2fde770a 100644 --- a/lib/vsc/utils/run.py +++ b/lib/vsc/utils/run.py @@ -113,7 +113,7 @@ def __init__(self, cmd=None, **kwargs): @param input: set "simple" input @param startpath: directory to change to before executing command @param disable_log: use fake logger (won't log anything) - @param use_shell: use the subshell + @param use_shell: use the subshell @param shell: change the shell """ self.input = kwargs.pop('input', None) @@ -380,29 +380,53 @@ def _killtasks(self, tasks=None, sig=signal.SIGKILL, kill_pgid=False): Kill all tasks @param: tasks list of processids @param: sig, signal to use to kill - @apram: kill_pgid, send kill to group + @param: kill_pgid, send kill to group """ if tasks is None: self.log.error("killtasks no tasks passed") - elif isinstance(tasks, basestring): - try: - tasks = [int(tasks)] - except: - self.log.error("killtasks failed to convert tasks string %s to int" % tasks) + return + elif not isinstance(tasks, (list, tuple,)): + tasks = [tasks] + pids = [] for pid in tasks: - pgid = os.getpgid(pid) + # Try to convert as much pids as possible try: - os.kill(int(pid), sig) - if kill_pgid: - os.killpg(pgid, sig) - self.log.debug("Killed %s with signal %s" % (pid, sig)) - except OSError, err: + pids.append(int(pid)) + except ValueError: + self.log.error("killtasks failed to convert task/pid %s to integer" % pid) + + def do_something_with_pid(fn, args, msg): + """ + Handle interaction with process ids gracefully + Does not raise anything, and handles missing pid + Return the result of the function call + args, or None + """ + res = None + try: + res = fn(*args) + except Exception as err: # ERSCH is no such process, so no issue - if not err.errno == errno.ESRCH: - self.log.error("Failed to kill %s: %s" % (pid, err)) - except Exception, err: - self.log.error("Failed to kill %s: %s" % (pid, err)) + if not (isinstance(err, OSError) and err.errno == errno.ESRCH): + self.log.error("Failed to %s from %s: %s" % (msg, pid, err)) + + return res + + for pid in pids: + + # Get pgid before killing it + pgid = None + if kill_pgid: + # This can't be fatal, whatever happens here, still try to kill the pid + pgid = do_something_with_pid(os.getpgid, [pid], 'find pgid') + + do_something_with_pid(os.kill, [pid, sig], 'kill pid') + + if kill_pgid: + if pgid is None: + self.log.error("Can't kill pgid for pid %s, None found" % pid) + else: + do_something_with_pid(os.kill, [pgid, sig], 'kill pgid') def stop_tasks(self): """Cleanup current run""" @@ -480,7 +504,7 @@ def _wait_for_process(self): # process after updating the self._process_ vars self._loop_process_output_final(output) - except RunLoopException, err: + except RunLoopException as err: self.log.debug('RunLoopException %s' % err) self._process_output = err.output self._process_exitcode = err.code diff --git a/setup.py b/setup.py index eb990a76..a1fcdfe8 100755 --- a/setup.py +++ b/setup.py @@ -39,7 +39,7 @@ VSC_INSTALL_REQ_VERSION = '0.10.1' PACKAGE = { - 'version': '2.5.1', + 'version': '2.5.2', 'author': [sdw, jt, ag, kh], 'maintainer': [sdw, jt, ag, kh], # as long as 1.0.0 is not out, vsc-base should still provide vsc.fancylogger From e98c67fe3ec1a522b2aafc977bd7cd5375d5282c Mon Sep 17 00:00:00 2001 From: stdweird Date: Mon, 29 Aug 2016 22:48:48 +0200 Subject: [PATCH 2/5] adapt to vsc-install --- .gitignore | 1 + bin/logdaemon.py | 2 +- lib/vsc/utils/affinity.py | 29 ++++++++++---------- lib/vsc/utils/dateandtime.py | 21 ++++++--------- lib/vsc/utils/docs.py | 1 - lib/vsc/utils/fancylogger.py | 6 ++--- lib/vsc/utils/generaloption.py | 49 +++++++++++++++++----------------- lib/vsc/utils/mail.py | 9 +++++-- lib/vsc/utils/missing.py | 19 ++++--------- lib/vsc/utils/optcomplete.py | 11 ++++---- lib/vsc/utils/run.py | 24 ++++++++--------- lib/vsc/utils/testing.py | 2 +- lib/vsc/utils/wrapper.py | 2 +- test/generaloption.py | 8 +++--- 14 files changed, 89 insertions(+), 95 deletions(-) diff --git a/.gitignore b/.gitignore index 6e447363..14fb6bd4 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,7 @@ setup.cfg # Packages +.eggs *.egg *.egg-info dist diff --git a/bin/logdaemon.py b/bin/logdaemon.py index 9656dcbe..a92f88bf 100644 --- a/bin/logdaemon.py +++ b/bin/logdaemon.py @@ -114,7 +114,7 @@ def run(self): self.logger.handle(logrecord) except (KeyboardInterrupt, SystemExit): raise - except: + except Exception: traceback.print_exc() diff --git a/lib/vsc/utils/affinity.py b/lib/vsc/utils/affinity.py index c9eefa5a..7872af53 100644 --- a/lib/vsc/utils/affinity.py +++ b/lib/vsc/utils/affinity.py @@ -120,7 +120,7 @@ def convert_hr_bits(self, txt): for rng in txt.split(','): indices = [int(x) for x in rng.split('-')] * 2 # always at least 2 elements: twice the same or start,end,start,end - ## sanity check + # sanity check if indices[1] < indices[0]: self.log.raiseException("convert_hr_bits: end is lower then start in '%s'" % rng) elif indices[0] < 0: @@ -153,7 +153,7 @@ def get_cpus(self): """ self.cpus = [] for bitmask in getattr(self, '__bits'): - for idx in xrange(NCPUBITS): + for _ in range(NCPUBITS): self.cpus.append(bitmask & 1) bitmask >>= 1 return self.cpus @@ -179,12 +179,12 @@ def set_bits(self, cpus=None): for idx in xrange(NMASKBITS): cpus = [2 ** cpuidx for cpuidx, val in enumerate(self.cpus[idx * NCPUBITS:(idx + 1) * NCPUBITS]) if val == 1] __bits[idx] = cpu_mask_t(sum(cpus)) - ## sanity check - if not prev_cpus == self.get_cpus(): - ## get_cpus() rescans - self.log.raiseException("set_bits: something went wrong: previous cpus %s; current ones %s" % (prev_cpus[:20], self.cpus[:20])) - else: + # sanity check + if prev_cpus == self.get_cpus(): self.log.debug("set_bits: new set to %s" % self.convert_bits_hr()) + else: + # get_cpus() rescans + self.log.raiseException("set_bits: something went wrong: previous cpus %s; current ones %s" % (prev_cpus[:20], self.cpus[:20])) def str_cpus(self): """Return a string representation of the cpus""" @@ -238,7 +238,7 @@ def sched_getcpu(): # tobin not used anymore def tobin(s): """Convert integer to binary format""" - ## bin() missing in 2.4 + # bin() missing in 2.4 # eg: self.cpus.extend([int(x) for x in tobin(bitmask).zfill(NCPUBITS)[::-1]]) if s <= 1: return str(s) @@ -260,7 +260,7 @@ def getpriority(which=None, who=None): """Get the priority""" if which is None: which = PRIO_PROCESS - elif not which in (PRIO_PROCESS, PRIO_PGRP, PRIO_USER,): + elif which not in (PRIO_PROCESS, PRIO_PGRP, PRIO_USER,): _logger.raiseException("getpriority: which %s not in correct range" % which) if who is None: who = 0 # current which-ever @@ -275,13 +275,14 @@ def setpriority(prio, which=None, who=None): """Set the priority (aka nice)""" if which is None: which = PRIO_PROCESS - elif not which in (PRIO_PROCESS, PRIO_PGRP, PRIO_USER,): + elif which not in (PRIO_PROCESS, PRIO_PGRP, PRIO_USER,): _logger.raiseException("setpriority: which %s not in correct range" % which) if who is None: who = 0 # current which-ever + try: prio = int(prio) - except: + except ValueError: _logger.raiseException("setpriority: failed to convert priority %s into int" % prio) if prio < PRIO_MIN or prio > PRIO_MAX: @@ -298,7 +299,7 @@ def setpriority(prio, which=None, who=None): if __name__ == '__main__': - ## some examples of usage + # some examples of usage setLogLevelDebug() cs = cpu_set_t() @@ -323,8 +324,8 @@ def setpriority(prio, which=None, who=None): print sched_getcpu() - ## resources - ## nice -n 5 python affinity.py prints 5 here + # resources + # nice -n 5 python affinity.py prints 5 here currentprio = getpriority() print "getpriority", currentprio newprio = 10 diff --git a/lib/vsc/utils/dateandtime.py b/lib/vsc/utils/dateandtime.py index 75c6201e..b010d0af 100644 --- a/lib/vsc/utils/dateandtime.py +++ b/lib/vsc/utils/dateandtime.py @@ -34,11 +34,6 @@ import time as _time from datetime import tzinfo, timedelta, datetime, date -try: - any([0, 1]) -except: - from vsc.utils.missing import any - class FancyMonth: """Convenience class for month math""" @@ -152,7 +147,7 @@ def interval(self, otherdate): raise(Exception(msg)) else: nr = self.number(otherdate) - startdate, enddate = self.get_start_end(otherdate) + startdate, _ = self.get_start_end(otherdate) start = self.__class__(startdate) all_dates = [start.get_other(m) for m in range(nr)] @@ -250,7 +245,7 @@ def datetime_parser(txt): try: sects = tmpts[1].split(':')[2].split('.') - except: + except (AttributeError, IndexError): sects = [0] # add seconds datetuple.append(int(sects[0])) @@ -280,13 +275,13 @@ def timestamp_parser(timestamp): class UTC(tzinfo): """UTC""" - def utcoffset(self, dt): + def utcoffset(self, dt): # pylint:disable=unused-argument return ZERO - def tzname(self, dt): + def tzname(self, dt): # pylint:disable=unused-argument return "UTC" - def dst(self, dt): + def dst(self, dt): # pylint:disable=unused-argument return ZERO utc = UTC() @@ -303,13 +298,13 @@ def __init__(self, offset, name): self.__offset = timedelta(minutes=offset) self.__name = name - def utcoffset(self, dt): + def utcoffset(self, dt): # pylint:disable=unused-argument return self.__offset - def tzname(self, dt): + def tzname(self, dt): # pylint:disable=unused-argument return self.__name - def dst(self, dt): + def dst(self, dt): # pylint:disable=unused-argument return ZERO diff --git a/lib/vsc/utils/docs.py b/lib/vsc/utils/docs.py index ae1341c9..435f7ff7 100644 --- a/lib/vsc/utils/docs.py +++ b/lib/vsc/utils/docs.py @@ -45,7 +45,6 @@ def mk_rst_table(titles, columns): msg = "Number of titles/columns should be equal, found %d titles and %d columns" % (title_cnt, col_cnt) raise LengthNotEqualException, msg table = [] - col_widths = [] tmpl = [] line= [] diff --git a/lib/vsc/utils/fancylogger.py b/lib/vsc/utils/fancylogger.py index c17e3c7e..f8db572f 100644 --- a/lib/vsc/utils/fancylogger.py +++ b/lib/vsc/utils/fancylogger.py @@ -194,7 +194,7 @@ def log_method(self, msg): RAISE_EXCEPTION_LOG_METHOD = log_method # method definition as it is in logging, can't change this - def makeRecord(self, name, level, pathname, lineno, msg, args, excinfo, func=None, extra=None): + def makeRecord(self, name, level, pathname, lineno, msg, args, excinfo, func=None, extra=None): # pylint: disable=unused-argument """ overwrite make record to use a fancy record (with more options) """ @@ -243,7 +243,7 @@ def raiseException(self, message, exception=None, catch=False): self.RAISE_EXCEPTION_LOG_METHOD(fullmessage) raise exception, message, tb - def deprecated(self, msg, cur_ver, max_ver, depth=2, exception=None, *args, **kwargs): + def deprecated(self, msg, cur_ver, max_ver, depth=2, exception=None, *args, **kwargs): # pylint: disable=unused-argument """ Log deprecation message, throw error if current version is passed given threshold. @@ -718,7 +718,7 @@ def _enable_disable_default_handlers(enable): try: _default_logTo(enable=enable, handler=handler) - except: + except Exception: pass diff --git a/lib/vsc/utils/generaloption.py b/lib/vsc/utils/generaloption.py index 3f4f16bd..9ce247c2 100644 --- a/lib/vsc/utils/generaloption.py +++ b/lib/vsc/utils/generaloption.py @@ -40,8 +40,8 @@ import StringIO import sys import textwrap -from optparse import OptionParser, OptionGroup, Option, Values, HelpFormatter -from optparse import BadOptionError, SUPPRESS_USAGE, NO_DEFAULT, OptionValueError +from optparse import OptionParser, OptionGroup, Option, Values +from optparse import BadOptionError, SUPPRESS_USAGE, OptionValueError from optparse import SUPPRESS_HELP as nohelp # supported in optparse of python v2.4 from optparse import gettext as _gettext # this is gettext.gettext normally from vsc.utils.dateandtime import date_parser, datetime_parser @@ -67,7 +67,7 @@ def set_columns(cols=None): if os.path.exists(stty): try: cols = int(os.popen('%s size 2>/dev/null' % stty).read().strip().split(' ')[1]) - except: + except (ValueError, OSError, AttributeError): # do nothing pass @@ -93,7 +93,7 @@ def what_str_list_tuple(name): return sep, klass, helpsep -def check_str_list_tuple(option, opt, value): +def check_str_list_tuple(option, opt, value): # pylint: disable=unused-argument """ check function for strlist and strtuple type assumes value is comma-separated list @@ -199,7 +199,7 @@ def _set_attrs(self, attrs): elif self.action in self.EXTOPTION_STORE_OR: setattr(self, 'store_or', self.action) - def store_or(option, opt_str, value, parser, *args, **kwargs): + def store_or(option, opt_str, value, parser, *args, **kwargs): # pylint: disable=unused-argument """Callback for supporting options with optional values.""" # see http://stackoverflow.com/questions/1229146/parsing-empty-options-in-python # ugly code, optparse is crap @@ -475,7 +475,7 @@ def __init__(self, *args, **kwargs): # py2.4 epilog compatibilty with py2.7 / optparse 1.5.3 self.epilog = kwargs.pop('epilog', None) - if not 'option_class' in kwargs: + if 'option_class' not in kwargs: kwargs['option_class'] = ExtOption OptionParser.__init__(self, *args, **kwargs) @@ -560,7 +560,7 @@ def set_description_docstring(self): stack = inspect.stack()[-1] try: docstr = stack[0].f_globals.get('__doc__', None) - except: + except (IndexError, ValueError, AttributeError): self.log.debug("set_description_docstring: no docstring found in latest stack globals") docstr = None @@ -577,7 +577,7 @@ def set_description_docstring(self): # default textwrap width try: kwargs['width'] = int(width) - except: + except ValueError: pass # deal with newlines in docstring @@ -738,7 +738,7 @@ def print_confighelp(self, fh=None): for gr in self.option_groups: section = gr.section_name if not (section is None or section == ExtOptionGroup.NO_SECTION): - if not section in sections: + if section not in sections: sections.append(section) ag = all_groups.setdefault(section, []) ag.extend(gr.section_options) @@ -762,8 +762,9 @@ def print_confighelp(self, fh=None): txt += "\n" # overwrite the format_help to be able to use the the regular print_help - def format_help(*args, **kwargs): + def format_help(*args, **kwargs): # pylint: disable=unused-argument return txt + self.format_help = format_help self.print_help(fh) @@ -829,7 +830,7 @@ def get_env_options(self): self.environment_arguments.append("%s=%s" % (lo, val)) else: # interpretation of values: 0/no/false means: don't set it - if not ("%s" % val).lower() in ("0", "no", "false",): + if ("%s" % val).lower() not in ("0", "no", "false",): self.environment_arguments.append("%s" % lo) else: self.log.debug("Environment variable %s is not set" % env_opt_name) @@ -1050,7 +1051,7 @@ def main_options(self): fn() self.auto_section_name = None # reset it - def make_option_metavar(self, longopt, details): + def make_option_metavar(self, longopt, details): # pylint: disable=unused-argument """Generate the metavar for option longopt @type longopt: str @type details: tuple @@ -1164,7 +1165,7 @@ def add_group_parser(self, opt_dict, description, prefix=None, otherdefaults=Non # this has to match PROCESSED_OPTIONS_PROPERTIES self.processed_options[opt_dest] = [typ, default, action, opt_name, prefix, section_name] # add longopt - if not len(self.processed_options[opt_dest]) == len(self.PROCESSED_OPTIONS_PROPERTIES): + if len(self.processed_options[opt_dest]) != len(self.PROCESSED_OPTIONS_PROPERTIES): self.log.raiseException("PROCESSED_OPTIONS_PROPERTIES length mismatch") nameds = { @@ -1218,7 +1219,7 @@ def add_group_parser(self, opt_dict, description, prefix=None, otherdefaults=Non # map between prefix and sectionnames prefix_section_names = self.config_prefix_sectionnames_map.setdefault(prefix, []) - if not section_name in prefix_section_names: + if section_name not in prefix_section_names: prefix_section_names.append(section_name) self.log.debug("Added prefix %s to list of sectionnames for %s" % (prefix, section_name)) @@ -1335,12 +1336,12 @@ def parseconfigfiles(self): try: parsed_files = self.configfile_parser.read(configfiles) - except: + except Exception: self.log.raiseException("parseconfigfiles: problem during read") self.log.debug("parseconfigfiles: following files were parsed %s" % parsed_files) self.log.debug("parseconfigfiles: following files were NOT parsed %s" % - [x for x in configfiles if not x in parsed_files]) + [x for x in configfiles if x not in parsed_files]) self.log.debug("parseconfigfiles: sections (w/o %s) %s" % (self.DEFAULTSECT, self.configfile_parser.sections())) @@ -1354,7 +1355,7 @@ def parseconfigfiles(self): # won't parse cfg_sections = self.config_prefix_sectionnames_map.values() # without DEFAULT for section in cfg_sections: - if not section in self.config_prefix_sectionnames_map.values(): + if section not in self.config_prefix_sectionnames_map.values(): self.log.warning("parseconfigfiles: found section %s, won't be parsed" % section) continue @@ -1417,7 +1418,7 @@ def parseconfigfiles(self): newval = self.configfile_parser.getboolean(section, opt) self.log.debug(('parseconfigfiles: getboolean for option %s value %s ' 'in section %s returned %s') % (opt, val, section, newval)) - except: + except Exception: self.log.raiseException(('parseconfigfiles: failed to getboolean for option %s value %s ' 'in section %s') % (opt, val, section)) if hasattr(self.parser.option_class, 'ENABLE') and hasattr(self.parser.option_class, 'DISABLE'): @@ -1443,7 +1444,7 @@ def parseconfigfiles(self): self.parser.process_env_options = False (parsed_configfile_options, parsed_configfile_args) = self.parser.parse_args(configfile_cmdline) self.parser.process_env_options = True - except: + except Exception: self.log.raiseException('parseconfigfiles: failed to parse options through cmdline %s' % configfile_cmdline) @@ -1456,7 +1457,7 @@ def parseconfigfiles(self): for opt_dest in configfile_cmdline_dest: try: configfile_values[opt_dest] = getattr(parsed_configfile_options, opt_dest) - except: + except AttributeError: self.log.raiseException('parseconfigfiles: failed to retrieve dest %s from parsed_configfile_options' % opt_dest) @@ -1494,7 +1495,7 @@ def make_options_option_name_and_destination(self, prefix, key): def _get_options_by_property(self, prop_type, prop_value): """Return all options with property type equal to value""" - if not prop_type in self.PROCESSED_OPTIONS_PROPERTIES: + if prop_type not in self.PROCESSED_OPTIONS_PROPERTIES: self.log.raiseException('Invalid prop_type %s for PROCESSED_OPTIONS_PROPERTIES %s' % (prop_type, self.PROCESSED_OPTIONS_PROPERTIES)) prop_idx = self.PROCESSED_OPTIONS_PROPERTIES.index(prop_type) @@ -1705,7 +1706,7 @@ class SimpleOptionParser(ExtOptionParser): class SimpleOption(GeneralOption): PARSER = SimpleOptionParser - def __init__(self, go_dict=None, descr=None, short_groupdescr=None, long_groupdescr=None, config_files=None): + def __init__(self, go_dict=None, short_groupdescr=None, long_groupdescr=None, config_files=None): """Initialisation @param go_dict : General Option option dict @param short_descr : short description of main options @@ -1740,7 +1741,7 @@ def main_options(self): self.add_group_parser(self.go_dict, self.descr, prefix=prefix) -def simple_option(go_dict=None, descr=None, short_groupdescr=None, long_groupdescr=None, config_files=None): +def simple_option(go_dict=None, short_groupdescr=None, long_groupdescr=None, config_files=None): """A function that returns a single level GeneralOption option parser @param go_dict : General Option option dict @@ -1754,4 +1755,4 @@ def simple_option(go_dict=None, descr=None, short_groupdescr=None, long_groupdes the generated help will include the docstring """ - return SimpleOption(go_dict, descr, short_groupdescr, long_groupdescr, config_files) + return SimpleOption(go_dict=go_dict, short_groupdescr=short_groupdescr, long_groupdescr=long_groupdescr, config_files=config_files) diff --git a/lib/vsc/utils/mail.py b/lib/vsc/utils/mail.py index 60e2f1b3..d49b4e34 100644 --- a/lib/vsc/utils/mail.py +++ b/lib/vsc/utils/mail.py @@ -149,12 +149,13 @@ def sendTextMail(self, """ self.log.info("Sending mail [%s] to %s." % (mail_subject, mail_to)) - if reply_to is None: - reply_to = mail_from msg = MIMEText(message) msg['Subject'] = mail_subject msg['From'] = mail_from msg['To'] = mail_to + + if reply_to is None: + reply_to = mail_from msg['Reply-to'] = reply_to self._send(mail_from, mail_to, mail_subject, msg) @@ -218,6 +219,10 @@ def sendHTMLMail(self, msg_root['From'] = mail_from msg_root['To'] = mail_to + if reply_to is None: + reply_to = mail_from + msg_root['Reply-to'] = reply_to + msg_root.preamble = 'This is a multi-part message in MIME format. If your email client does not support this (correctly), the first part is the plain text version.' # Create the body of the message (a plain-text and an HTML version). diff --git a/lib/vsc/utils/missing.py b/lib/vsc/utils/missing.py index 80353e8c..0dfe4213 100644 --- a/lib/vsc/utils/missing.py +++ b/lib/vsc/utils/missing.py @@ -66,18 +66,9 @@ def newfunc(*fargs, **fkeywords): newfunc.keywords = keywords return newfunc - -def any(ls): - """Reimplementation of 'any' function, which is not available in Python 2.4 yet.""" - - return sum([bool(x) for x in ls]) != 0 - - -def all(ls): - """Reimplementation of 'all' function, which is not available in Python 2.4 yet.""" - - return sum([bool(x) for x in ls]) == len(ls) - +# Placeholder, used to have implementations for any and all that were missing in py24 +any = any # pylint: disable=redefined-builtin +all = all # pylint: disable=redefined-builtin def nub(list_): """Returns the unique items of a list of hashables, while preserving order of @@ -253,7 +244,7 @@ def __init__(self, *args, **kwargs): # handle unknown keys: either ignore them or raise an exception tmpdict = dict(*args, **kwargs) - unknown_keys = [key for key in tmpdict.keys() if not key in self.KNOWN_KEYS] + unknown_keys = [key for key in tmpdict.keys() if key not in self.KNOWN_KEYS] if unknown_keys: if ignore_unknown_keys: for key in unknown_keys: @@ -373,6 +364,6 @@ def topological_sort(graph): visited = set() for root in graph: for node in post_order(graph, root): - if not node in visited: + if node not in visited: yield node visited.add(node) diff --git a/lib/vsc/utils/optcomplete.py b/lib/vsc/utils/optcomplete.py index f6e2ddaa..a5384864 100644 --- a/lib/vsc/utils/optcomplete.py +++ b/lib/vsc/utils/optcomplete.py @@ -154,7 +154,7 @@ def __call__(self, **kwargs): if self.CALL_ARGS is not None: for arg in self.CALL_ARGS: all_args.append(arg) - if not arg in kwargs: + if arg not in kwargs: msg = "%s __call__ missing mandatory arg %s" % (self.__class__.__name__, arg) raise CompleterMissingCallArgument(msg) @@ -162,13 +162,13 @@ def __call__(self, **kwargs): all_args.extend(self.CALL_ARGS_OPTIONAL) for arg in kwargs.keys(): - if not arg in all_args: + if arg not in all_args: # remove it kwargs.pop(arg) return self._call(**kwargs) - def _call(self, **kwargs): + def _call(self, **kwargs): # pylint: disable=unused-argument """Return empty list""" return [] @@ -343,7 +343,7 @@ def extract_word(line, point): def error_override(self, msg): """Hack to keep OptionParser from writing to sys.stderr when calling self.exit from self.error""" - self.exit(2, msg=None) + self.exit(2, msg=msg) def guess_first_nonoption(gparser, subcmds_map): @@ -352,9 +352,8 @@ def guess_first_nonoption(gparser, subcmds_map): subcommand syntax, so that we can generate the appropriate completions for the subcommand.""" - gparser = copy.deepcopy(gparser) - def print_usage_nousage (self, *args, **kwargs): + def print_usage_nousage (self, *args, **kwargs): # pylint: disable=unused-argument pass gparser.print_usage = print_usage_nousage diff --git a/lib/vsc/utils/run.py b/lib/vsc/utils/run.py index 2fde770a..fd8e1e99 100644 --- a/lib/vsc/utils/run.py +++ b/lib/vsc/utils/run.py @@ -71,7 +71,7 @@ import sys import time -from vsc.utils.fancylogger import getLogger, getAllExistingLoggers +from vsc.utils.fancylogger import getLogger PROCESS_MODULE_ASYNCPROCESS_PATH = 'vsc.utils.asyncprocess' @@ -87,7 +87,7 @@ class DummyFunction(object): def __getattr__(self, name): - def dummy(*args, **kwargs): + def dummy(*args, **kwargs): # pylint: disable=unused-argument pass return dummy @@ -253,7 +253,7 @@ def _start_in_path(self): try: self._cwd_before_startpath = os.getcwd() # store it some one can return to it os.chdir(self.startpath) - except: + except OSError: self.raiseException("_start_in_path: failed to change path from %s to startpath %s" % (self._cwd_before_startpath, self.startpath)) else: @@ -272,11 +272,11 @@ def _return_to_previous_start_in_path(self): if os.path.isdir(self._cwd_before_startpath): try: currentpath = os.getcwd() - if not currentpath == self.startpath: + if currentpath != self.startpath: self.log.warning(("_return_to_previous_start_in_path: current diretory %s does not match " "startpath %s") % (currentpath, self.startpath)) os.chdir(self._cwd_before_startpath) - except: + except OSError: self.raiseException(("_return_to_previous_start_in_path: failed to change path from current %s " "to previous path %s") % (currentpath, self._cwd_before_startpath)) else: @@ -325,7 +325,7 @@ def _init_input(self): if self.input is not None: # allow empty string (whatever it may mean) try: self._process.stdin.write(self.input) - except: + except Exception: self.log.raiseException("_init_input: Failed write input %s to process" % self.input) if self.INIT_INPUT_CLOSE: @@ -341,7 +341,7 @@ def _wait_for_process(self): try: self._process_exitcode = self._process.wait() self._process_output = self._read_process(-1) # -1 is read all - except: + except Exception: self.log.raiseException("_wait_for_process: problem during wait exitcode %s output %s" % (self._process_exitcode, self._process_output)) @@ -433,7 +433,7 @@ def stop_tasks(self): self._killtasks(tasks=[self._process.pid]) try: os.waitpid(-1, os.WNOHANG) - except: + except OSError: pass @@ -610,13 +610,13 @@ def _make_popen_named_args(self, others=None): if dirname and not os.path.isdir(dirname): try: os.makedirs(dirname) - except: + except OSError: self.log.raiseException(("_make_popen_named_args: dirname %s for file %s does not exists. " "Creating it failed.") % (dirname, self.filename)) try: self.filehandle = open(self.filename, 'w') - except: + except OSError: self.log.raiseException("_make_popen_named_args: failed to open filehandle for file %s" % self.filename) others = { @@ -629,7 +629,7 @@ def _cleanup_process(self): """Close the filehandle""" try: self.filehandle.close() - except: + except OSError: self.log.raiseException("_cleanup_process: failed to close filehandle for filename %s" % self.filename) def _read_process(self, readsize=None): @@ -645,7 +645,7 @@ def _read_process(self, readsize=None): def _make_popen_named_args(self, others=None): if others is None: - (master, slave) = pty.openpty() + (_, slave) = pty.openpty() others = { 'stdin': slave, 'stdout': slave, diff --git a/lib/vsc/utils/testing.py b/lib/vsc/utils/testing.py index 1e0412ba..fd98ab55 100644 --- a/lib/vsc/utils/testing.py +++ b/lib/vsc/utils/testing.py @@ -30,4 +30,4 @@ """ -from vsc.install.testing import TestCase as EnhancedTestCase +from vsc.install.testing import TestCase as EnhancedTestCase # pylint: disable=unused-import diff --git a/lib/vsc/utils/wrapper.py b/lib/vsc/utils/wrapper.py index 569ca379..31de7af2 100644 --- a/lib/vsc/utils/wrapper.py +++ b/lib/vsc/utils/wrapper.py @@ -30,7 +30,7 @@ class __metaclass__(type): def __init__(cls, name, bases, dct): def make_proxy(name): - def proxy(self, *args): + def proxy(self, *args): # pylint:disable=unused-argument return getattr(self._obj, name) return proxy diff --git a/test/generaloption.py b/test/generaloption.py index fa10614f..fa8095f9 100644 --- a/test/generaloption.py +++ b/test/generaloption.py @@ -693,9 +693,12 @@ def test_optcomplete(self): ec, out = run_simple('%s %s; test $? == 1' % (pythonpath, gen_cmdline(cmd_list, partial, shebang=False))) # tabcompletion ends with exit 1!; test returns this to 0 # avoids run.log.error message - self.assertEqual(ec, 0) + self.assertEqual(ec, 0, msg="simple_option.py test script ran success") - compl_opts = reg_reply.search(out).group(1).split() + reply_match = reg_reply.search(out) + self.assertTrue(reply_match, msg="COMPREPLY in output %s" % out) + + compl_opts = reply_match.group(1).split() basic_opts = ['--debug', '--enable-debug', '--disable-debug', '-d', '--help', '-h', '-H', '--shorthelp', '--configfiles', '--info', @@ -905,4 +908,3 @@ def test_option_as_value(self): # but first error still wins self._match_testoption1_sysexit(['--store', '--foo', '--nosuchoptiondefinedfoobar'], "Value '--foo' starts with a '-'") - From 84877a5f34207c3d84515f84a8103e83ac9b2e7b Mon Sep 17 00:00:00 2001 From: stdweird Date: Tue, 30 Aug 2016 14:27:41 +0200 Subject: [PATCH 3/5] run: add unittest for the kill tasks code --- lib/vsc/utils/run.py | 8 +++- test/run.py | 85 ++++++++++++++++++++++++++++++++++--- test/runtests/run_nested.sh | 14 ++++++ 3 files changed, 100 insertions(+), 7 deletions(-) create mode 100755 test/runtests/run_nested.sh diff --git a/lib/vsc/utils/run.py b/lib/vsc/utils/run.py index fd8e1e99..ca20ff20 100644 --- a/lib/vsc/utils/run.py +++ b/lib/vsc/utils/run.py @@ -97,6 +97,7 @@ class Run(object): INIT_INPUT_CLOSE = True USE_SHELL = True SHELL = SHELL # set the shell via the module constant + KILL_PGID = False @classmethod def run(cls, cmd, **kwargs): @@ -375,13 +376,16 @@ def _run_return(self): """What to return""" return self._process_exitcode, self._process_output - def _killtasks(self, tasks=None, sig=signal.SIGKILL, kill_pgid=False): + def _killtasks(self, tasks=None, sig=signal.SIGKILL, kill_pgid=None): """ Kill all tasks @param: tasks list of processids @param: sig, signal to use to kill @param: kill_pgid, send kill to group """ + if kill_pgid is None: + kill_pgid = self.KILL_PGID + if tasks is None: self.log.error("killtasks no tasks passed") return @@ -655,7 +659,7 @@ def _make_popen_named_args(self, others=None): class RunTimeout(RunLoop, RunAsync): - """Question/Answer processing""" + """Run for maximum timeout seconds""" def __init__(self, cmd, **kwargs): self.timeout = float(kwargs.pop('timeout', None)) diff --git a/test/run.py b/test/run.py index 9e2dbc07..719e9b13 100644 --- a/test/run.py +++ b/test/run.py @@ -35,9 +35,10 @@ import sys import tempfile import time +import shutil from unittest import TestLoader, main -from vsc.utils.run import run_simple, run_asyncloop, run_timeout, RunQA +from vsc.utils.run import run_simple, run_asyncloop, run_timeout, RunQA, RunTimeout from vsc.utils.run import RUNRUN_TIMEOUT_OUTPUT, RUNRUN_TIMEOUT_EXITCODE, RUNRUN_QA_MAX_MISS_EXITCODE from vsc.install.testing import TestCase @@ -45,6 +46,7 @@ SCRIPTS_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'runtests') SCRIPT_SIMPLE = os.path.join(SCRIPTS_DIR, 'simple.py') SCRIPT_QA = os.path.join(SCRIPTS_DIR, 'qa.py') +SCRIPT_NESTED = os.path.join(SCRIPTS_DIR, 'run_nested.sh') class RunQAShort(RunQA): @@ -56,6 +58,14 @@ class RunQAShort(RunQA): class TestRun(TestCase): """Test for the run module.""" + def setUp(self): + super(TestCase, self).setUp() + self.tempdir = tempfile.mkdtemp() + + def tearDown(self): + super(TestCase, self).tearDown() + shutil.rmtree(self.tempdir) + def test_simple(self): ec, output = run_simple([sys.executable, SCRIPT_SIMPLE, 'shortsleep']) self.assertEqual(ec, 0) @@ -67,14 +77,79 @@ def test_simple_asyncloop(self): self.assertTrue('shortsleep' in output.lower()) def test_timeout(self): - start = time.time() timeout = 3 + # longsleep is 10sec + start = time.time() ec, output = run_timeout([sys.executable, SCRIPT_SIMPLE, 'longsleep'], timeout=timeout) stop = time.time() - self.assertEqual(ec, RUNRUN_TIMEOUT_EXITCODE) - self.assertTrue(RUNRUN_TIMEOUT_OUTPUT == output) - self.assertTrue(stop - start < timeout + 1) # give 1 sec margin + self.assertEqual(ec, RUNRUN_TIMEOUT_EXITCODE, msg='longsleep stopped due to timeout') + self.assertEqual(RUNRUN_TIMEOUT_OUTPUT, output, msg='longsleep expected output') + self.assertTrue(stop - start < timeout + 1, msg='longsleep timeout within margin') # give 1 sec margin + + # run_nested is also 10 seconds sleep + # 1st arg depth: 2 recursive starts + # 2nd arg file: output file: format 'depth pid' (only other processes are sleep) + + def check_pid(pid): + """ Check For the existence of a unix pid. """ + try: + os.kill(pid, 0) + except OSError: + return False + else: + return True + + default = RunTimeout.KILL_PGID + + def do_test(kill_pgid): + depth = 2 # this is the parent + res_fn = os.path.join(self.tempdir, 'nested_kill_pgid_%s' % kill_pgid) + start = time.time() + RunTimeout.KILL_PGID = kill_pgid + ec, output = run_timeout([SCRIPT_NESTED, str(depth), res_fn], timeout=timeout) + # reset it to default + RunTimeout.KILL_PGID = default + stop = time.time() + self.assertEqual(ec, RUNRUN_TIMEOUT_EXITCODE, msg='run_nested kill_pgid %s stopped due to timeout' % kill_pgid) + self.assertTrue(stop - start < timeout + 1, msg='run_nested kill_pgid %s timeout within margin' % kill_pgid) # give 1 sec margin + # make it's not too fast + time.sleep(1) + # there's now 5 seconds to complete the remainder + pids = range(depth+1) + # normally this is ordered output, but you never know + for line in open(res_fn).readlines(): + dep, pid, _ = line.strip().split(" ") # 3rd is PPID + pids[int(dep)] = int(pid) + + # pids[0] should be killed + self.assertFalse(check_pid(pids[depth]), "main depth=%s pid (pids %s) is killed by timeout" % (depth, pids,)) + + if kill_pgid: + # others should be killed as well + test_fn = self.assertFalse + msg = "" + else: + # others should be running + test_fn = self.assertTrue + msg = " not" + + for dep, pid in enumerate(pids[:depth]): + test_fn(check_pid(pid), "depth=%s pid (pids %s) is%s killed kill_pgid %s" % (dep, pids, msg, kill_pgid)) + + # clean them all + for pid in pids: + try: + os.kill(pid, 0) # test first + os.kill(pid, 9) + except OSError: + pass + + do_test(False) + # TODO: find a way to change the pid group before starting this test + # now it kills the test process too ;) + # It's ok not to test, as it is not the default, and it's not easy to change it + #do_test(True) def test_qa_simple(self): """Simple testing""" diff --git a/test/runtests/run_nested.sh b/test/runtests/run_nested.sh new file mode 100755 index 00000000..553acf84 --- /dev/null +++ b/test/runtests/run_nested.sh @@ -0,0 +1,14 @@ +#!/bin/bash + +depth=${1:-1} +path=${2:-/dev/null} + +mysleep=10 + +echo "$depth $$ $PPID" >> $path + +if [ $depth -ne 0 ]; then + $0 $(($depth -1)) $path & +fi + +sleep $mysleep From 4dfe1f1abe8bfe0fcb394945a60a97ab45839d7f Mon Sep 17 00:00:00 2001 From: stdweird Date: Tue, 30 Aug 2016 14:41:59 +0200 Subject: [PATCH 4/5] generaloption: deprecated SimpleOption descr argument --- lib/vsc/utils/generaloption.py | 11 ++++++++--- test/run.py | 6 +++--- test/runtests/run_nested.sh | 2 +- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/vsc/utils/generaloption.py b/lib/vsc/utils/generaloption.py index 9ce247c2..82a1b6d1 100644 --- a/lib/vsc/utils/generaloption.py +++ b/lib/vsc/utils/generaloption.py @@ -1706,7 +1706,7 @@ class SimpleOptionParser(ExtOptionParser): class SimpleOption(GeneralOption): PARSER = SimpleOptionParser - def __init__(self, go_dict=None, short_groupdescr=None, long_groupdescr=None, config_files=None): + def __init__(self, go_dict=None, descr=None, short_groupdescr=None, long_groupdescr=None, config_files=None): """Initialisation @param go_dict : General Option option dict @param short_descr : short description of main options @@ -1735,13 +1735,18 @@ def __init__(self, go_dict=None, short_groupdescr=None, long_groupdescr=None, co super(SimpleOption, self).__init__(**kwargs) + if descr is not None: + # TODO: as there is no easy/clean way to access the version of the vsc-base package, + # this is equivalent to a warning + self.log.deprecated('SimpleOption descr argument', '2.5.0', '3.0.0') + def main_options(self): if self.go_dict is not None: prefix = None self.add_group_parser(self.go_dict, self.descr, prefix=prefix) -def simple_option(go_dict=None, short_groupdescr=None, long_groupdescr=None, config_files=None): +def simple_option(go_dict=None, descr=None, short_groupdescr=None, long_groupdescr=None, config_files=None): """A function that returns a single level GeneralOption option parser @param go_dict : General Option option dict @@ -1755,4 +1760,4 @@ def simple_option(go_dict=None, short_groupdescr=None, long_groupdescr=None, con the generated help will include the docstring """ - return SimpleOption(go_dict=go_dict, short_groupdescr=short_groupdescr, long_groupdescr=long_groupdescr, config_files=config_files) + return SimpleOption(go_dict=go_dict, descr=descr, short_groupdescr=short_groupdescr, long_groupdescr=long_groupdescr, config_files=config_files) diff --git a/test/run.py b/test/run.py index 719e9b13..0780ed80 100644 --- a/test/run.py +++ b/test/run.py @@ -87,7 +87,7 @@ def test_timeout(self): self.assertEqual(RUNRUN_TIMEOUT_OUTPUT, output, msg='longsleep expected output') self.assertTrue(stop - start < timeout + 1, msg='longsleep timeout within margin') # give 1 sec margin - # run_nested is also 10 seconds sleep + # run_nested is 15 seconds sleep # 1st arg depth: 2 recursive starts # 2nd arg file: output file: format 'depth pid' (only other processes are sleep) @@ -114,8 +114,8 @@ def do_test(kill_pgid): self.assertEqual(ec, RUNRUN_TIMEOUT_EXITCODE, msg='run_nested kill_pgid %s stopped due to timeout' % kill_pgid) self.assertTrue(stop - start < timeout + 1, msg='run_nested kill_pgid %s timeout within margin' % kill_pgid) # give 1 sec margin # make it's not too fast - time.sleep(1) - # there's now 5 seconds to complete the remainder + time.sleep(5) + # there's now 6 seconds to complete the remainder pids = range(depth+1) # normally this is ordered output, but you never know for line in open(res_fn).readlines(): diff --git a/test/runtests/run_nested.sh b/test/runtests/run_nested.sh index 553acf84..7c700de0 100755 --- a/test/runtests/run_nested.sh +++ b/test/runtests/run_nested.sh @@ -3,7 +3,7 @@ depth=${1:-1} path=${2:-/dev/null} -mysleep=10 +mysleep=15 echo "$depth $$ $PPID" >> $path From 07975d3a24cdba386de60431bbcde3e8020ec4cc Mon Sep 17 00:00:00 2001 From: stdweird Date: Tue, 30 Aug 2016 14:54:17 +0200 Subject: [PATCH 5/5] affinity: remove unused internal tobin function --- lib/vsc/utils/affinity.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/lib/vsc/utils/affinity.py b/lib/vsc/utils/affinity.py index 7872af53..d64f9ce6 100644 --- a/lib/vsc/utils/affinity.py +++ b/lib/vsc/utils/affinity.py @@ -234,18 +234,6 @@ def sched_getcpu(): """Get currently used cpu""" return _libc.sched_getcpu() -#Utility function -# tobin not used anymore -def tobin(s): - """Convert integer to binary format""" - # bin() missing in 2.4 - # eg: self.cpus.extend([int(x) for x in tobin(bitmask).zfill(NCPUBITS)[::-1]]) - if s <= 1: - return str(s) - else: - return tobin(s >> 1) + str(s & 1) - - #/* Return the highest priority of any process specified by WHICH and WHO # (see above); if WHO is zero, the current process, process group, or user # (as specified by WHO) is used. A lower priority number means higher