From 5cbf4e70d49ef05300c917a9aa270cd66d4d07e1 Mon Sep 17 00:00:00 2001 From: Martin BTS Date: Sat, 16 Nov 2019 13:31:37 +0100 Subject: [PATCH] Lost patches and a bugfix * askbot/patches/__init__.py relied on some ancient method for obtaining the currently installed Django version => updated * Missing change: use the exported instances from askbot.deployment.parameters instead of creating new instances all the time * Missing change: add reset() to ConfigManger because for testing purposes we now need to reset ConfigManagers as we are not creating new instances for each test * Bugfix: ConfigManager._order(unsorted_list) would not return the result of its ordering, but the original unsorted_list, making this method effectively a noop. It now returns an ordered list as intended. --- askbot/deployment/__init__.py | 4 +- askbot/deployment/base/configmanager.py | 9 ++- .../deployment/parameters/configmanagers.py | 11 +++ askbot/deployment/parameters/database.py | 2 +- askbot/patches/__init__.py | 3 +- askbot/tests/test_installer.py | 74 +++++++++---------- 6 files changed, 59 insertions(+), 44 deletions(-) diff --git a/askbot/deployment/__init__.py b/askbot/deployment/__init__.py index 77283d799e..346d0c981e 100644 --- a/askbot/deployment/__init__.py +++ b/askbot/deployment/__init__.py @@ -11,7 +11,7 @@ from askbot.deployment import path_utils from askbot.utils.functions import generate_random_key from askbot.deployment.template_loader import DeploymentTemplate -from askbot.deployment.parameters import ConfigManagerCollection +from askbot.deployment.parameters import askbotCollection from askbot.deployment.base import ObjectWithOutput from askbot.deployment.deployables.components import DeployableComponent import askbot.deployment.deployables as deployable @@ -24,7 +24,7 @@ def __init__(self, interactive=True, verbosity=-128): super(AskbotSetup, self).__init__(verbosity=verbosity) self.parser = ArgumentParser(description="Setup a Django project and app for Askbot") self._todo = {} - self.configManagers = ConfigManagerCollection(interactive=interactive, verbosity=verbosity) + self.configManagers = askbotCollection self.database_engines = self.configManagers.configManager( 'database').configField( 'database_engine').database_engines diff --git a/askbot/deployment/base/configmanager.py b/askbot/deployment/base/configmanager.py index aa8834433f..8ae9b2097c 100644 --- a/askbot/deployment/base/configmanager.py +++ b/askbot/deployment/base/configmanager.py @@ -125,7 +125,7 @@ def _order(self, keys): if f in known_fields: # only fields the caller wants sorted ordered_keys.append(f) known_fields.remove(f) # avoid duplicates - return keys + return ordered_keys def complete(self, collection): """Main method of this :class:ConfigManager. @@ -139,6 +139,13 @@ def complete(self, collection): contribution.setdefault(k, v) collection.update(contribution) + def reset(self): + """ConfigManagers may keep a state. This method shall be used to reset + the state to whatever that means for the specific config manager. This + implementation merely flushes the memory about completed work.""" + self._managed_config = dict() + + # one could make a case for not deriving ConfigManagerCollection from # ConfigManager because the collection serves a different purpose than the diff --git a/askbot/deployment/parameters/configmanagers.py b/askbot/deployment/parameters/configmanagers.py index 823169d459..afdee7fc4b 100644 --- a/askbot/deployment/parameters/configmanagers.py +++ b/askbot/deployment/parameters/configmanagers.py @@ -15,10 +15,21 @@ def _remember(self, name, value): self._catalog['cache_db'].defaultOk = False self._catalog['cache_password'].defaultOk = False + def reset(self): + super(CacheConfigManager, self).reset() + self._catalog['cache_nodes'].defaultOk = False + self._catalog['cache_db'].defaultOk = True + self._catalog['cache_password'].defaultOk = True + class DbConfigManager(ConfigManager): """A config manager for validating setup parameters pertaining to the database Askbot will use.""" + def reset(self): + super(DbConfigManager, self).reset() + self._catalog['database_user'].defaultOk = False + self._catalog['database_password'].defaultOk = False + def _remember(self, name, value): if name == 'database_engine': value = int(value) diff --git a/askbot/deployment/parameters/database.py b/askbot/deployment/parameters/database.py index 99ccd9851f..a9753b3159 100644 --- a/askbot/deployment/parameters/database.py +++ b/askbot/deployment/parameters/database.py @@ -37,7 +37,7 @@ def acceptable(self, value): self.print(f'DbEngine.complete called with {value} of type {type(value)}', 2) try: return value in [e[0] for e in self.database_engines] - finally: + except AttributeError: return False def ask_user(self, current_value, depth=0): diff --git a/askbot/patches/__init__.py b/askbot/patches/__init__.py index daa9c5bcb0..70f377fffc 100644 --- a/askbot/patches/__init__.py +++ b/askbot/patches/__init__.py @@ -4,10 +4,9 @@ """ import django from askbot.patches import django_patches -from askbot.deployment import package_utils def patch_django(): - (major, minor, micro) = package_utils.get_django_version() + (major, minor, micro, _, __) = django.VERSION if major == 1 and minor > 4: # This shouldn't be required with django < 1.4.x diff --git a/askbot/tests/test_installer.py b/askbot/tests/test_installer.py index 6094a5662d..7932464549 100644 --- a/askbot/tests/test_installer.py +++ b/askbot/tests/test_installer.py @@ -29,7 +29,7 @@ def test_get_options(self): default_dict = vars(default_opts) def test_db_configmanager(self): - manager = DbConfigManager(interactive=False, verbosity=0) + manager = databaseManager new_empty = lambda:dict([(k,None) for k in manager.keys]) parameters = new_empty() # includes ALL database parameters @@ -54,29 +54,29 @@ class DatabaseEngineTest(AskbotTestCase): def setUp(self): self.installer = AskbotSetup() self.parser = self.installer.parser + self.manager = databaseManager + self.manager.reset() def test_database_engine(self): - manager = DbConfigManager(interactive=False, verbosity=0) - new_empty = lambda: dict([(k, None) for k in manager.keys]) - + new_empty = lambda: dict([(k, None) for k in self.manager.keys]) # DbConfigManager is supposed to test database_engine first # here: engine is NOT acceptable and name is NOT acceptable parameters = new_empty() # includes ALL database parameters try: - manager.complete(parameters) + self.manager.complete(parameters) except ValueError as e: self.assertIn('database_engine', str(e)) # With a database_engine set, users must provide a database_name # here: engine is acceptable and name is NOT acceptable - engines = manager._catalog['database_engine'].database_engines + engines = self.manager._catalog['database_engine'].database_engines parameters = {'database_engine': None, 'database_name': None} caught_exceptions = 0 for db_type in [e[0] for e in engines]: parameters['database_engine'] = db_type try: - manager.complete(parameters) + self.manager.complete(parameters) except ValueError as ve: caught_exceptions += 1 self.assertIn('database_name', str(ve)) @@ -86,17 +86,17 @@ def test_database_engine(self): parameters = {'database_engine': None, 'database_name': 'acceptable_value'} e = None try: - manager.complete(parameters) + self.manager.complete(parameters) except ValueError as ve: e = ve self.assertIn('database_engine', str(e)) # here: engine is acceptable and name is acceptable - acceptable_engine = manager._catalog['database_engine'].database_engines[0][0] + acceptable_engine = self.manager._catalog['database_engine'].database_engines[0][0] parameters = {'database_engine': acceptable_engine, 'database_name': 'acceptable_value'} e = None try: - manager.complete(parameters) + self.manager.complete(parameters) except ValueError as ve: e = ve self.assertIsNone(e) @@ -104,27 +104,27 @@ def test_database_engine(self): # at the moment, the parameter parse does not have special code for # mysql and oracle, so we do not provide dedicated tests for them def test_database_postgres(self): - manager = DbConfigManager(interactive=False, verbosity=0) - new_empty = lambda: dict([(k, None) for k in manager.keys]) + new_empty = lambda: dict([(k, None) for k in self.manager.keys]) parameters = new_empty() parameters['database_engine'] = 1 - acceptable_answers = { - 'database_name': 'testDB', - 'database_user': 'askbot', - 'database_password': 'd34db33f', - } - expected_issues = acceptable_answers.keys() + ordered_acceptable_answers = ( + ('database_name', 'testDB'), + ('database_user', 'askbot'), + ('database_password', 'd34db33f'), + ) + + acceptable_answers = dict(ordered_acceptable_answers) + expected_issues = [item[0] for item in ordered_acceptable_answers] met_issues = set() for i in expected_issues: e = None try: - manager.complete(parameters) + self.manager.complete(parameters) except ValueError as ve: e = ve matches = [issue for issue in expected_issues if issue in str(e)] self.assertEqual(len(matches), 1, str(e)) - self.assertIs(type(e), ValueError) issue = matches[0] cnt_old = len(met_issues) @@ -135,14 +135,13 @@ def test_database_postgres(self): self.assertEqual(len(expected_issues), len(met_issues)) e = None try: - manager.complete(parameters) + self.manager.complete(parameters) except ValueError as ve: e = ve self.assertIsNone(e) def test_database_sqlite(self): - manager = DbConfigManager(interactive=False, verbosity=0) - new_empty = lambda: dict([(k, None) for k in manager.keys]) + new_empty = lambda: dict([(k, None) for k in self.manager.keys]) parameters = new_empty() parameters['database_engine'] = 2 @@ -154,7 +153,7 @@ def test_database_sqlite(self): for i in expected_issues: e = None try: - manager.complete(parameters) + self.manager.complete(parameters) except ValueError as ve: e = ve matches = [issue for issue in expected_issues if issue in str(e)] @@ -170,7 +169,7 @@ def test_database_sqlite(self): self.assertEqual(len(expected_issues), len(met_issues)) e = None try: - manager.complete(parameters) + self.manager.complete(parameters) except ValueError as ve: e = ve self.assertIsNone(e) @@ -181,13 +180,13 @@ class CacheEngineTest(AskbotTestCase): def setUp(self): self.installer = AskbotSetup() self.parser = self.installer.parser + self.manager = cacheManager + self.manager.reset() - @staticmethod - def _setUpTest(): - manager = CacheConfigManager(interactive=False, verbosity=0) - engines = manager._catalog['cache_engine'].cache_engines - new_empty = lambda: dict([(k, None) for k in manager.keys]) - return manager, engines, new_empty + def _setUpTest(self): + engines = self.manager._catalog['cache_engine'].cache_engines + new_empty = lambda: dict([(k, None) for k in self.manager.keys]) + return self.manager, engines, new_empty @staticmethod def run_complete(manager, parameters): @@ -325,12 +324,12 @@ class FilesystemTests(AskbotTestCase): def setUp(self): self.installer = AskbotSetup() self.parser = self.installer.parser + self.manager = filesystemManager + self.manager.reset() - @staticmethod - def _setUpTest(): - manager = FilesystemConfigManager(interactive=False, verbosity=0) - new_empty = lambda: dict([(k, None) for k in manager.keys]) - return manager, new_empty + def _setUpTest(self): + new_empty = lambda: dict([(k, None) for k in self.manager.keys]) + return self.manager, new_empty @staticmethod def run_complete(manager, parameters): @@ -415,12 +414,11 @@ def __test_project_dir_interactive(self): """The console functions contain endless loops, which impedes testability. If we mock the console functions, then there is no merrit in testing interactively at all.""" - manager = FilesystemConfigManager(interactive=True, verbosity=1) parameters = {'dir_name': ''} #with patch('askbot.utils.console.simple_dialog', return_value='validDeployment'), patch('askbot.utils.console.choice_dialog', return_value='yes'): with patch('builtins.input', new=MockInput('martin', 'yes')): - e = self.run_complete(manager, parameters) + e = self.run_complete(self.manager, parameters) self.assertIsNone(e) class MainInstallerTests(AskbotTestCase):