From 0693fe7dd09bd4c2d193cbc1ee2a0df9c390fcc0 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Wed, 4 Dec 2019 11:28:46 +0100 Subject: [PATCH] Housekeeping (#1315) * Reduce memory usage by patroni init process * More cleanup in setup.py * Implement missing tests --- patroni/__init__.py | 2 +- patroni/postgresql/__init__.py | 5 ++--- setup.py | 38 +++++++++++++++++++--------------- tests/test_api.py | 4 ++++ tests/test_ctl.py | 8 ++++++- tests/test_log.py | 4 ++++ tests/test_postgresql.py | 2 +- 7 files changed, 40 insertions(+), 23 deletions(-) diff --git a/patroni/__init__.py b/patroni/__init__.py index e3699986..5a681de0 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -217,8 +217,8 @@ def check_psycopg2(): def main(): - check_psycopg2() if os.getpid() != 1: + check_psycopg2() return patroni_main() # Patroni started with PID=1, it looks like we are in the container diff --git a/patroni/postgresql/__init__.py b/patroni/postgresql/__init__.py index d84afd33..be864fba 100644 --- a/patroni/postgresql/__init__.py +++ b/patroni/postgresql/__init__.py @@ -627,9 +627,8 @@ class Postgresql(object): # Don't try to call pg_controldata during backup restore if self._version_file_exists() and self.state != 'creating replica': try: - env = {'LANG': 'C', 'LC_ALL': 'C', 'PATH': os.getenv('PATH')} - if os.getenv('SYSTEMROOT') is not None: - env['SYSTEMROOT'] = os.getenv('SYSTEMROOT') + env = os.environ.copy() + env.update(LANG='C', LC_ALL='C') data = subprocess.check_output([self.pgcommand('pg_controldata'), self._data_dir], env=env) if data: data = data.decode('utf-8').splitlines() diff --git a/setup.py b/setup.py index f82d37de..ac280920 100644 --- a/setup.py +++ b/setup.py @@ -8,16 +8,8 @@ import inspect import os import sys -from patroni import check_psycopg2, fatal -from patroni.version import __version__ as VERSION from setuptools import Command, find_packages, setup -if sys.version_info < (2, 7, 0): - fatal('patroni needs to be run with Python 2.7+') -check_psycopg2() -del sys.modules['patroni'] -del sys.modules['patroni.version'] - __location__ = os.path.join(os.getcwd(), os.path.dirname(inspect.getfile(inspect.currentframe()))) NAME = 'patroni' @@ -30,6 +22,8 @@ AUTHOR_EMAIL = 'alexander.kukushkin@zalando.de, dmitrii.dolgov@zalando.de, alexk KEYWORDS = 'etcd governor patroni postgresql postgres ha haproxy confd' +\ ' zookeeper exhibitor consul streaming replication kubernetes k8s' +EXTRAS_REQUIRE = {'aws': ['boto'], 'etcd': ['python-etcd'], 'consul': ['python-consul'], + 'exhibitor': ['kazoo'], 'zookeeper': ['kazoo'], 'kubernetes': ['kubernetes']} COVERAGE_XML = True COVERAGE_HTML = False @@ -99,7 +93,7 @@ class PyTest(Command): def run(self): from pkg_resources import evaluate_marker - requirements = self.distribution.install_requires + ['flake8', 'mock>=2.0.0', 'pytest-cov', 'pytest'] +\ + requirements = self.distribution.install_requires + ['mock>=2.0.0', 'pytest-cov', 'pytest'] +\ [v for k, v in self.distribution.extras_require.items() if not k.startswith(':') or evaluate_marker(k[1:])] self.distribution.fetch_build_eggs(requirements) self.run_tests() @@ -110,22 +104,20 @@ def read(fname): return fd.read() -def setup_package(): +def setup_package(version): # Assemble additional setup commands cmdclass = {'test': PyTest} install_requires = [] - extras_require = {'aws': ['boto'], 'etcd': ['python-etcd'], 'consul': ['python-consul'], - 'exhibitor': ['kazoo'], 'zookeeper': ['kazoo'], 'kubernetes': ['kubernetes']} for r in read('requirements.txt').split('\n'): r = r.strip() if r == '': continue extra = False - for e, v in extras_require.items(): + for e, v in EXTRAS_REQUIRE.items(): if r.startswith(v[0]): - extras_require[e] = [r] + EXTRAS_REQUIRE[e] = [r] extra = True if not extra: install_requires.append(r) @@ -138,7 +130,7 @@ def setup_package(): setup( name=NAME, - version=VERSION, + version=version, url=URL, author=AUTHOR, author_email=AUTHOR_EMAIL, @@ -151,7 +143,8 @@ def setup_package(): package_data={MAIN_PACKAGE: ["*.json"]}, python_requires='>=2.7', install_requires=install_requires, - extras_require=extras_require, + extras_require=EXTRAS_REQUIRE, + setup_requires='flake8', cmdclass=cmdclass, command_options=command_options, entry_points={'console_scripts': CONSOLE_SCRIPTS}, @@ -159,4 +152,15 @@ def setup_package(): if __name__ == '__main__': - setup_package() + old_modules = sys.modules.copy() + try: + from patroni import check_psycopg2, fatal, __version__ + finally: + sys.modules.clear() + sys.modules.update(old_modules) + + if sys.version_info < (2, 7, 0): + fatal('Patroni needs to be run with Python 2.7+') + check_psycopg2() + + setup_package(__version__) diff --git a/tests/test_api.py b/tests/test_api.py index a6230ecd..803b15ed 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -438,3 +438,7 @@ class TestRestApiServer(unittest.TestCase): raise Exception() except Exception: self.assertIsNone(MockRestApiServer.handle_error(None, ('127.0.0.1', 55555))) + + def test_socket_error(self): + with patch.object(BaseHTTPServer.HTTPServer, '__init__', Mock(side_effect=socket.error)): + self.assertRaises(socket.error, MockRestApiServer, Mock(), '', {'listen': '*:8008'}) diff --git a/tests/test_ctl.py b/tests/test_ctl.py index be858262..c035a4ea 100644 --- a/tests/test_ctl.py +++ b/tests/test_ctl.py @@ -1,6 +1,5 @@ import etcd import os -import sys import unittest from click.testing import CliRunner @@ -584,3 +583,10 @@ class TestCtl(unittest.TestCase): self.assertIsNone(find_executable('vim')) with patch('os.path.isfile', Mock(side_effect=[False, True])): self.assertEqual(find_executable('vim', '/'), '/vim.exe') + + @patch('patroni.ctl.get_dcs') + def test_get_members(self, mock_get_dcs): + mock_get_dcs.return_value = self.e + mock_get_dcs.return_value.get_cluster = get_cluster_not_initialized_without_leader + result = self.runner.invoke(ctl, ['reinit', 'dummy']) + assert "cluster doesn\'t have any members" in result.output diff --git a/tests/test_log.py b/tests/test_log.py index 688feb72..613d18cd 100644 --- a/tests/test_log.py +++ b/tests/test_log.py @@ -9,6 +9,8 @@ from patroni.config import Config from patroni.log import PatroniLogger from six.moves.queue import Queue, Full +_LOG = logging.getLogger(__name__) + class TestPatroniLogger(unittest.TestCase): @@ -38,6 +40,7 @@ class TestPatroniLogger(unittest.TestCase): logger = PatroniLogger() patroni_config = Config(None) logger.reload_config(patroni_config['log']) + _LOG.exception('test') logger.start() with patch.object(logging.Handler, 'format', Mock(side_effect=Exception)): @@ -46,6 +49,7 @@ class TestPatroniLogger(unittest.TestCase): self.assertEqual(logger.log_handler.maxBytes, config['log']['file_size']) self.assertEqual(logger.log_handler.backupCount, config['log']['file_num']) + config['log']['level'] = 'DEBUG' config['log'].pop('dir') with patch('logging.Handler.close', Mock(side_effect=Exception)): logger.reload_config(config['log']) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 4930d21d..a0118370 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -222,7 +222,7 @@ class TestPostgresql(BaseTestPostgresql): mock_get_pg_settings.return_value['recovery_min_apply_delay'][1] = '1' self.assertEqual(self.p.config.check_recovery_conf(None), (True, True)) mock_get_pg_settings.return_value['primary_conninfo'][1] = 'host=1 passfile='\ - + re.sub(r'([\'\\ ])', r'\\\1', self.p.config._pgpass) + + re.sub(r'([\'\\ ])', r'\\\1', self.p.config._pgpass) mock_get_pg_settings.return_value['recovery_min_apply_delay'][1] = '0' self.assertEqual(self.p.config.check_recovery_conf(None), (True, True)) self.p.config.write_recovery_conf({'standby_mode': 'on', 'primary_conninfo': conninfo.copy()})