From a3a97a97211f259cb46322864f6db46c95938581 Mon Sep 17 00:00:00 2001 From: Maurice Makaay Date: Thu, 12 Dec 2019 13:48:17 +0100 Subject: [PATCH] Made pylint happy. --- pgbouncemgr/drop_privileges.py | 2 +- pgbouncemgr/manager.py | 2 +- pgbouncemgr/node_config.py | 2 +- pgbouncemgr/node_poller.py | 2 +- pgbouncemgr/postgres.py | 65 ++++++++++++++++++++++++++-------- pgbouncemgr/state.py | 8 ++--- pgbouncemgr/state_store.py | 5 ++- 7 files changed, 60 insertions(+), 26 deletions(-) diff --git a/pgbouncemgr/drop_privileges.py b/pgbouncemgr/drop_privileges.py index 02eb523..1fad4ac 100644 --- a/pgbouncemgr/drop_privileges.py +++ b/pgbouncemgr/drop_privileges.py @@ -3,7 +3,7 @@ """Functionality to drop privileges to those of a specified user/group id.""" -from os import setuid, setgid, setuid, setgroups, getuid, geteuid, getgroups +from os import setuid, setgid, setgroups, getuid, geteuid, getgroups from pwd import getpwnam, getpwuid from grp import getgrnam, getgrgid from pgbouncemgr.logger import format_ex diff --git a/pgbouncemgr/manager.py b/pgbouncemgr/manager.py index e29e57c..7569bad 100644 --- a/pgbouncemgr/manager.py +++ b/pgbouncemgr/manager.py @@ -6,7 +6,7 @@ from time import sleep from argparse import ArgumentParser -from pgbouncemgr.logger import Logger, ConsoleLog, SyslogLog, format_ex +from pgbouncemgr.logger import Logger, ConsoleLog, SyslogLog from pgbouncemgr.config import Config from pgbouncemgr.state import State from pgbouncemgr.drop_privileges import drop_privileges diff --git a/pgbouncemgr/node_config.py b/pgbouncemgr/node_config.py index bf8aa02..0a0d37e 100644 --- a/pgbouncemgr/node_config.py +++ b/pgbouncemgr/node_config.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -# no-pylint: disable=missing-docstring,too-many-instance-attributes +# no-pylint: disable=missing-docstring,too-many-instance-attributes,too-few-public-methods import os from pgbouncemgr.config import InvalidConfigValue diff --git a/pgbouncemgr/node_poller.py b/pgbouncemgr/node_poller.py index e8dacfa..a86811e 100644 --- a/pgbouncemgr/node_poller.py +++ b/pgbouncemgr/node_poller.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -# no-pylint: disable=missing-docstring +# no-pylint: disable=missing-docstring,too-few-public-methods class NodePoller(): """The NodePoller is used to poll all the nodes that are available diff --git a/pgbouncemgr/postgres.py b/pgbouncemgr/postgres.py index 0c0c58c..f42e95a 100644 --- a/pgbouncemgr/postgres.py +++ b/pgbouncemgr/postgres.py @@ -4,9 +4,9 @@ """This module provides various classes that uare used for connecting to a PostgreSQL or pgbouncer server.""" -import psycopg2 import multiprocessing -from psycopg2.extras imoprt LogicalReplicationConnection +import psycopg2 +from psycopg2.extras import LogicalReplicationConnection from pgbouncemgr.logger import format_ex @@ -39,6 +39,10 @@ CONNECTED = 'CONNECTED' REUSED = 'REUSED' RECONNECTED = 'RECONNECTED' +# The properties that can be used in a configuration object to +# define connection parameters for psycopg2. +CONNECTION_PARAMS = ['host', 'port', 'connect_timeout', 'user', 'password'] + class PgConnection(): """Implements a connection to a PostgreSQL server.""" def __init__(self, config): @@ -47,10 +51,17 @@ class PgConnection(): self.conn = None def _create_conn_params(self, config): - """Use only connection parameters that don't have value None.""" - return dict( - (k, v) for k, v in config.items() - if v is not None) + """Extract all connection parameters from the provided configuration, + that don't have value of None.""" + conn_params = {} + for key in CONNECTION_PARAMS: + if not hasattr(config, key): + continue + value = getattr(config, key) + if value is None: + continue + conn_params[key] = value + return conn_params def connect(self): """Connect to the database server. When a connection exists, @@ -142,6 +153,31 @@ class PgReplicationConnection(PgConnection): return status +# The query that is used to check if pgbouncer is connected to the expected +# backend server. We don't really have a unique identifier for the node to +# work with there, but we can request the server host + port to see if they +# match those of the expected node. Additionally, a check is done to see if +# the node is running as a primary node. +# +# It would be really nice if it were possible to retrieve the system_id +# and timeline_id for the backend server, but that is only possible when +# using a replication connection. Pgbouncer (through which we are connected +# when running this query) does not support pooled replication connections. +VERIFY_QUERY = """ + SELECT CASE + WHEN ( + inet_server_addr() IS DISTINCT FROM %(host)s OR + inet_server_port() IS DISTINCT FROM %(port)s) + THEN format( + 'connected to wrong server: %%s:%%s (expected %%s:%%s)', + inet_server_addr(), inet_server_port(), %(host)s, %(port)s) + WHEN pg_is_in_recovery() + THEN format( + 'server %%s:%%s is not running as primary', + %(host)s, %(port)s) + ELSE NULL + END +""" class PgConnectionViaPgbouncer(PgConnection): """This PostgreSQL connection class is used to setup a connection @@ -158,8 +194,8 @@ class PgConnectionViaPgbouncer(PgConnection): # Secondly, override parameters to redirect the connection to # the pgbouncer instance. - self.conn_params["host"] = pgbouncer_config["host"] - self.conn_params["port"] = pgbouncer_config["port"] + self.conn_params["host"] = pgbouncer_config["host"] + self.conn_params["port"] = pgbouncer_config["port"] # Note that we don't setup a replication connection here. This is # unfortunately not possible, because pgbouncer does not support @@ -178,7 +214,7 @@ class PgConnectionViaPgbouncer(PgConnection): # then stall for quite a while. For swift operation of backend # switching, we therefore use the timeout setup here, to reconfigure # the system more quickly. - # + # # Note that in most situations this shouldn't be an issue, since we # will always reload pgbouncer after a configuration change and the # connection from below will, because of that, work as intended @@ -207,9 +243,9 @@ class PgConnectionViaPgbouncer(PgConnection): self.disconnect() return report_func(False, exception) - # When the verify query did not return an error message, then we - # are in the green. - return report_func(True, None) + # When the verify query did not return an error message, then we + # are in the green. + return report_func(True, None) parent_conn, child_conn = multiprocessing.Pipe() def report_func(true_or_false, exception): @@ -238,7 +274,7 @@ class PgBouncerConsoleConnection(PgConnection): # For the console connection, the database name "pgbouncer" # must be used. - self.conn_params["dbname"] = "pgbouncer" + self.conn_params["dbname"] = "pgbouncer" # The default ping query does not work when connected to the # pgbouncer console. Here's a simple replacement for it. @@ -251,7 +287,7 @@ class PgBouncerConsoleConnection(PgConnection): transaction. Transactions are not supported by the pgbouncer console.""" result = super().connect() - self.conn.autocommit = True + self.conn.autocommit = True return result def reload(self): @@ -268,4 +304,3 @@ class PgBouncerConsoleConnection(PgConnection): except Exception as exception: raise ReloadingPgbouncerFailed( "An exception occurred: %s" % format_ex(exception)) - diff --git a/pgbouncemgr/state.py b/pgbouncemgr/state.py index 777c855..e5d5e78 100644 --- a/pgbouncemgr/state.py +++ b/pgbouncemgr/state.py @@ -113,8 +113,8 @@ class State(): state = State(logger) for node_id, settings in config.nodes.items(): node_config = NodeConfig(node_id) - for k, v in settings.items(): - setattr(node_config, k, v) + for key, value in settings.items(): + setattr(node_config, key, value) state.add_node(node_config) return state @@ -241,9 +241,9 @@ class State(): "node id: node config changed? (leader_node_id=%s)" % leader_node_id) raise UnknownLeaderNodeId(leader_node_id) - if self._leader_node_id == leader_node_id: + if self._leader_node_id == node.node_id: return - self._leader_node_id = leader_node_id + self._leader_node_id = node.node_id self.modified = True @property diff --git a/pgbouncemgr/state_store.py b/pgbouncemgr/state_store.py index b4db532..d491712 100644 --- a/pgbouncemgr/state_store.py +++ b/pgbouncemgr/state_store.py @@ -2,7 +2,6 @@ # no-pylint: disable=missing-docstring,broad-except,too-many-instance-attributes import json -from datetime import datetime from os import rename from os.path import isfile from pgbouncemgr.logger import format_ex @@ -71,5 +70,5 @@ class StateStore(): rename(swap_path, self.path) except Exception as exception: raise StateStoreException( - "Storing state to file (%s) failed: %s" % ( - self.path, format_ex(exception))) + "Storing state to file (%s) failed: %s" % + (self.path, format_ex(exception)))