From 53f991df0f46f6b806adb6663ab45afca6fe2d46 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Mon, 8 Aug 2016 15:30:33 +0200 Subject: [PATCH] More code-review related fixes - Add missing delete_cluster. - Simplify parts of the code by removing exception handlers where they are not needed. - Fix typos. --- patroni/ctl.py | 19 +++++++------------ patroni/dcs/__init__.py | 2 +- tests/test_ctl.py | 9 ++++----- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/patroni/ctl.py b/patroni/ctl.py index cc0bca7e..f6fedfc3 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -661,11 +661,8 @@ def touch_member(config, dcs): 'state': p.state, 'role': p.role } - try: - dcs.touch_member(json.dumps(data, separators=(',', ':')), permanent=True) - except DCSError: - return False - return True + + return dcs.touch_member(json.dumps(data, separators=(',', ':')), permanent=True) def set_defaults(config, cluster_name): @@ -694,11 +691,9 @@ def scaffold(cluster_name, config_file, dcs, sysid): set_defaults(config, cluster_name) - try: - # make sure the leader keys will never expire - if not (touch_member(config, dcs) and dcs.attempt_to_acquire_leader(permanent=True)): - # we did initialize this cluster, but failed to write the leader or member keys, wipe it down completely. - raise PatroniCtlException("Unable to install permanent leader for cluster {0}".format(cluster_name)) - except: - raise + # make sure the leader keys will never expire + if not (touch_member(config, dcs) and dcs.attempt_to_acquire_leader(permanent=True)): + # we did initialize this cluster, but failed to write the leader or member keys, wipe it down completely. + dcs.delete_cluster() + raise PatroniCtlException("Unable to install permanent leader for cluster {0}".format(cluster_name)) click.echo("Cluster {0} has been created successfully".format(cluster_name)) diff --git a/patroni/dcs/__init__.py b/patroni/dcs/__init__.py index 480c85bb..a37ac6c9 100644 --- a/patroni/dcs/__init__.py +++ b/patroni/dcs/__init__.py @@ -316,7 +316,7 @@ class AbstractDCS(object): def attempt_to_acquire_leader(self, permanent=False): """Attempt to acquire leader lock This method should create `/leader` key with value=`~self._name` - :param permanent: if set to `!True`, the leader key will never expie. Used in patronictl for the external master + :param permanent: if set to `!True`, the leader key will never expire. Used in patronictl for the external master :returns: `!True` if key has been created successfully. Key must be created atomically. In case if key already exists it should not be diff --git a/tests/test_ctl.py b/tests/test_ctl.py index de07ff44..270dff1f 100644 --- a/tests/test_ctl.py +++ b/tests/test_ctl.py @@ -10,8 +10,7 @@ from six.moves import BaseHTTPServer from click.testing import CliRunner from mock import patch, Mock from patroni.ctl import ctl, members, store_config, load_config, output_members, post_patroni, get_dcs, parse_dcs, \ - wait_for_leader, get_all_members, get_any_member, get_cursor, query_member, configure, touch_member, set_defaults,\ - PatroniCtlException + wait_for_leader, get_all_members, get_any_member, get_cursor, query_member, configure, PatroniCtlException from patroni.exceptions import DCSError from psycopg2 import OperationalError @@ -316,9 +315,9 @@ class TestCtl(unittest.TestCase): with patch.object(self.e, 'initialize', return_value=False): result = self.runner.invoke(ctl, ['scaffold', 'alpha']) - assert result.exit_code == 1 + assert result.exception - with patch.object(mock_get_dcs.return_value, 'touch_member', Mock(side_effect=DCSError("foo"))): + with patch.object(mock_get_dcs.return_value, 'touch_member', Mock(return_value=False)): result = self.runner.invoke(ctl, ['scaffold', 'alpha']) assert result.exception @@ -327,4 +326,4 @@ class TestCtl(unittest.TestCase): mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader result = self.runner.invoke(ctl, ['scaffold', 'alpha']) - assert result.exit_code == 1 + assert result.exception