From dee07b8b76f51ea9745f16f0893f4ce57d6d5925 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Thu, 24 Dec 2015 10:15:39 +0100 Subject: [PATCH 1/6] Add -H and -P options to specify the host/port. --- pg_view.py | 64 +++++++++++++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/pg_view.py b/pg_view.py index adf0580..f52adf9 100644 --- a/pg_view.py +++ b/pg_view.py @@ -126,6 +126,9 @@ def parse_args(): action='store', dest='username') parser.add_option('-d', '--dbname', help='database name to connect to', action='store', dest='dbname') + parser.add_option('-H', '--host', help='database connection host (or a directory path for the unix socket connection)', + action='store', dest='host') + parser.add_option('-P', '--port', help='database port number', action='store', dest='port') options, args = parser.parse_args() return options, args @@ -2925,9 +2928,9 @@ def detect_db_port(socket_dir): return port -def detect_default_user_database(): - user = options.username or os.environ.get('PGUSER') or getpass.getuser() - database = options.dbname or os.environ.get('PGDATABASE') or user +def detect_default_user_database(user, database): + user = user or os.environ.get('PGUSER') or getpass.getuser() + database = database or os.environ.get('PGDATABASE') or user return (user, database) @@ -3028,7 +3031,7 @@ def pick_connection_arguments(conn_args): def can_connect_with_connection_arguments(host, port): """ check that we can connect given the specified arguments """ - user, database = detect_default_user_database() + user, database = detect_default_user_database(options.username, options.dbname) try: test_conn = psycopg2.connect('host={0} port={1} user={2} dbname={3}'.format(host, port, user, database)) test_conn.close() @@ -3073,30 +3076,22 @@ def detect_db_connection_arguments(work_directory, pid, version): return result -def establish_user_defined_connection(dbname, args, clusters): +def establish_user_defined_connection(instance, clusters, host, port, user, dbname): """ 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 + connstring = "host={0} port={1} user={2} dbname={3}".format(host, port, user, dbname) # sanity check - if not (args.get('port') or args.get('host')): - missing = ('port' if not args.get('port') else 'host') - logger.error('Not all required connection arguments ' + - '({0}) are specified for the database {1}, skipping it'.format(missing, dbname)) + if not (host and port and user and dbname): + logger.error("Connection string is incomplete: {0}".format(connstring)) return None - - port = args['port'] - host = args['host'] - user = args.get('user', 'postgres') - # the db is the actual database we connect to, while dbname is the name of the postgres cluster - db = args.get('dbname', 'postgres') # establish a new connection try: - pgcon = psycopg2.connect('host={0} port={1} user={2} dbname={3}'.format(host, port, user, db)) + pgcon = psycopg2.connect('{0}'.format(connstring)) except Exception as e: - logger.error('failed to establish connection to {0} on port {1} user {2} database {3}'.format(host, port, user, - dbname)) + logger.error('failed to establish connection to {0} via {1}'.format(instance, connstring)) logger.error('PostgreSQL exception: {0}'.format(e)) return None # get the database version from the pgcon properties @@ -3109,7 +3104,7 @@ def establish_user_defined_connection(dbname, args, clusters): cur.close() pgcon.commit() # now, when we have the work directory, acquire the pid of the postmaster. - pid = read_postmaster_pid(work_directory, dbname) + pid = read_postmaster_pid(work_directory, instance) if pid is None: logger.error('failed to read pid of the postmaster on {0}:{1}'.format(host, port)) return None @@ -3118,14 +3113,14 @@ def establish_user_defined_connection(dbname, args, clusters): # the same database (one for the unix_socket_directory and another for the host) pids = [opt['pid'] for opt in clusters if 'pid' in opt] if pid in pids: - duplicate_dbname = [opt['name'] for opt in clusters if 'pid' in opt and opt.get('pid', 0) == pid][0] + duplicate_instance = [opt['name'] for opt in clusters if 'pid' in opt and opt.get('pid', 0) == pid][0] logger.error('duplicate connection options detected ' + - 'for databases {0} and {1}, same pid {2}, skipping {0}'.format(dbname, duplicate_dbname, pid)) + 'for databases {0} and {1}, same pid {2}, skipping {0}'.format(instance, duplicate_instance, pid)) pgcon.close() return True # now we have all components to create the result result = { - 'name': dbname, + 'name': instance, 'ver': dbver, 'wd': work_directory, 'pid': pid, @@ -3287,15 +3282,26 @@ def main(): clusters = [] # now try to read the configuration file - config_data = (read_configuration(options.config_file) if options.config_file else None) - if config_data: - for dbname in config_data: - if user_dbname and dbname != user_dbname: + 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: continue # pass already aquired connections to make sure we only list unique clusters. - if not establish_user_defined_connection(dbname, config_data[dbname], clusters): + host = config[instance].get('host') + port = config[instance].get('port') + user, database = detect_default_user_database(config[instance].get('user'), config[instance].get('database')) + + if not establish_user_defined_connection(instance, clusters, host, port, user, database): logger.error('failed to acquire details about ' + - 'the database cluster {0}, the server will be skipped'.format(dbname)) + '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 + user, dbname = detect_default_user_database(options.username, options.dbname) + instance = options.instance or "default" + if not establish_user_defined_connection(instance, clusters, options.host, port, user, dbname): + logger.error("unable to continue with cluster {0}".format(instance)) else: # do autodetection postmasters = get_postmasters_directories() @@ -3315,7 +3321,7 @@ def main(): continue host = conndata['host'] port = conndata['port'] - user, database = detect_default_user_database() + user, database = detect_default_user_database(options.username, options.dbname) pgcon = psycopg2.connect('host={0} port={1} user={2} dbname={3}'.format(host, port, user, database)) except Exception as e: logger.error('PostgreSQL exception {0}'.format(e)) From c1b9af165761312cb13049a729f01681c1b8f467 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Thu, 24 Dec 2015 10:43:26 +0100 Subject: [PATCH 2/6] Redefine some options: use -H and -P for help and pid select, and -h/-p for host port. --- pg_view.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pg_view.py b/pg_view.py index f52adf9..64023cc 100644 --- a/pg_view.py +++ b/pg_view.py @@ -104,7 +104,8 @@ def output_method_is_valid(method): def parse_args(): '''parse command-line options''' - parser = OptionParser() + parser = OptionParser(add_help_option=False) + parser.add_option('-H', '--help', help='show_help', action='help') parser.add_option('-v', '--verbose', help='verbose mode', action='store_true', dest='verbose') parser.add_option('-i', '--instance', help='name of the instance to monitor', action='store', dest='instance') parser.add_option('-t', '--tick', help='tick length (in seconds)', @@ -120,15 +121,15 @@ def parse_args(): dest='clear_screen') parser.add_option('-c', '--configuration-file', help='configuration file for PostgreSQL connections', action='store', default='', dest='config_file') - parser.add_option('-p', '--pid', help='always track a given pid (may be used multiple times)', + parser.add_option('-P', '--pid', help='always track a given pid (may be used multiple times)', action='append', type=int, default=[]) parser.add_option('-U', '--username', help='database user name', action='store', dest='username') parser.add_option('-d', '--dbname', help='database name to connect to', action='store', dest='dbname') - parser.add_option('-H', '--host', help='database connection host (or a directory path for the unix socket connection)', + parser.add_option('-h', '--host', help='database connection host (or a directory path for the unix socket connection)', action='store', dest='host') - parser.add_option('-P', '--port', help='database port number', action='store', dest='port') + parser.add_option('-p', '--port', help='database port number', action='store', dest='port') options, args = parser.parse_args() return options, args From ac38f4a169fe89b153d5d618a05b944cc6aad095 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Thu, 24 Dec 2015 10:49:33 +0100 Subject: [PATCH 3/6] Make flake8 happy. --- pg_view.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pg_view.py b/pg_view.py index 64023cc..e97e3e5 100644 --- a/pg_view.py +++ b/pg_view.py @@ -127,7 +127,8 @@ def parse_args(): action='store', dest='username') parser.add_option('-d', '--dbname', help='database name to connect to', action='store', dest='dbname') - parser.add_option('-h', '--host', help='database connection host (or a directory path for the unix socket connection)', + parser.add_option('-h', '--host', help='database connection host ' + '(or a directory path for the unix socket connection)', action='store', dest='host') parser.add_option('-p', '--port', help='database port number', action='store', dest='port') @@ -3291,7 +3292,8 @@ def main(): # pass already aquired connections to make sure we only list unique clusters. host = config[instance].get('host') port = config[instance].get('port') - user, database = detect_default_user_database(config[instance].get('user'), config[instance].get('database')) + user, database = detect_default_user_database(config[instance].get('user'), + config[instance].get('database')) if not establish_user_defined_connection(instance, clusters, host, port, user, database): logger.error('failed to acquire details about ' + From 02c4c16b62d4185fe51fe9cf109a6511a1f8875c Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Thu, 24 Dec 2015 11:36:04 +0100 Subject: [PATCH 4/6] Rely on libpq to supply connection parameter defaults. --- pg_view.py | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/pg_view.py b/pg_view.py index e97e3e5..c9fa8af 100644 --- a/pg_view.py +++ b/pg_view.py @@ -5,7 +5,6 @@ import re import sys import glob -import getpass import logging from optparse import OptionParser from operator import itemgetter @@ -2930,10 +2929,17 @@ def detect_db_port(socket_dir): return port -def detect_default_user_database(user, database): - user = user or os.environ.get('PGUSER') or getpass.getuser() - database = database or os.environ.get('PGDATABASE') or user - return (user, database) +def build_connection_string(host, port, user, database): + result = [] + if host: + result.append('host={0}'.format(host)) + if port: + result.append('port={0}'.format(port)) + if user: + result.append('user={0}'.format(user)) + if database: + result.append('database={0}'.format(database)) + return ' '.join(result) def detect_postgres_version(work_directory): @@ -3033,9 +3039,9 @@ def pick_connection_arguments(conn_args): def can_connect_with_connection_arguments(host, port): """ check that we can connect given the specified arguments """ - user, database = detect_default_user_database(options.username, options.dbname) + connstring = build_connection_string(host, port, options.username, options.dbname) try: - test_conn = psycopg2.connect('host={0} port={1} user={2} dbname={3}'.format(host, port, user, database)) + test_conn = psycopg2.connect(connstring) test_conn.close() except psycopg2.OperationalError: return False @@ -3078,17 +3084,12 @@ def detect_db_connection_arguments(work_directory, pid, version): return result -def establish_user_defined_connection(instance, clusters, host, port, user, dbname): +def establish_user_defined_connection(instance, connstring, 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 - connstring = "host={0} port={1} user={2} dbname={3}".format(host, port, user, dbname) - # sanity check - if not (host and port and user and dbname): - logger.error("Connection string is incomplete: {0}".format(connstring)) - return None # establish a new connection try: pgcon = psycopg2.connect('{0}'.format(connstring)) @@ -3108,7 +3109,7 @@ def establish_user_defined_connection(instance, clusters, host, port, user, dbna # now, when we have the work directory, acquire the pid of the postmaster. pid = read_postmaster_pid(work_directory, instance) if pid is None: - logger.error('failed to read pid of the postmaster on {0}:{1}'.format(host, port)) + logger.error('failed to read pid of the postmaster on {0}'.format(connstring)) return None # check that we don't have the same pid already in the accumulated results. # for instance, a user may specify 2 different set of connection options for @@ -3292,18 +3293,18 @@ def main(): # pass already aquired connections to make sure we only list unique clusters. host = config[instance].get('host') port = config[instance].get('port') - user, database = detect_default_user_database(config[instance].get('user'), - config[instance].get('database')) + connstring = build_connection_string(host, port, + config[instance].get('user'), config[instance].get('database')) - if not establish_user_defined_connection(instance, clusters, host, port, user, database): + if not establish_user_defined_connection(instance, connstring, clusters): 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 - user, dbname = detect_default_user_database(options.username, options.dbname) + connstring = build_connection_string(options.host, options.port, options.username, options.dbname) instance = options.instance or "default" - if not establish_user_defined_connection(instance, clusters, options.host, port, user, dbname): + if not establish_user_defined_connection(instance, connstring, clusters): logger.error("unable to continue with cluster {0}".format(instance)) else: # do autodetection @@ -3324,8 +3325,8 @@ def main(): continue host = conndata['host'] port = conndata['port'] - user, database = detect_default_user_database(options.username, options.dbname) - pgcon = psycopg2.connect('host={0} port={1} user={2} dbname={3}'.format(host, port, user, database)) + connstring = build_connection_string(host, port, options.username, options.dbname) + pgcon = psycopg2.connect(connstring) except Exception as e: logger.error('PostgreSQL exception {0}'.format(e)) pgcon = None From 8eb945716676049fe1b0fca38b78888dfe96fac4 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Tue, 29 Dec 2015 10:18:04 +0100 Subject: [PATCH 5/6] Pass connection arguments as a dict, not as string. Passing it as string required proper escaping. --- pg_view.py | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/pg_view.py b/pg_view.py index c9fa8af..e17087e 100644 --- a/pg_view.py +++ b/pg_view.py @@ -2929,17 +2929,17 @@ def detect_db_port(socket_dir): return port -def build_connection_string(host, port, user, database): - result = [] +def build_connection(host, port, user, database): + result = {} if host: - result.append('host={0}'.format(host)) + result['host'] = host if port: - result.append('port={0}'.format(port)) + result['port'] = port if user: - result.append('user={0}'.format(user)) + result['user'] = user if database: - result.append('database={0}'.format(database)) - return ' '.join(result) + result['dbname'] = database + return result def detect_postgres_version(work_directory): @@ -3039,9 +3039,9 @@ def pick_connection_arguments(conn_args): def can_connect_with_connection_arguments(host, port): """ check that we can connect given the specified arguments """ - connstring = build_connection_string(host, port, options.username, options.dbname) + conn = build_connection(host, port, options.username, options.dbname) try: - test_conn = psycopg2.connect(connstring) + test_conn = psycopg2.connect(**conn) test_conn.close() except psycopg2.OperationalError: return False @@ -3084,7 +3084,7 @@ def detect_db_connection_arguments(work_directory, pid, version): return result -def establish_user_defined_connection(instance, connstring, clusters): +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 """ @@ -3092,9 +3092,9 @@ def establish_user_defined_connection(instance, connstring, clusters): pgcon = None # establish a new connection try: - pgcon = psycopg2.connect('{0}'.format(connstring)) + pgcon = psycopg2.connect(**conn) except Exception as e: - logger.error('failed to establish connection to {0} via {1}'.format(instance, connstring)) + logger.error('failed to establish connection to {0} via {1}'.format(instance, conn)) logger.error('PostgreSQL exception: {0}'.format(e)) return None # get the database version from the pgcon properties @@ -3109,7 +3109,7 @@ def establish_user_defined_connection(instance, connstring, clusters): # now, when we have the work directory, acquire the pid of the postmaster. pid = read_postmaster_pid(work_directory, instance) if pid is None: - logger.error('failed to read pid of the postmaster on {0}'.format(connstring)) + logger.error('failed to read pid of the postmaster on {0}'.format(conn)) return None # check that we don't have the same pid already in the accumulated results. # for instance, a user may specify 2 different set of connection options for @@ -3293,18 +3293,18 @@ def main(): # pass already aquired connections to make sure we only list unique clusters. host = config[instance].get('host') port = config[instance].get('port') - connstring = build_connection_string(host, port, - config[instance].get('user'), config[instance].get('database')) + conn = build_connection(host, port, + config[instance].get('user'), config[instance].get('database')) - if not establish_user_defined_connection(instance, connstring, clusters): + if not establish_user_defined_connection(instance, conn, clusters): 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 - connstring = build_connection_string(options.host, options.port, options.username, options.dbname) + conn = build_connection(options.host, options.port, options.username, options.dbname) instance = options.instance or "default" - if not establish_user_defined_connection(instance, connstring, clusters): + if not establish_user_defined_connection(instance, conn, clusters): logger.error("unable to continue with cluster {0}".format(instance)) else: # do autodetection @@ -3325,8 +3325,8 @@ def main(): continue host = conndata['host'] port = conndata['port'] - connstring = build_connection_string(host, port, options.username, options.dbname) - pgcon = psycopg2.connect(connstring) + conn = build_connection(host, port, options.username, options.dbname) + pgcon = psycopg2.connect(**conn) except Exception as e: logger.error('PostgreSQL exception {0}'.format(e)) pgcon = None From 85d64a4183fd4782635c0aaa05f15b5dd0058bfb Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Tue, 29 Dec 2015 16:14:36 +0100 Subject: [PATCH 6/6] For keyword-based connections database should be used. Although dbname also works, it's specifically written in the docs to use 'database' for keyword arguments, so the other version may be obsoleted in the future. --- pg_view.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pg_view.py b/pg_view.py index e17087e..441dc16 100644 --- a/pg_view.py +++ b/pg_view.py @@ -2938,7 +2938,7 @@ def build_connection(host, port, user, database): if user: result['user'] = user if database: - result['dbname'] = database + result['database'] = database return result