github-personal/patroni
1
0
Fork 0
mirror of https://github.com/outbackdingo/patroni.git synced 2026-01-27 02:20:01 +00:00
Code Issues Packages Projects Releases Wiki Activity

Improve quality of code by resolving issues found by quantifiedcode and codacy

Browse Source
This commit is contained in:
Alexander Kukushkin
2016-02-12 14:52:23 +01:00
parent 36aef07738
commit ab0ef91f24
5 changed files with 89 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
patroni/api.py
View File

@@ -253,7 +253,7 @@ class RestApiHandler(BaseHTTPRequestHandler):
return {'tags': self.server.patroni.tags}
def log_message(self, fmt, *args):
logger.debug("API thread: " + fmt % args)
logger.debug("API thread: %s - - [%s] %s", self.client_address[0], self.log_date_time_string() + fmt % args)
class RestApiServer(ThreadingMixIn, HTTPServer, Thread):

48
patroni/ctl.py
View File

@@ -61,7 +61,7 @@ def load_config(path, dcs):
try:
with open(path, 'rb') as fd:
config = yaml.safe_load(fd)
except:
except (IOError, yaml.YAMLError):
logging.exception('Could not load configuration file')
if dcs:
@@ -81,7 +81,7 @@ def store_config(config, path):
option_config_file = click.option('--config-file', '-c', help='Configuration file', default=CONFIG_FILE_PATH)
option_format = click.option('--format', '-f', help='Output format (pretty, json)', default='pretty')
option_format = click.option('--format', '-f', 'fmt', help='Output format (pretty, json)', default='pretty')
option_dcs = click.option('--dcs', '-d', help='Use this DCS', envvar='DCS')
option_watchrefresh = click.option('-w', '--watch', type=float, help='Auto update the screen every X seconds')
option_watch = click.option('-W', is_flag=True, help='Auto update the screen every 2 seconds')
@@ -114,9 +114,9 @@ def post_patroni(member, endpoint, content, headers=None):
data=json.dumps(content), timeout=60)
def print_output(columns, rows=None, alignment=None, format='pretty', header=True, delimiter='\t'):
def print_output(columns, rows=None, alignment=None, fmt='pretty', header=True, delimiter='\t'):
rows = rows or []
if format == 'pretty':
if fmt == 'pretty':
t = PrettyTable(columns)
for k, v in (alignment or {}).items():
t.align[k] = v
@@ -125,14 +125,14 @@ def print_output(columns, rows=None, alignment=None, format='pretty', header=Tru
click.echo(t)
return
if format == 'json':
if fmt == 'json':
elements = list()
for r in rows:
elements.append(dict(zip(columns, r)))
click.echo(json.dumps(elements))
if format == 'tsv':
if fmt == 'tsv':
if columns is not None and header:
click.echo(delimiter.join(columns) + '\n')
@@ -251,8 +251,8 @@ def dsn(cluster_name, config_file, dcs, role, member):
@click.argument('cluster_name')
@option_config_file
@option_format
@click.option('--format', help='Output format (pretty, json)', default='tsv')
@click.option('--file', '-f', help='Execute the SQL commands from this file', type=click.File('rb'))
@click.option('--format', 'fmt', help='Output format (pretty, json)', default='tsv')
@click.option('--file', '-f', 'p_file', help='Execute the SQL commands from this file', type=click.File('rb'))
@click.option('--password', help='force password prompt', is_flag=True)
@click.option('-U', '--username', help='database user name', type=str)
@option_dcs
@@ -274,21 +274,21 @@ def query(
watch,
delimiter,
command,
file,
p_file,
password,
username,
dbname,
format='tsv',
fmt='tsv',
):
if role is not None and member is not None:
raise PatroniCtlException('--role and --member are mutually exclusive options')
if member is None and role is None:
role = 'master'
if file is not None and command is not None:
if p_file is not None and command is not None:
raise PatroniCtlException('--file and --command are mutually exclusive options')
if file is None and command is None:
if p_file is None and command is None:
raise PatroniCtlException('You need to specify either --command or --file')
connect_parameters = dict()
@@ -299,8 +299,8 @@ def query(
if dbname:
connect_parameters['database'] = dbname
if file is not None:
command = file.read()
if p_file is not None:
command = p_file.read()
config, dcs, cluster = ctl_load_config(cluster_name, config_file, dcs)
@@ -309,7 +309,7 @@ def query(
output, cursor = query_member(cluster=cluster, cursor=cursor, member=member, role=role, command=command,
connect_parameters=connect_parameters)
print_output(None, output, format=format, delimiter=delimiter)
print_output(None, output, fmt=fmt, delimiter=delimiter)
if cursor is None:
cluster = dcs.get_cluster()
@@ -351,13 +351,13 @@ def query_member(cluster, cursor, member, role, command, connect_parameters=None
@option_config_file
@option_format
@option_dcs
def remove(config_file, cluster_name, format, dcs):
def remove(config_file, cluster_name, fmt, dcs):
config, dcs, cluster = ctl_load_config(cluster_name, config_file, dcs)
if not isinstance(dcs, Etcd):
raise PatroniCtlException('We have not implemented this for DCS of type {0}'.format(type(dcs)))
output_members(cluster, format=format)
output_members(cluster, fmt=fmt)
confirm = click.prompt('Please confirm the cluster name to remove', type=str)
if confirm != cluster_name:
@@ -431,11 +431,11 @@ def ctl_load_config(cluster_name, config_file, dcs):
@click.argument('member_names', nargs=-1)
@click.option('--role', '-r', help='Restart only members with this role', default='any',
type=click.Choice(['master', 'replica', 'any']))
@click.option('--any', help='Restart a single member only', is_flag=True)
@click.option('--any', 'p_any', help='Restart a single member only', is_flag=True)
@option_config_file
@option_force
@option_dcs
def restart(cluster_name, member_names, config_file, dcs, force, role, any):
def restart(cluster_name, member_names, config_file, dcs, force, role, p_any):
config, dcs, cluster = ctl_load_config(cluster_name, config_file, dcs)
role_names = [m.name for m in get_all_members(cluster=cluster, role=role)]
@@ -445,7 +445,7 @@ def restart(cluster_name, member_names, config_file, dcs, force, role, any):
else:
member_names = role_names
if any:
if p_any:
random.shuffle(member_names)
member_names = member_names[:1]
@@ -552,7 +552,7 @@ def failover(config_file, cluster_name, master, candidate, force, dcs):
output_members(cluster, name=cluster_name)
def output_members(cluster, name=None, format='pretty'):
def output_members(cluster, name=None, fmt='pretty'):
rows = []
logging.debug(cluster)
leader_name = None
@@ -597,7 +597,7 @@ def output_members(cluster, name=None, format='pretty'):
]
alignment = {'Cluster': 'l', 'Member': 'l', 'Host': 'l', 'Lag in MB': 'r'}
print_output(columns, rows, alignment, format)
print_output(columns, rows, alignment, fmt)
@ctl.command('list', help='List the Patroni members for a given Patroni')
@@ -607,7 +607,7 @@ def output_members(cluster, name=None, format='pretty'):
@option_watch
@option_watchrefresh
@option_dcs
def members(config_file, cluster_names, format, watch, w, dcs):
def members(config_file, cluster_names, fmt, watch, w, dcs):
if not cluster_names:
logging.warning('Listing members: No cluster names were provided')
return
@@ -617,7 +617,7 @@ def members(config_file, cluster_names, format, watch, w, dcs):
dcs = get_dcs(config, cn)
for _ in watching(w, watch):
output_members(dcs.get_cluster(), name=cn, format=format)
output_members(dcs.get_cluster(), name=cn, fmt=fmt)
def timestamp(precision=6):

17
patroni/scripts/wale_restore.py
View File

@@ -107,23 +107,18 @@ class WALERestore(object):
# construct the LSN from the segment and offset
backup_start_lsn = '{0}/{1}'.format(lsn_segment, lsn_offset)
conn = None
cursor = None
diff_in_bytes = long(backup_size)
if not self.no_master:
try:
# get the difference in bytes between the current WAL location and the backup start offset
conn = psycopg2.connect(self.master_connection)
conn.autocommit = True
cursor = conn.cursor()
cursor.execute("SELECT pg_xlog_location_diff(pg_current_xlog_location(), %s)", (backup_start_lsn,))
diff_in_bytes = long(cursor.fetchone()[0])
with psycopg2.connect(self.master_connection) as con:
con.autocommit = True
with con.cursor() as cur:
cur.execute("SELECT pg_xlog_location_diff(pg_current_xlog_location(), %s)", (backup_start_lsn,))
diff_in_bytes = long(cur.fetchone()[0])
except psycopg2.Error as e:
logger.error('could not determine difference with the master location: {}'.format(e))
logger.error('could not determine difference with the master location: %s', e)
return False
finally:
cursor and cursor.close()
conn and conn.close()
else:
# always try to use WAL-E if base backup is available
diff_in_bytes = 0

2
patronictl.py
View File

@@ -2,4 +2,4 @@
from patroni.ctl import ctl
if __name__ == '__main__':
ctl()
ctl(None)

129
tests/test_ctl.py
View File

@@ -53,6 +53,7 @@ class TestCtl(unittest.TestCase):
@patch('socket.getaddrinfo', socket_getaddrinfo)
def setUp(self):
self.runner = CliRunner()
with patch.object(Client, 'machines') as mock_machines:
mock_machines.__get__ = Mock(return_value=['http://remotehost:2379'])
self.p = MockPostgresql()
@@ -83,9 +84,9 @@ class TestCtl(unittest.TestCase):
def test_output_members(self):
cluster = get_cluster_initialized_with_leader()
output_members(cluster, name='abc', format='pretty')
output_members(cluster, name='abc', format='json')
output_members(cluster, name='abc', format='tsv')
output_members(cluster, name='abc', fmt='pretty')
output_members(cluster, name='abc', fmt='json')
output_members(cluster, name='abc', fmt='tsv')
@patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_leader()))
@patch('patroni.etcd.Etcd.get_etcd_client', Mock(return_value=None))
@@ -95,49 +96,47 @@ class TestCtl(unittest.TestCase):
@patch('requests.post', requests_get)
@patch('patroni.ctl.post_patroni', Mock(return_value=MockResponse()))
def test_failover(self):
runner = CliRunner()
with patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_leader())):
result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader
result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader
other
y''')
assert 'Failing over to new leader' in result.output
result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader
result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader
other
N''')
assert 'Aborting failover' in str(result.output)
result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader
result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader
leader
y''')
assert 'target and source are the same' in str(result.output)
result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader
result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader
Reality
y''')
assert 'Reality does not exist' in str(result.output)
result = runner.invoke(ctl, ['failover', 'dummy', '--force'])
result = self.runner.invoke(ctl, ['failover', 'dummy', '--force'])
assert 'Failing over to new leader' in result.output
result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='dummy')
result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='dummy')
assert 'is not the leader of cluster' in str(result.output)
with patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_only_leader())):
result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader
result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader
other
y''')
assert 'No candidates found to failover to' in str(result.output)
with patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_without_leader())):
result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader
result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader
other
y''')
assert 'This cluster has no master' in str(result.output)
with patch('patroni.ctl.post_patroni', Mock(side_effect=Exception())):
result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader
result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader
other
y''')
assert 'falling back to DCS' in result.output
@@ -146,27 +145,27 @@ y''')
mocked = Mock()
mocked.return_value.status_code = 500
with patch('patroni.ctl.post_patroni', Mock(return_value=mocked)):
result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader
result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader
other
y''')
assert 'Failover failed, details' in result.output
# with patch('patroni.dcs.AbstractDCS.get_cluster', Mock(return_value=get_cluster_initialized_with_leader())):
# result = runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='nonsense')
# result = self.runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='nonsense')
# assert 'is not the leader of cluster' in str(result.output)
# result = runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8', '--master', 'nonsense'])
# assert 'is not the leader of cluster' in str(result.output)
# result = runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='leader\nother\nn')
# assert 'Aborting failover' in str(result.output)
# with patch('patroni.ctl.wait_for_leader', Mock(return_value = get_cluster_initialized_with_leader())):
# result = runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='leader\nother\nY')
# assert 'master did not change after' in result.output
# result = runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='leader\nother\nY')
# assert 'Failover failed' in result.output
#
# result = self.runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8', '--master', 'nonsense'])
# assert 'is not the leader of cluster' in str(result.output)
#
# result = self.runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='leader\nother\nn')
# assert 'Aborting failover' in str(result.output)
#
# with patch('patroni.ctl.wait_for_leader', Mock(return_value = get_cluster_initialized_with_leader())):
# result = self.runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='leader\nother\nY')
# assert 'master did not change after' in result.output
#
# result = self.runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='leader\nother\nY')
# assert 'Failover failed' in result.output
def test_(self):
self.assertRaises(patroni.exceptions.PatroniCtlException, get_dcs, {'scheme': 'dummy'}, 'dummy')
@@ -174,10 +173,8 @@ y''')
@patch('psycopg2.connect', psycopg2_connect)
@patch('patroni.ctl.query_member', Mock(return_value=([['mock column']], None)))
def test_query(self):
runner = CliRunner()
with patch('patroni.ctl.get_dcs', Mock(return_value=self.e)):
result = runner.invoke(ctl, [
result = self.runner.invoke(ctl, [
'query',
'alpha',
'--member',
@@ -187,23 +184,23 @@ y''')
])
assert 'mutually exclusive' in str(result.output)
with runner.isolated_filesystem():
with self.runner.isolated_filesystem():
with open('dummy', 'w') as dummy_file:
dummy_file.write('SELECT 1')
result = runner.invoke(ctl, [
result = self.runner.invoke(ctl, [
'query',
'alpha'
])
assert 'You need to specify' in str(result.output)
result = runner.invoke(ctl, [
result = self.runner.invoke(ctl, [
'query',
'alpha'
])
assert 'You need to specify' in str(result.output)
result = runner.invoke(ctl, [
result = self.runner.invoke(ctl, [
'query',
'alpha',
'--file',
@@ -213,15 +210,15 @@ y''')
])
assert 'mutually exclusive' in str(result.output)
result = runner.invoke(ctl, ['query', 'alpha', '--file', 'dummy'])
result = self.runner.invoke(ctl, ['query', 'alpha', '--file', 'dummy'])
os.remove('dummy')
result = runner.invoke(ctl, ['query', 'alpha', '--command', 'SELECT 1'])
result = self.runner.invoke(ctl, ['query', 'alpha', '--command', 'SELECT 1'])
assert 'mock column' in result.output
result = runner.invoke(ctl, ['query', 'alpha', '--command', 'SELECT 1', '--dbname', 'dummy',
'--password', '--username', 'dummy'], input='password\n')
result = self.runner.invoke(ctl, ['query', 'alpha', '--command', 'SELECT 1', '--dbname', 'dummy',
'--password', '--username', 'dummy'], input='password\n')
assert 'mock column' in result.output
@patch('patroni.ctl.get_cursor', Mock(return_value=MockConnect().cursor()))
@@ -247,13 +244,11 @@ y''')
@patch('patroni.dcs.AbstractDCS.get_cluster', Mock(return_value=get_cluster_initialized_with_leader()))
def test_dsn(self):
runner = CliRunner()
with patch('patroni.ctl.get_dcs', Mock(return_value=self.e)):
result = runner.invoke(ctl, ['dsn', 'alpha', '--dcs', '8.8.8.8'])
result = self.runner.invoke(ctl, ['dsn', 'alpha', '--dcs', '8.8.8.8'])
assert 'host=127.0.0.1 port=5435' in result.output
result = runner.invoke(ctl, [
result = self.runner.invoke(ctl, [
'dsn',
'alpha',
'--role',
@@ -263,10 +258,10 @@ y''')
])
assert 'mutually exclusive' in str(result.output)
result = runner.invoke(ctl, ['dsn', 'alpha', '--member', 'dummy'])
result = self.runner.invoke(ctl, ['dsn', 'alpha', '--member', 'dummy'])
assert 'Can not find' in str(result.output)
# result = runner.invoke(ctl, ['dsn', 'alpha', '--dcs', '8.8.8.8', '--role', 'replica'])
# result = self.runner.invoke(ctl, ['dsn', 'alpha', '--dcs', '8.8.8.8', '--role', 'replica'])
# assert 'host=127.0.0.1 port=5436' in result.output
@patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_leader()))
@@ -274,13 +269,11 @@ y''')
@patch('requests.get', requests_get)
@patch('requests.post', requests_get)
def test_restart_reinit(self):
runner = CliRunner()
result = self.runner.invoke(ctl, ['restart', 'alpha', '--dcs', '8.8.8.8'], input='y')
result = self.runner.invoke(ctl, ['reinit', 'alpha', '--dcs', '8.8.8.8'], input='y')
result = runner.invoke(ctl, ['restart', 'alpha', '--dcs', '8.8.8.8'], input='y')
result = runner.invoke(ctl, ['reinit', 'alpha', '--dcs', '8.8.8.8'], input='y')
result = runner.invoke(ctl, ['restart', 'alpha', '--dcs', '8.8.8.8'], input='N')
result = runner.invoke(ctl, [
result = self.runner.invoke(ctl, ['restart', 'alpha', '--dcs', '8.8.8.8'], input='N')
result = self.runner.invoke(ctl, [
'restart',
'alpha',
'--dcs',
@@ -291,36 +284,34 @@ y''')
assert 'not a member' in str(result.output)
with patch('requests.post', Mock(return_value=MockResponse())):
result = runner.invoke(ctl, ['restart', 'alpha', '--dcs', '8.8.8.8'], input='y')
result = self.runner.invoke(ctl, ['restart', 'alpha', '--dcs', '8.8.8.8'], input='y')
@patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_leader()))
@patch('patroni.etcd.Etcd.get_etcd_client', Mock(return_value=None))
def test_remove(self):
runner = CliRunner()
result = runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='alpha\nslave')
result = self.runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='alpha\nslave')
assert 'Please confirm' in result.output
assert 'You are about to remove all' in result.output
assert 'You did not exactly type' in str(result.output)
result = runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='''alpha
result = self.runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='''alpha
Yes I am aware
slave''')
assert 'You did not specify the current master of the cluster' in str(result.output)
result = runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='beta\nleader')
result = self.runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='beta\nleader')
assert 'Cluster names specified do not match' in str(result.output)
with patch('patroni.etcd.Etcd.get_cluster', get_cluster_initialized_with_leader):
result = runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'],
input='''alpha
result = self.runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'],
input='''alpha
Yes I am aware
leader''')
assert 'object has no attribute' in str(result.exception)
with patch('patroni.ctl.get_dcs', Mock(return_value=Mock())):
result = runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'],
input='''alpha
result = self.runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'],
input='''alpha
Yes I am aware
leader''')
assert 'We have not implemented this for DCS of type' in str(result.output)
@@ -340,11 +331,9 @@ leader''')
self.assertRaises(requests.exceptions.ConnectionError, post_patroni, member, 'dummy', {})
def test_ctl(self):
runner = CliRunner()
self.runner.invoke(ctl, ['list'])
runner.invoke(ctl, ['list'])
result = runner.invoke(ctl, ['--help'])
result = self.runner.invoke(ctl, ['--help'])
assert 'Usage:' in result.output
def test_get_any_member(self):
@@ -374,15 +363,11 @@ leader''')
@patch('requests.get', requests_get)
@patch('requests.post', requests_get)
def test_members(self):
runner = CliRunner()
result = runner.invoke(members, ['alpha'])
result = self.runner.invoke(members, ['alpha'])
assert result.exit_code == 0
def test_configure(self):
runner = CliRunner()
result = runner.invoke(configure, [
result = self.runner.invoke(configure, [
'--dcs',
'abc',
'-c',