From d64ca0f4636cee4fd25aea562026618a4bb883ac Mon Sep 17 00:00:00 2001 From: PHeanEX Date: Wed, 22 Mar 2017 10:52:18 +0100 Subject: [PATCH] Refactor master (#81) Stylistic cleanup. @pheanex --- pg_view/__init__.py | 20 +++++------ pg_view/collectors/base_collector.py | 42 ++++++++++++----------- pg_view/collectors/host_collector.py | 10 +++--- pg_view/collectors/memory_collector.py | 7 ++-- pg_view/collectors/partition_collector.py | 23 ++++++++----- pg_view/collectors/pg_collector.py | 8 ++--- pg_view/models/db_client.py | 13 +++---- pg_view/models/outputs.py | 29 +++++++++------- pg_view/models/parsers.py | 4 +-- requirements.txt | 1 + setup.py | 3 +- tests/test_dummy.py | 1 + 12 files changed, 83 insertions(+), 78 deletions(-) diff --git a/pg_view/__init__.py b/pg_view/__init__.py index 07422ea..c971d0d 100644 --- a/pg_view/__init__.py +++ b/pg_view/__init__.py @@ -114,7 +114,6 @@ def poll_keys(screen, output): def do_loop(screen, groups, output_method, collectors, consumer): """ Display output (or pass it through to ncurses) """ - output = None if output_method == OUTPUT_METHOD.curses: if screen is None: logger.error('No parent screen is passed to the curses application') @@ -131,17 +130,15 @@ def do_loop(screen, groups, output_method, collectors, consumer): # process input: consumer.consume() for st in collectors: - if output_method == OUTPUT_METHOD.curses: - if not poll_keys(screen, output): - # bail out immediately - return + if output_method == OUTPUT_METHOD.curses and not poll_keys(screen, output): + # bail out immediately + return st.set_units_display(flags.display_units) st.set_ignore_autohide(not flags.autohide_fields) st.set_notrim(flags.notrim) process_single_collector(st) - if output_method == OUTPUT_METHOD.curses: - if not poll_keys(screen, output): - return + if output_method == OUTPUT_METHOD.curses and not poll_keys(screen, output): + return if output_method == OUTPUT_METHOD.curses: process_groups(groups) @@ -182,7 +179,7 @@ def main(): if output_method == OUTPUT_METHOD.curses and not curses_available: print('Curses output is selected, but curses are unavailable, falling back to console output') - output_method == OUTPUT_METHOD.console + output_method = OUTPUT_METHOD.console # set basic logging setup_logger(options) @@ -192,7 +189,7 @@ def main(): clusters = [] # now try to read the configuration file - config = (read_configuration(options.config_file) if options.config_file else None) + config = read_configuration(options.config_file) if options.config_file else None if config: for instance in config: if user_dbname and instance != user_dbname: @@ -207,7 +204,6 @@ def main(): logger.error('failed to acquire details about ' + 'the database cluster {0}, the server will be skipped'.format(instance)) elif options.host: - port = options.port or "5432" # try to connet to the database specified by command-line options conn = build_connection(options.host, options.port, options.username, options.dbname) instance = options.instance or "default" @@ -286,7 +282,7 @@ def main(): def setup_logger(options): - logger.setLevel((logging.INFO if options.verbose else logging.ERROR)) + logger.setLevel(logging.INFO if options.verbose else logging.ERROR) if options.log_file: LOG_FILE_NAME = options.log_file # truncate the former logs diff --git a/pg_view/collectors/base_collector.py b/pg_view/collectors/base_collector.py index ad1f3b3..d056ec6 100644 --- a/pg_view/collectors/base_collector.py +++ b/pg_view/collectors/base_collector.py @@ -101,23 +101,23 @@ def validate_list_out(l): @staticmethod def ticks_to_seconds(tick_value_str): - return (float(tick_value_str) / StatCollector.USER_HZ if tick_value_str is not None else None) + return float(tick_value_str) / StatCollector.USER_HZ if tick_value_str is not None else None @staticmethod def bytes_to_mbytes(bytes_val): - return (float(bytes_val) / 1048576 if bytes_val is not None else None) + return float(bytes_val) / 1048576 if bytes_val is not None else None @staticmethod def sectors_to_mbytes(sectors): - return (float(sectors) / 2048 if sectors is not None else None) + return float(sectors) / 2048 if sectors is not None else None @staticmethod def kb_to_mbytes(kb): - return (float(kb) / 1024 if kb is not None else None) + return float(kb) / 1024 if kb is not None else None @staticmethod def time_diff_to_percent(timediff_val): - return (float(timediff_val) * 100 if timediff_val is not None else None) + return float(timediff_val) * 100 if timediff_val is not None else None @staticmethod def format_date_from_epoch(epoch_val): @@ -139,7 +139,7 @@ def kb_pretty_print_long(b): d = b / n if d: r.append(str(d) + l) - b = b % n + b %= n return ' '.join(r) @staticmethod @@ -147,7 +147,6 @@ def kb_pretty_print(b): """ Show memory size as a float value in the biggest measurement units """ r = [] - v = 0 for l, n in StatCollector.BYTE_MAP: if b > n: v = round(float(b) / n, 1) @@ -160,10 +159,10 @@ def kb_pretty_print(b): @staticmethod def time_interval_pretty_print(start_time, is_delta): - '''Returns a human readable string that shows a time between now and the timestamp passed as an argument. + """Returns a human readable string that shows a time between now and the timestamp passed as an argument. The passed argument can be a timestamp (returned by time.time() call) a datetime object or a timedelta object. In case it is a timedelta object, then it is formatted only - ''' + """ if isinstance(start_time, Number): if is_delta: @@ -222,7 +221,7 @@ def sectors_pretty_print(b): @staticmethod def int_lower_than_non_zero(row, col, val, bound): - return val > 0 and val < bound + return 0 < val < bound @staticmethod def time_field_to_seconds(val): @@ -391,7 +390,8 @@ def _produce_output_row(self, row): result[attname] = val return result - def _produce_output_value(self, row, col, method=OUTPUT_METHOD.console): + @staticmethod + def _produce_output_value(row, col, method=OUTPUT_METHOD.console): # get the input value if 'in' in col: val = row.get(col['in'], None) @@ -414,7 +414,8 @@ def _produce_output_name(self, col): attname += ' ' + col['units'] return attname - def _calculate_output_status(self, row, col, val, method): + @staticmethod + def _calculate_output_status(row, col, val, method): """ Examine the current status indicators and produce the status value for the specific column of the given row """ @@ -555,7 +556,8 @@ def _transform_dict(self, l, custom_transformation_data=None): return result raise Exception('No data for the dict transformation supplied') - def _transform_string(self, d): + @staticmethod + def _transform_string(d): raise Exception('transformation of input type string is not implemented') def _output_template_for_console(self): @@ -652,7 +654,8 @@ def _calculate_statuses_for_row(self, row, method): statuses.append(self._calculate_output_status(row, col, row[num], method)) return statuses - def _calculate_column_types(self, rows): + @staticmethod + def _calculate_column_types(rows): result = {} if len(rows) > 0: colnames = rows[0].keys() @@ -728,12 +731,11 @@ def ncurses_output(self, rows, before_string=None, after_string=None): types_row = self._calculate_column_types(values_rows) - result = {} - result['rows'] = result_rows - result['statuses'] = status_rows - result['hide'] = self._get_columns_to_hide(result_rows, status_rows) - result['highlights'] = dict(zip(result_header, self._get_highlights())) - result['types'] = types_row + result = {'rows': result_rows, + 'statuses': status_rows, + 'hide': self._get_columns_to_hide(result_rows, status_rows), + 'highlights': dict(zip(result_header, self._get_highlights())), + 'types': types_row} for x in StatCollector.NCURSES_CUSTOM_OUTPUT_FIELDS: result[x] = self.ncurses_custom_fields.get(x, None) for k in StatCollector.NCURSES_DEFAULTS.keys(): diff --git a/pg_view/collectors/host_collector.py b/pg_view/collectors/host_collector.py index b3c4d5d..60f238f 100644 --- a/pg_view/collectors/host_collector.py +++ b/pg_view/collectors/host_collector.py @@ -92,7 +92,8 @@ def _load_avg_state(self, row, col): state[no] = COLSTATUS.cs_ok return state - def _concat_load_avg(self, colname, row, optional): + @staticmethod + def _concat_load_avg(colname, row, optional): """ concat all load averages into a single string """ if len(row) >= 3: @@ -100,7 +101,8 @@ def _concat_load_avg(self, colname, row, optional): else: return '' - def _load_avg_status(self, row, col, val, bound): + @staticmethod + def _load_avg_status(row, col, val, bound): if val is not None: loads = str(val).split() if len(loads) != 3: @@ -118,10 +120,10 @@ def _read_cpus(): cpus = cpu_count() except: logger.error('multiprocessing does not support cpu_count') - pass return {'cores': cpus} - def _construct_sysname(self, attname, row, optional): + @staticmethod + def _construct_sysname(attname, row, optional): if len(row) < 3: return None return '{0} {1}'.format(row[0], row[2]) diff --git a/pg_view/collectors/memory_collector.py b/pg_view/collectors/memory_collector.py index 8e32270..582faf4 100644 --- a/pg_view/collectors/memory_collector.py +++ b/pg_view/collectors/memory_collector.py @@ -117,7 +117,8 @@ def refresh(self): raw_result = self._transform_input(memdata) self._do_refresh([raw_result]) - def _read_memory_data(self): + @staticmethod + def _read_memory_data(): """ Read relevant data from /proc/meminfo. We are interesed in the following fields: MemTotal, MemFree, Buffers, Cached, Dirty, CommitLimit, Committed_AS """ @@ -131,9 +132,9 @@ def _read_memory_data(self): # if we have units of measurement different from kB - transform the result if len(vals) == 3 and vals[2] in ('mB', 'gB'): if vals[2] == 'mB': - val = val + '0' * 3 + val += '0' * 3 if vals[2] == 'gB': - val = val + '0' * 6 + val += '0' * 6 if len(str(name)) > 1: result[str(name)[:-1]] = val else: diff --git a/pg_view/collectors/partition_collector.py b/pg_view/collectors/partition_collector.py index 6af4996..c79e929 100644 --- a/pg_view/collectors/partition_collector.py +++ b/pg_view/collectors/partition_collector.py @@ -126,15 +126,16 @@ def __init__(self, dbname, dbversion, work_directory, consumer): }, {'out': 'path', 'pos': 10}, ] - self.ncurses_custom_fields = {'header': True} - self.ncurses_custom_fields['prefix'] = None + self.ncurses_custom_fields = {'header': True, + 'prefix': None} self.postinit() def ident(self): return '{0} ({1}/{2})'.format(super(PartitionStatCollector, self).ident(), self.dbname, self.dbver) - def _dereference_dev_name(self, devname): - return (devname.replace('/dev/', '') if devname else None) + @staticmethod + def _dereference_dev_name(devname): + return devname.replace('/dev/', '') if devname else None def refresh(self): result = {} @@ -162,14 +163,18 @@ def refresh(self): self._do_refresh([result[PartitionStatCollector.DATA_NAME], result[PartitionStatCollector.XLOG_NAME]]) - def calculate_time_until_full(self, colname, prev, cur): + @staticmethod + def calculate_time_until_full(colname, prev, cur): # both should be expressed in common units, guaranteed by BLOCK_SIZE - if cur.get('path_size', 0) > 0 and prev.get('path_size', 0) > 0 and cur.get('space_left', 0) > 0: - if cur['path_size'] < prev['path_size']: - return cur['space_left'] / (prev['path_size'] - cur['path_size']) + if (cur.get('path_size', 0) > 0 and + prev.get('path_size', 0) > 0 and + cur.get('space_left', 0) > 0 and + cur['path_size'] < prev['path_size']): + return cur['space_left'] / (prev['path_size'] - cur['path_size']) return None - def get_io_data(self, pnames): + @staticmethod + def get_io_data(pnames): """ Retrieve raw data from /proc/diskstat (transformations are perfromed via io_list_transformation)""" result = {} found = 0 # stop if we found records for all partitions diff --git a/pg_view/collectors/pg_collector.py b/pg_view/collectors/pg_collector.py index 0b29ae1..70941d4 100644 --- a/pg_view/collectors/pg_collector.py +++ b/pg_view/collectors/pg_collector.py @@ -208,8 +208,8 @@ def __init__(self, pgcon, reconnect, pid, dbname, dbver, always_track_pids): }, ] - self.ncurses_custom_fields = {'header': True} - self.ncurses_custom_fields['prefix'] = None + self.ncurses_custom_fields = {'header': True, + 'prefix': None} self.postinit() @@ -275,7 +275,7 @@ def _get_psinfo(cmdline): pstype = 'backend' if pstype == 'autovacuum worker': pstype = 'autovacuum' - return (pstype, action) + return pstype, action @staticmethod def _is_auxiliary_process(pstype): @@ -369,7 +369,7 @@ def _read_proc(self, pid, is_backend, is_active): # Assume we managed to read the row if we can get its PID for cat in 'stat', 'io': - result.update(self._transform_input(raw_result.get(cat, ({} if cat == 'io' else [])))) + result.update(self._transform_input(raw_result.get(cat, {} if cat == 'io' else []))) # generated columns result['cmdline'] = raw_result.get('cmd', None) if not is_backend: diff --git a/pg_view/models/db_client.py b/pg_view/models/db_client.py index c795ef5..a779e16 100644 --- a/pg_view/models/db_client.py +++ b/pg_view/models/db_client.py @@ -43,7 +43,7 @@ def build_connection(host, port, user, database): def pick_connection_arguments(conn_args, username, dbname): """ go through all decected connections, picking the first one that actually works """ result = {} - for conn_type in ('unix', 'tcp', 'tcp6'): + for conn_type in 'unix', 'tcp', 'tcp6': if len(result) > 0: break for arg in conn_args.get(conn_type, []): @@ -65,7 +65,6 @@ def can_connect_with_connection_arguments(host, port, username, dbname): def detect_with_proc_net(pid): - result = None inodes = fetch_socket_inodes_for_process(pid) parser = ProcNetParser() result = parser.match_socket_inodes(inodes) @@ -81,7 +80,6 @@ def detect_db_connection_arguments(work_directory, pid, version, username, dbnam We do this by first extracting useful information from postmaster.pid, next reading the postgresql.conf if necessary and, at last, """ - result = {} conn_args = detect_with_proc_net(pid) if not conn_args: # if we failed to detect the arguments via the /proc/net/ readings, @@ -104,7 +102,6 @@ def establish_user_defined_connection(instance, conn, clusters): """ connect the database and get all necessary options like pid and work_directory we use port, host and socket_directory, prefering socket over TCP connections """ - pgcon = None # establish a new connection try: pgcon = psycopg2.connect(**conn) @@ -147,7 +144,7 @@ def make_cluster_desc(name, version, workdir, pid, pgcon, conn): def reconnect(): pgcon = psycopg2.connect(**conn) pid = read_postmaster_pid(workdir, name) - return (pgcon, pid) + return pgcon, pid return { 'name': name, @@ -170,7 +167,6 @@ def get_postmasters_directories(): # make sure the particular pid is accessible to us if not os.access(f, os.R_OK): continue - stat_fields = [] try: with open(f, 'rU') as fp: stat_fields = fp.read().strip().split() @@ -280,7 +276,6 @@ def detect_with_postmaster_pid(work_directory, version): if version is None or version == 9.0: return None PID_FILE = '{0}/postmaster.pid'.format(work_directory) - lines = [] # try to access the socket directory if not os.access(work_directory, os.R_OK | os.X_OK): @@ -312,12 +307,12 @@ def detect_with_postmaster_pid(work_directory, version): def get_dbname_from_path(db_path): - ''' + """ >>> get_dbname_from_path('foo') 'foo' >>> get_dbname_from_path('/pgsql_bar/9.4/data') 'bar' - ''' + """ m = re.search(r'/pgsql_(.*?)(/\d+.\d+)?/data/?', db_path) if m: dbname = m.group(1) diff --git a/pg_view/models/outputs.py b/pg_view/models/outputs.py index d5ef306..dbcd08b 100644 --- a/pg_view/models/outputs.py +++ b/pg_view/models/outputs.py @@ -31,10 +31,12 @@ class CommonOutput(object): def __init__(self): super(CommonOutput, self) - def display(self, data): + @staticmethod + def display(data): print(data) - def refresh(self): + @staticmethod + def refresh(): os.system('clear') @@ -124,7 +126,6 @@ def refresh(self): def screen_erase(self): self.screen.erase() self.screen.refresh() - pass def update_screen_metrics(self): self.screen_y, self.screen_x = self.screen.getmaxyx() @@ -146,9 +147,9 @@ def print_text(self, starty, startx, text, attr=None, trim_middle=False): def show_help_bar_item(self, key, description, selected, x): x = self.print_text(self.screen_y - 1, x, '{0}:'.format(key), - ((self.COLOR_MENU_SELECTED if selected else self.COLOR_MENU)) | curses.A_BOLD) + (self.COLOR_MENU_SELECTED if selected else self.COLOR_MENU) | curses.A_BOLD) x = self.print_text(self.screen_y - 1, x, '{0} '.format(description), - (self.COLOR_MENU_SELECTED if selected else self.COLOR_MENU)) + self.COLOR_MENU_SELECTED if selected else self.COLOR_MENU) return x def show_help_bar(self): @@ -238,7 +239,7 @@ def color_value(self, val, xcol, status_map, highlight, result): 'width': len(val), 'color': color, }) - xcol += (len(val) + 1) + xcol += len(val) + 1 else: # XXX: we are calculating the world boundaries again here # (first one in calculate_output_status) and using a different method to do so. @@ -263,7 +264,7 @@ def color_value(self, val, xcol, status_map, highlight, result): 'color': color, }) last_position = xcol + word.end(0) - xcol += (last_position + 1) + xcol += last_position + 1 return xcol def help(self): @@ -350,7 +351,7 @@ def show_collector_data(self, collector, clock=False): if layout[field].get('truncate', False): # XXX: why do we truncate even when truncate for the column is set to False? - header, text = self.truncate_column_value(row[field], w, (w > self.MIN_ELLIPSIS_FIELD_LENGTH)) + header, text = self.truncate_column_value(row[field], w, w > self.MIN_ELLIPSIS_FIELD_LENGTH) else: header, text = row[field].header, row[field].value text = self._align_field(text, header, w, column_alignment, types.get(field, COLTYPES.ct_string)) @@ -361,7 +362,8 @@ def show_collector_data(self, collector, clock=False): f['color']) self.next_y += 1 - def truncate_column_value(self, cv, maxlen, ellipsis=True): + @staticmethod + def truncate_column_value(cv, maxlen, ellipsis=True): """ make sure that a pair of header and value fits into the allocated field length """ value = cv.value header = cv.header @@ -376,14 +378,14 @@ def truncate_column_value(self, cv, maxlen, ellipsis=True): header = header[:maxlen] + (' ' if maxlen == h_len + 1 else '') + ('...' if ellipsis else '') value = '' else: - value = value[:(maxlen - h_len - 1)] + ('...' if ellipsis else '') + value = value[:maxlen - h_len - 1] + ('...' if ellipsis else '') elif header_position == COLHEADER.ch_append: if v_len + 1 >= maxlen: # prepend the value, consider if we have to truncate it and omit the header altogether value = value[:maxlen] + (' ' if maxlen == v_len + 1 else '') + ('...' if ellipsis else '') header = '' else: - header = header[:(maxlen - v_len - 1)] + ('...' if ellipsis else '') + header = header[:maxlen - v_len - 1] + ('...' if ellipsis else '') else: # header is set to '' by the collector value = value[:maxlen] + ('...' if ellipsis else '') @@ -400,7 +402,7 @@ def display_prefix(self, collector, header): elif prefix_len >= self.screen_x / 5 and not prefix_newline: return 0 - color = (self.COLOR_INVERSE_HIGHLIGHT if prefix_newline else self.COLOR_NORMAL) + color = self.COLOR_INVERSE_HIGHLIGHT if prefix_newline else self.COLOR_NORMAL self.screen.addnstr(self.next_y, 1, str(prefix), len(str(prefix)), color) if prefix_newline: @@ -458,7 +460,8 @@ def _get_fields_sorted_by_position(self, collector): sorted_by_pos = sorted(((x, pos[x]) for x in pos if pos[x] != -1), key=itemgetter(1)) return [f[0] for f in sorted_by_pos] - def _invisible_fields_status(self, layout, statuses): + @staticmethod + def _invisible_fields_status(layout, statuses): highest_status = COLSTATUS.cs_ok invisible = [col for col in statuses if col not in layout] for col in invisible: diff --git a/pg_view/models/parsers.py b/pg_view/models/parsers.py index 917f928..38abe62 100644 --- a/pg_view/models/parsers.py +++ b/pg_view/models/parsers.py @@ -5,7 +5,7 @@ from pg_view.loggers import logger -class ProcNetParser(): +class ProcNetParser: """ Parse /proc/net/{tcp,tcp6,unix} and return the list of address:port pairs given the set of socket descriptors belonging to the object. The result is grouped by the socket type in a dictionary. @@ -22,7 +22,7 @@ def reinit(self): self.unix_socket_header_len = 0 # initialize the sockets hash with the contents of unix # and tcp sockets. tcp IPv6 is also read if it's present - for fname in (ProcNetParser.NET_UNIX_FILENAME, ProcNetParser.NET_TCP_FILENAME): + for fname in ProcNetParser.NET_UNIX_FILENAME, ProcNetParser.NET_TCP_FILENAME: self.read_socket_file(fname) if os.access(ProcNetParser.NET_TCP6_FILENAME, os.R_OK): self.read_socket_file(ProcNetParser.NET_TCP6_FILENAME) diff --git a/requirements.txt b/requirements.txt index 658130b..fbd795d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1 +1,2 @@ +pytest psycopg2 diff --git a/setup.py b/setup.py index d038597..9d05d3d 100644 --- a/setup.py +++ b/setup.py @@ -98,8 +98,7 @@ def read(fname): def setup_package(): # Assemble additional setup commands - cmdclass = {} - cmdclass['test'] = PyTest + cmdclass = {'test': PyTest} install_reqs = get_install_requirements('requirements.txt') diff --git a/tests/test_dummy.py b/tests/test_dummy.py index 81bd2ee..42bd47c 100644 --- a/tests/test_dummy.py +++ b/tests/test_dummy.py @@ -1,5 +1,6 @@ import pg_view + def test_dummy(): pass